review: Use targetWidth/Height instead of listening to element bounds

This commit is contained in:
Valere
2026-03-09 09:45:25 +01:00
parent 273fff20bd
commit 513477d280
6 changed files with 37 additions and 65 deletions

View File

@@ -27,7 +27,13 @@ interface Props<M, R extends HTMLElement> {
state: Parameters<Handler<"drag", EventTypes["drag"]>>[0], state: Parameters<Handler<"drag", EventTypes["drag"]>>[0],
) => void ) => void
> | null; > | null;
/**
* The width this tile will have once its animations have settled.
*/
targetWidth: number; targetWidth: number;
/**
* The width this tile will have once its animations have settled.
*/
targetHeight: number; targetHeight: number;
model: M; model: M;
Tile: ComponentType<TileProps<M, R>>; Tile: ComponentType<TileProps<M, R>>;

View File

@@ -62,12 +62,12 @@ export interface BaseUserMediaViewModel extends MemberMediaViewModel {
RTCInboundRtpStreamStats | RTCOutboundRtpStreamStats | undefined RTCInboundRtpStreamStats | RTCOutboundRtpStreamStats | undefined
>; >;
/** /**
* Set the actual dimensions of the HTML element. * Set the target dimensions of the HTML element (final dimension after anim).
* This can be used to determine the best video fit (fit to frame / keep ratio). * This can be used to determine the best video fit (fit to frame / keep ratio).
* @param width - The actual width of the HTML element displaying the video. * @param targetWidth - The target width of the HTML element displaying the video.
* @param height - The actual height of the HTML element displaying the video. * @param targetHeight - The target height of the HTML element displaying the video.
*/ */
setActualDimensions: (width: number, height: number) => void; setTargetDimensions: (targetWidth: number, targetHeight: number) => void;
} }
export interface BaseUserMediaInputs extends Omit< export interface BaseUserMediaInputs extends Omit<
@@ -98,7 +98,9 @@ export function createBaseUserMedia(
); );
const toggleCropVideo$ = new Subject<void>(); const toggleCropVideo$ = new Subject<void>();
const actualSize$ = new BehaviorSubject< // The target size of the video element, used to determine the best video fit.
// The target size is the final size of the HTML element after any animations have completed.
const targetSize$ = new BehaviorSubject<
{ width: number; height: number } | undefined { width: number; height: number } | undefined
>(undefined); >(undefined);
@@ -130,7 +132,7 @@ export function createBaseUserMedia(
videoFit$: videoFit$( videoFit$: videoFit$(
scope, scope,
videoSizeFromParticipant$(participant$), videoSizeFromParticipant$(participant$),
actualSize$, targetSize$,
), ),
toggleCropVideo: () => toggleCropVideo$.next(), toggleCropVideo: () => toggleCropVideo$.next(),
rtcBackendIdentity, rtcBackendIdentity,
@@ -155,8 +157,8 @@ export function createBaseUserMedia(
return observeRtpStreamStats$(p, Track.Source.Camera, statsType); return observeRtpStreamStats$(p, Track.Source.Camera, statsType);
}), }),
), ),
setActualDimensions: (width: number, height: number): void => { setTargetDimensions: (targetWidth: number, targetHeight: number): void => {
actualSize$.next({ width, height }); targetSize$.next({ width: targetWidth, height: targetHeight });
}, },
}; };
} }

View File

@@ -6,7 +6,7 @@ Please see LICENSE in the repository root for full details.
*/ */
import { type RemoteTrackPublication } from "livekit-client"; import { type RemoteTrackPublication } from "livekit-client";
import { test, expect, beforeAll } from "vitest"; import { test, expect } from "vitest";
import { render, screen } from "@testing-library/react"; import { render, screen } from "@testing-library/react";
import { axe } from "vitest-axe"; import { axe } from "vitest-axe";
import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc"; import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc";
@@ -28,22 +28,6 @@ global.IntersectionObserver = class MockIntersectionObserver {
public disconnect(): void {} public disconnect(): void {}
} as unknown as typeof IntersectionObserver; } as unknown as typeof IntersectionObserver;
// Mock ResizeObserver as it is needed by the useMeasure hook used in the GridTile, but is not implemented in JSDOM.
// We just need to mock it with empty methods as we don't need to test its functionality here.
beforeAll(() => {
window.ResizeObserver = class ResizeObserver {
public observe(): void {
// do nothing
}
public unobserve(): void {
// do nothing
}
public disconnect(): void {
// do nothing
}
};
});
test("GridTile is accessible", async () => { test("GridTile is accessible", async () => {
const vm = mockRemoteMedia( const vm = mockRemoteMedia(
mockRtcMembership("@alice:example.org", "AAAA"), mockRtcMembership("@alice:example.org", "AAAA"),

View File

@@ -37,7 +37,6 @@ import {
Menu, Menu,
} from "@vector-im/compound-web"; } from "@vector-im/compound-web";
import { useObservableEagerState } from "observable-hooks"; import { useObservableEagerState } from "observable-hooks";
import useMeasure from "react-use-measure";
import styles from "./GridTile.module.css"; import styles from "./GridTile.module.css";
import { Slider } from "../Slider"; import { Slider } from "../Slider";
@@ -88,6 +87,8 @@ const UserMediaTile: FC<UserMediaTileProps> = ({
displayName, displayName,
mxcAvatarUrl, mxcAvatarUrl,
focusable, focusable,
targetWidth,
targetHeight,
...props ...props
}) => { }) => {
const { toggleRaisedHand } = useReactionsSender(); const { toggleRaisedHand } = useReactionsSender();
@@ -110,19 +111,12 @@ const UserMediaTile: FC<UserMediaTileProps> = ({
const handRaised = useBehavior(vm.handRaised$); const handRaised = useBehavior(vm.handRaised$);
const reaction = useBehavior(vm.reaction$); const reaction = useBehavior(vm.reaction$);
// We need to keep track of the tile size.
// We use this to get the tile ratio, and compare it to the video ratio to decide
// whether to fit the video to frame or keep the ratio.
const [measureRef, bounds] = useMeasure();
// There is already a ref being passed in, so we need to merge it with the measureRef.
const tileRef = useMergedRefs(ref, measureRef);
// Whenever bounds change, inform the viewModel // Whenever bounds change, inform the viewModel
useEffect(() => { useEffect(() => {
if (bounds.width > 0 && bounds.height > 0) { if (targetWidth > 0 && targetHeight > 0) {
vm.setActualDimensions(bounds.width, bounds.height); vm.setTargetDimensions(targetWidth, targetHeight);
} }
}, [bounds.width, bounds.height, vm]); }, [targetWidth, targetHeight, vm]);
const AudioIcon = playbackMuted const AudioIcon = playbackMuted
? VolumeOffSolidIcon ? VolumeOffSolidIcon
@@ -155,7 +149,7 @@ const UserMediaTile: FC<UserMediaTileProps> = ({
const tile = ( const tile = (
<MediaView <MediaView
ref={tileRef} ref={ref}
video={video} video={video}
userId={vm.userId} userId={vm.userId}
unencryptedWarning={unencryptedWarning} unencryptedWarning={unencryptedWarning}
@@ -207,6 +201,8 @@ const UserMediaTile: FC<UserMediaTileProps> = ({
audioStreamStats={audioStreamStats} audioStreamStats={audioStreamStats}
videoStreamStats={videoStreamStats} videoStreamStats={videoStreamStats}
rtcBackendIdentity={rtcBackendIdentity} rtcBackendIdentity={rtcBackendIdentity}
targetWidth={targetWidth}
targetHeight={targetHeight}
{...props} {...props}
/> />
); );

View File

@@ -5,7 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE in the repository root for full details. Please see LICENSE in the repository root for full details.
*/ */
import { test, expect, vi, beforeAll } from "vitest"; import { test, expect, vi } from "vitest";
import { isInaccessible, render, screen } from "@testing-library/react"; import { isInaccessible, render, screen } from "@testing-library/react";
import { axe } from "vitest-axe"; import { axe } from "vitest-axe";
import userEvent from "@testing-library/user-event"; import userEvent from "@testing-library/user-event";
@@ -27,22 +27,6 @@ global.IntersectionObserver = class MockIntersectionObserver {
public unobserve(): void {} public unobserve(): void {}
} as unknown as typeof IntersectionObserver; } as unknown as typeof IntersectionObserver;
// Mock ResizeObserver as it is needed by the useMeasure hook used in the SpotlightTile, but is not implemented in JSDOM.
// We just need to mock it with empty methods as we don't need to test its functionality here.
beforeAll(() => {
window.ResizeObserver = class ResizeObserver {
public observe(): void {
// do nothing
}
public unobserve(): void {
// do nothing
}
public disconnect(): void {
// do nothing
}
};
});
test("SpotlightTile is accessible", async () => { test("SpotlightTile is accessible", async () => {
const vm1 = mockRemoteMedia( const vm1 = mockRemoteMedia(
mockRtcMembership("@alice:example.org", "AAAA"), mockRtcMembership("@alice:example.org", "AAAA"),

View File

@@ -27,7 +27,6 @@ import { useObservableRef } from "observable-hooks";
import { useTranslation } from "react-i18next"; import { useTranslation } from "react-i18next";
import classNames from "classnames"; import classNames from "classnames";
import { type TrackReferenceOrPlaceholder } from "@livekit/components-core"; import { type TrackReferenceOrPlaceholder } from "@livekit/components-core";
import useMeasure from "react-use-measure";
import FullScreenMaximiseIcon from "../icons/FullScreenMaximise.svg?react"; import FullScreenMaximiseIcon from "../icons/FullScreenMaximise.svg?react";
import FullScreenMinimiseIcon from "../icons/FullScreenMinimise.svg?react"; import FullScreenMinimiseIcon from "../icons/FullScreenMinimise.svg?react";
@@ -152,7 +151,13 @@ const SpotlightRemoteScreenShareItem: FC<
interface SpotlightItemProps { interface SpotlightItemProps {
ref?: Ref<HTMLDivElement>; ref?: Ref<HTMLDivElement>;
vm: MediaViewModel; vm: MediaViewModel;
/**
* The width this tile will have once its animations have settled.
*/
targetWidth: number; targetWidth: number;
/**
* The height this tile will have once its animations have settled.
*/
targetHeight: number; targetHeight: number;
focusable: boolean; focusable: boolean;
intersectionObserver$: Observable<IntersectionObserver>; intersectionObserver$: Observable<IntersectionObserver>;
@@ -175,21 +180,16 @@ const SpotlightItem: FC<SpotlightItemProps> = ({
}) => { }) => {
const ourRef = useRef<HTMLDivElement | null>(null); const ourRef = useRef<HTMLDivElement | null>(null);
// We need to keep track of the tile size. // Whenever target bounds change, inform the viewModel
// We use this to get the tile ratio, and compare it to the video ratio to decide
// whether to fit the video to frame or keep the ratio.
const [measureRef, bounds] = useMeasure();
// Whenever bounds change, inform the viewModel
useEffect(() => { useEffect(() => {
if (bounds.width > 0 && bounds.height > 0) { if (targetWidth > 0 && targetHeight > 0) {
if (vm.type != "screen share") { if (vm.type != "screen share") {
vm.setActualDimensions(bounds.width, bounds.height); vm.setTargetDimensions(targetWidth, targetHeight);
} }
} }
}, [bounds.width, bounds.height, vm]); }, [targetWidth, targetHeight, vm]);
const ref = useMergedRefs(ourRef, theirRef, measureRef); const ref = useMergedRefs(ourRef, theirRef);
const focusUrl = useBehavior(vm.focusUrl$); const focusUrl = useBehavior(vm.focusUrl$);
const displayName = useBehavior(vm.displayName$); const displayName = useBehavior(vm.displayName$);
const mxcAvatarUrl = useBehavior(vm.mxcAvatarUrl$); const mxcAvatarUrl = useBehavior(vm.mxcAvatarUrl$);