Compute the 'waiting for media' state less implicitly

On second glance, the way that we determined a media tile to be 'waiting for media' was too implicit for my taste. It would appear on a surface reading to depend on whether a participant was currently publishing any video. But in reality, the 'video' object was always defined as long as a LiveKit participant existed, so in reality it depended on just the participant. We should show this relationship more explicitly by moving the computation into the view model, where it can depend on the participant directly.
This commit is contained in:
Robin
2025-12-10 17:16:38 -05:00
parent 2c54263b2f
commit 93ab3ba1ff
9 changed files with 104 additions and 36 deletions

View File

@@ -20,6 +20,7 @@ import {
createLocalMedia,
createRemoteMedia,
withTestScheduler,
mockRemoteParticipant,
} from "../utils/test";
import { getValue } from "../utils/observable";
import { constant } from "./Behavior";
@@ -44,7 +45,11 @@ const rtcMembership = mockRtcMembership("@alice:example.org", "AAAA");
test("control a participant's volume", () => {
const setVolumeSpy = vi.fn();
const vm = createRemoteMedia(rtcMembership, {}, { setVolume: setVolumeSpy });
const vm = createRemoteMedia(
rtcMembership,
{},
mockRemoteParticipant({ setVolume: setVolumeSpy }),
);
withTestScheduler(({ expectObservable, schedule }) => {
schedule("-ab---c---d|", {
a() {
@@ -88,7 +93,7 @@ test("control a participant's volume", () => {
});
test("toggle fit/contain for a participant's video", () => {
const vm = createRemoteMedia(rtcMembership, {}, {});
const vm = createRemoteMedia(rtcMembership, {}, mockRemoteParticipant({}));
withTestScheduler(({ expectObservable, schedule }) => {
schedule("-ab|", {
a: () => vm.toggleFitContain(),
@@ -199,3 +204,25 @@ test("switch cameras", async () => {
});
expect(deviceId).toBe("front camera");
});
test("remote media is in waiting state when participant has not yet connected", () => {
const vm = createRemoteMedia(rtcMembership, {}, null); // null participant
expect(vm.waitingForMedia$.value).toBe(true);
});
test("remote media is not in waiting state when participant is connected", () => {
const vm = createRemoteMedia(rtcMembership, {}, mockRemoteParticipant({}));
expect(vm.waitingForMedia$.value).toBe(false);
});
test("remote media is not in waiting state when participant is connected with no publications", () => {
const vm = createRemoteMedia(
rtcMembership,
{},
mockRemoteParticipant({
getTrackPublication: () => undefined,
getTrackPublications: () => [],
}),
);
expect(vm.waitingForMedia$.value).toBe(false);
});

View File

@@ -268,7 +268,7 @@ abstract class BaseMediaViewModel {
encryptionSystem: EncryptionSystem,
audioSource: AudioSource,
videoSource: VideoSource,
livekitRoom$: Behavior<LivekitRoom | undefined>,
protected readonly livekitRoom$: Behavior<LivekitRoom | undefined>,
public readonly focusUrl$: Behavior<string | undefined>,
public readonly displayName$: Behavior<string>,
public readonly mxcAvatarUrl$: Behavior<string | undefined>,
@@ -596,6 +596,14 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel {
* A remote participant's user media.
*/
export class RemoteUserMediaViewModel extends BaseUserMediaViewModel {
/**
* Whether we are waiting for this user's LiveKit participant to exist. This
* could be because either we or the remote party are still connecting.
*/
public readonly waitingForMedia$ = this.scope.behavior<boolean>(
this.participant$.pipe(map((participant) => participant === null)),
);
// This private field is used to override the value from the superclass
private __speaking$: Behavior<boolean>;
public get speaking$(): Behavior<boolean> {

View File

@@ -12,7 +12,11 @@ import { axe } from "vitest-axe";
import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc";
import { GridTile } from "./GridTile";
import { mockRtcMembership, createRemoteMedia } from "../utils/test";
import {
mockRtcMembership,
createRemoteMedia,
mockRemoteParticipant,
} from "../utils/test";
import { GridTileViewModel } from "../state/TileViewModel";
import { ReactionsSenderProvider } from "../reactions/useReactionsSender";
import type { CallViewModel } from "../state/CallViewModel/CallViewModel";
@@ -31,11 +35,11 @@ test("GridTile is accessible", async () => {
rawDisplayName: "Alice",
getMxcAvatarUrl: () => "mxc://adfsg",
},
{
mockRemoteParticipant({
setVolume() {},
getTrackPublication: () =>
({}) as Partial<RemoteTrackPublication> as RemoteTrackPublication,
},
}),
);
const fakeRtcSession = {

View File

@@ -69,6 +69,7 @@ interface UserMediaTileProps extends TileProps {
vm: UserMediaViewModel;
mirror: boolean;
locallyMuted: boolean;
waitingForMedia?: boolean;
primaryButton?: ReactNode;
menuStart?: ReactNode;
menuEnd?: ReactNode;
@@ -79,6 +80,7 @@ const UserMediaTile: FC<UserMediaTileProps> = ({
vm,
showSpeakingIndicators,
locallyMuted,
waitingForMedia,
primaryButton,
menuStart,
menuEnd,
@@ -194,7 +196,7 @@ const UserMediaTile: FC<UserMediaTileProps> = ({
raisedHandTime={handRaised ?? undefined}
currentReaction={reaction ?? undefined}
raisedHandOnClick={raisedHandOnClick}
localParticipant={vm.local}
waitingForMedia={waitingForMedia}
focusUrl={focusUrl}
audioStreamStats={audioStreamStats}
videoStreamStats={videoStreamStats}
@@ -290,6 +292,7 @@ const RemoteUserMediaTile: FC<RemoteUserMediaTileProps> = ({
...props
}) => {
const { t } = useTranslation();
const waitingForMedia = useBehavior(vm.waitingForMedia$);
const locallyMuted = useBehavior(vm.locallyMuted$);
const localVolume = useBehavior(vm.localVolume$);
const onSelectMute = useCallback(
@@ -311,6 +314,7 @@ const RemoteUserMediaTile: FC<RemoteUserMediaTileProps> = ({
<UserMediaTile
ref={ref}
vm={vm}
waitingForMedia={waitingForMedia}
locallyMuted={locallyMuted}
mirror={false}
menuStart={

View File

@@ -47,7 +47,6 @@ describe("MediaView", () => {
video: trackReference,
userId: "@alice:example.com",
mxcAvatarUrl: undefined,
localParticipant: false,
focusable: true,
};
@@ -66,24 +65,13 @@ describe("MediaView", () => {
});
});
describe("with no participant", () => {
it("shows avatar for local user", () => {
render(
<MediaView {...baseProps} video={undefined} localParticipant={true} />,
);
describe("with no video", () => {
it("shows avatar", () => {
render(<MediaView {...baseProps} video={undefined} />);
expect(
screen.getByRole("img", { name: "@alice:example.com" }),
).toBeVisible();
expect(screen.queryAllByText("Waiting for media...").length).toBe(0);
});
it("shows avatar and label for remote user", () => {
render(
<MediaView {...baseProps} video={undefined} localParticipant={false} />,
);
expect(
screen.getByRole("img", { name: "@alice:example.com" }),
).toBeVisible();
expect(screen.getByText("Waiting for media...")).toBeVisible();
expect(screen.queryByTestId("video")).toBe(null);
});
});
@@ -94,6 +82,22 @@ describe("MediaView", () => {
});
});
describe("waitingForMedia", () => {
test("defaults to false", () => {
render(<MediaView {...baseProps} />);
expect(screen.queryAllByText("Waiting for media...").length).toBe(0);
});
test("shows and is accessible", async () => {
const { container } = render(
<TooltipProvider>
<MediaView {...baseProps} waitingForMedia={true} />
</TooltipProvider>,
);
expect(await axe(container)).toHaveNoViolations();
expect(screen.getByText("Waiting for media...")).toBeVisible();
});
});
describe("unencryptedWarning", () => {
test("is shown and accessible", async () => {
const { container } = render(

View File

@@ -43,7 +43,7 @@ interface Props extends ComponentProps<typeof animated.div> {
raisedHandTime?: Date;
currentReaction?: ReactionOption;
raisedHandOnClick?: () => void;
localParticipant: boolean;
waitingForMedia?: boolean;
audioStreamStats?: RTCInboundRtpStreamStats | RTCOutboundRtpStreamStats;
videoStreamStats?: RTCInboundRtpStreamStats | RTCOutboundRtpStreamStats;
// The focus url, mainly for debugging purposes
@@ -71,7 +71,7 @@ export const MediaView: FC<Props> = ({
raisedHandTime,
currentReaction,
raisedHandOnClick,
localParticipant,
waitingForMedia,
audioStreamStats,
videoStreamStats,
focusUrl,
@@ -129,7 +129,7 @@ export const MediaView: FC<Props> = ({
/>
)}
</div>
{!video && !localParticipant && (
{waitingForMedia && (
<div className={styles.status}>
{t("video_tile.waiting_for_media")}
</div>

View File

@@ -17,6 +17,7 @@ import {
mockRtcMembership,
createLocalMedia,
createRemoteMedia,
mockRemoteParticipant,
} from "../utils/test";
import { SpotlightTileViewModel } from "../state/TileViewModel";
import { constant } from "../state/Behavior";
@@ -33,7 +34,7 @@ test("SpotlightTile is accessible", async () => {
rawDisplayName: "Alice",
getMxcAvatarUrl: () => "mxc://adfsg",
},
{},
mockRemoteParticipant({}),
);
const vm2 = createLocalMedia(

View File

@@ -38,6 +38,7 @@ import {
type MediaViewModel,
ScreenShareViewModel,
type UserMediaViewModel,
type RemoteUserMediaViewModel,
} from "../state/MediaViewModel";
import { useInitial } from "../useInitial";
import { useMergedRefs } from "../useMergedRefs";
@@ -84,6 +85,21 @@ const SpotlightLocalUserMediaItem: FC<SpotlightLocalUserMediaItemProps> = ({
SpotlightLocalUserMediaItem.displayName = "SpotlightLocalUserMediaItem";
interface SpotlightRemoteUserMediaItemProps
extends SpotlightUserMediaItemBaseProps {
vm: RemoteUserMediaViewModel;
}
const SpotlightRemoteUserMediaItem: FC<SpotlightRemoteUserMediaItemProps> = ({
vm,
...props
}) => {
const waitingForMedia = useBehavior(vm.waitingForMedia$);
return (
<MediaView waitingForMedia={waitingForMedia} mirror={false} {...props} />
);
};
interface SpotlightUserMediaItemProps extends SpotlightItemBaseProps {
vm: UserMediaViewModel;
}
@@ -103,7 +119,7 @@ const SpotlightUserMediaItem: FC<SpotlightUserMediaItemProps> = ({
return vm instanceof LocalUserMediaViewModel ? (
<SpotlightLocalUserMediaItem vm={vm} {...baseProps} />
) : (
<MediaView mirror={false} {...baseProps} />
<SpotlightRemoteUserMediaItem vm={vm} {...baseProps} />
);
};

View File

@@ -319,12 +319,12 @@ export function mockLocalParticipant(
}
export function createLocalMedia(
localRtcMember: CallMembership,
rtcMember: CallMembership,
roomMember: Partial<RoomMember>,
localParticipant: LocalParticipant,
mediaDevices: MediaDevices,
): LocalUserMediaViewModel {
const member = mockMatrixRoomMember(localRtcMember, roomMember);
const member = mockMatrixRoomMember(rtcMember, roomMember);
return new LocalUserMediaViewModel(
testScope(),
"local",
@@ -359,22 +359,26 @@ export function mockRemoteParticipant(
}
export function createRemoteMedia(
localRtcMember: CallMembership,
rtcMember: CallMembership,
roomMember: Partial<RoomMember>,
participant: Partial<RemoteParticipant>,
participant: RemoteParticipant | null,
): RemoteUserMediaViewModel {
const member = mockMatrixRoomMember(localRtcMember, roomMember);
const remoteParticipant = mockRemoteParticipant(participant);
const member = mockMatrixRoomMember(rtcMember, roomMember);
return new RemoteUserMediaViewModel(
testScope(),
"remote",
member.userId,
of(remoteParticipant),
constant(participant),
{
kind: E2eeType.PER_PARTICIPANT,
},
constant(
mockLivekitRoom({}, { remoteParticipants$: of([remoteParticipant]) }),
mockLivekitRoom(
{},
{
remoteParticipants$: of(participant ? [participant] : []),
},
),
),
constant("https://rtc-example.org"),
constant(false),