Make video tiles be based on MatrixRTC member not LiveKit participants (#2701)

* make tiles based on rtc member

* display missing lk participant + fix tile multiplier

* add show_non_member_participants config option

* per member tiles

* merge fixes

* linter

* linter and tests

* tests

* adapt tests (wip)

* Remove unused keys

* Fix optionality of nonMemberItemCount

* video is optional

* Mock RTC members

* Lint

* Merge fixes

* Fix user id

* Add explicit types for public fields

* isRTCParticipantAvailable => isLiveKitParticipantAvailable

* isLiveKitParticipantAvailable

* Readonly

* More keys removal

* Make local field based on view model class not observable

* Wording

* Fix RTC members in tes

* Tests again

* Lint

* Disable showing non-member tiles by default

* Duplicate screen sharing tiles like we used to

* Lint

* Revert function reordering

* Remove throttleTime from bad merge

* Cleanup

* Tidy config of show non-member settings

* tidy up handling of local rtc member in tests

* tidy up test init

* Fix mocks

* Cleanup

* Apply local override where participant not yet known

* Handle no visible media id

* Assertions for one-on-one view

* Remove isLiveKitParticipantAvailable and show via encryption status

* Handle no local media (yet)

* Remove unused effect for setting

* Tidy settings

* Avoid case of one-to-one layout with missing local or remote

* Iterate

* Remove option to show non-member tiles to simplify code review

* Remove unused code

* Remove more remnants of show-non-member-tiles

* iterate

* back

* Fix unit test

* Refactor

* Expose TestScheduler as global

* Fix incorrect type assertion

* Simplify speaking observer

* Fix

* Whitespace

* Make it clear that we are mocking MatrixRTC memberships

* Test case for only showing tiles for MatrixRTC session members

* Simplify diff

* Simplify diff

These changes are in https://github.com/element-hq/element-call/pull/2809

* .

* Whitespaces

* Use asObservable when exposing subject

* Show "waiting for media..." when no participant

* Additional test case

* Don't show "waiting for media..." in case of local participant

* Make the loading state more subtle
 - instead of a label we show a animated gradient

* Use correct key for matrix rtc foci in code comment. (#2838)

* Update src/tile/SpotlightTile.tsx

Co-authored-by: Timo <16718859+toger5@users.noreply.github.com>

* Update src/state/CallViewModel.ts

Co-authored-by: Timo <16718859+toger5@users.noreply.github.com>

* Make the purpose of BaseMediaViewModel.local explicit

* Use named object instead of unnamed array for spotlightAndPip

* Refactor spotlightAndPip into spotlight and pip

* Use if statement instead of ternary for readability in spotlight and pip logic

* Review feedback

* Fix tests for CallEventAudioRenderer

* Lint

* Revert "Make the loading state more subtle"

This reverts commit 765f7b4f319b86839fcb4fde28d1e0604e542577.

* Update src/state/CallViewModel.ts

Co-authored-by: Timo <16718859+toger5@users.noreply.github.com>

* Fix spelling

* Remove a non-null assertion that failed at runtime

---------

Co-authored-by: Hugh Nimmo-Smith <hughns@element.io>
Co-authored-by: Hugh Nimmo-Smith <hughns@users.noreply.github.com>
This commit is contained in:
Timo
2024-12-06 12:28:37 +01:00
committed by GitHub
parent 21b62dbd89
commit 43c81a2758
16 changed files with 729 additions and 307 deletions

View File

@@ -13,7 +13,7 @@ import { of } from "rxjs";
import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession";
import { GridTile } from "./GridTile";
import { withRemoteMedia } from "../utils/test";
import { mockRtcMembership, withRemoteMedia } from "../utils/test";
import { GridTileViewModel } from "../state/TileViewModel";
import { ReactionsProvider } from "../useReactions";
@@ -25,6 +25,7 @@ global.IntersectionObserver = class MockIntersectionObserver {
test("GridTile is accessible", async () => {
await withRemoteMedia(
mockRtcMembership("@alice:example.org", "AAAA"),
{
rawDisplayName: "Alice",
getMxcAvatarUrl: () => "mxc://adfsg",

View File

@@ -175,6 +175,7 @@ const UserMediaTile = forwardRef<HTMLDivElement, UserMediaTileProps>(
raisedHandTime={handRaised}
currentReaction={currentReaction}
raisedHandOnClick={raisedHandOnClick}
localParticipant={vm.local}
{...props}
/>
);

View File

@@ -74,9 +74,9 @@ unconditionally select the container so we can use cqmin units */
calc(var(--media-view-border-radius) - var(--cpd-space-3x))
);
display: grid;
grid-template-columns: 1fr auto;
grid-template-columns: 30px 1fr 30px;
grid-template-rows: 1fr auto;
grid-template-areas: "status status" "nameTag button";
grid-template-areas: "reactions status ." "nameTag nameTag button";
gap: var(--cpd-space-1x);
place-items: start;
}
@@ -101,8 +101,8 @@ unconditionally select the container so we can use cqmin units */
grid-area: status;
justify-self: center;
align-self: start;
padding: var(--cpd-space-1x);
padding-block: var(--cpd-space-1x);
padding: var(--cpd-space-2x);
padding-block: var(--cpd-space-2x);
color: var(--cpd-color-text-primary);
background-color: var(--cpd-color-bg-canvas-default);
display: flex;
@@ -116,6 +116,12 @@ unconditionally select the container so we can use cqmin units */
text-align: center;
}
.reactions {
grid-area: reactions;
display: flex;
gap: var(--cpd-space-1x);
}
.nameTag > svg,
.nameTag > span {
flex-shrink: 0;

View File

@@ -5,7 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only
Please see LICENSE in the repository root for full details.
*/
import { describe, expect, test } from "vitest";
import { describe, expect, it, test } from "vitest";
import { render, screen } from "@testing-library/react";
import { axe } from "vitest-axe";
import { TooltipProvider } from "@vector-im/compound-web";
@@ -42,6 +42,7 @@ describe("MediaView", () => {
unencryptedWarning: false,
video: trackReference,
member: undefined,
localParticipant: false,
};
test("is accessible", async () => {
@@ -59,6 +60,25 @@ describe("MediaView", () => {
});
});
describe("with no participant", () => {
it("shows avatar for local user", () => {
render(
<MediaView {...baseProps} video={undefined} localParticipant={true} />,
);
expect(screen.getByRole("img", { name: "some name" })).toBeVisible();
expect(screen.queryAllByText("video_tile.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: "some name" })).toBeVisible();
expect(screen.getByText("video_tile.waiting_for_media")).toBeVisible();
});
});
describe("name tag", () => {
test("is shown with name", () => {
render(<MediaView {...baseProps} displayName="Bob" />);

View File

@@ -28,7 +28,7 @@ interface Props extends ComponentProps<typeof animated.div> {
style?: ComponentProps<typeof animated.div>["style"];
targetWidth: number;
targetHeight: number;
video: TrackReferenceOrPlaceholder;
video: TrackReferenceOrPlaceholder | undefined;
videoFit: "cover" | "contain";
mirror: boolean;
member: RoomMember | undefined;
@@ -41,6 +41,7 @@ interface Props extends ComponentProps<typeof animated.div> {
raisedHandTime?: Date;
currentReaction?: ReactionOption;
raisedHandOnClick?: () => void;
localParticipant: boolean;
}
export const MediaView = forwardRef<HTMLDivElement, Props>(
@@ -63,6 +64,7 @@ export const MediaView = forwardRef<HTMLDivElement, Props>(
raisedHandTime,
currentReaction,
raisedHandOnClick,
localParticipant,
...props
},
ref,
@@ -90,21 +92,21 @@ export const MediaView = forwardRef<HTMLDivElement, Props>(
size={avatarSize}
src={member?.getMxcAvatarUrl()}
className={styles.avatar}
style={{ display: videoEnabled ? "none" : "initial" }}
style={{ display: video && videoEnabled ? "none" : "initial" }}
/>
{video.publication !== undefined && (
{video?.publication !== undefined && (
<VideoTrack
trackRef={video}
// There's no reason for this to be focusable
tabIndex={-1}
disablePictureInPicture
style={{ display: videoEnabled ? "block" : "none" }}
style={{ display: video && videoEnabled ? "block" : "none" }}
data-testid="video"
/>
)}
</div>
<div className={styles.fg}>
<div style={{ display: "flex", gap: "var(--cpd-space-1x)" }}>
<div className={styles.reactions}>
<RaisedHandIndicator
raisedHandTime={raisedHandTime}
miniature={avatarSize < 96}
@@ -118,6 +120,11 @@ export const MediaView = forwardRef<HTMLDivElement, Props>(
/>
)}
</div>
{!video && !localParticipant && (
<div className={styles.status}>
{t("video_tile.waiting_for_media")}
</div>
)}
{/* TODO: Bring this back once encryption status is less broken */}
{/*encryptionStatus !== EncryptionStatus.Okay && (
<div className={styles.status}>

View File

@@ -12,7 +12,11 @@ import userEvent from "@testing-library/user-event";
import { of } from "rxjs";
import { SpotlightTile } from "./SpotlightTile";
import { withLocalMedia, withRemoteMedia } from "../utils/test";
import {
mockRtcMembership,
withLocalMedia,
withRemoteMedia,
} from "../utils/test";
import { SpotlightTileViewModel } from "../state/TileViewModel";
global.IntersectionObserver = class MockIntersectionObserver {
@@ -22,6 +26,7 @@ global.IntersectionObserver = class MockIntersectionObserver {
test("SpotlightTile is accessible", async () => {
await withRemoteMedia(
mockRtcMembership("@alice:example.org", "AAAA"),
{
rawDisplayName: "Alice",
getMxcAvatarUrl: () => "mxc://adfsg",
@@ -29,6 +34,7 @@ test("SpotlightTile is accessible", async () => {
{},
async (vm1) => {
await withLocalMedia(
mockRtcMembership("@bob:example.org", "BBBB"),
{
rawDisplayName: "Bob",
getMxcAvatarUrl: () => "mxc://dlskf",

View File

@@ -49,12 +49,13 @@ interface SpotlightItemBaseProps {
"data-id": string;
targetWidth: number;
targetHeight: number;
video: TrackReferenceOrPlaceholder;
video: TrackReferenceOrPlaceholder | undefined;
member: RoomMember | undefined;
unencryptedWarning: boolean;
encryptionStatus: EncryptionStatus;
displayName: string;
"aria-hidden"?: boolean;
localParticipant: boolean;
}
interface SpotlightUserMediaItemBaseProps extends SpotlightItemBaseProps {
@@ -163,6 +164,7 @@ const SpotlightItem = forwardRef<HTMLDivElement, SpotlightItemProps>(
displayName,
encryptionStatus,
"aria-hidden": ariaHidden,
localParticipant: vm.local,
};
return vm instanceof ScreenShareViewModel ? (
@@ -210,7 +212,9 @@ export const SpotlightTile = forwardRef<HTMLDivElement, Props>(
const ref = useMergedRefs(ourRef, theirRef);
const maximised = useObservableEagerState(vm.maximised);
const media = useObservableEagerState(vm.media);
const [visibleId, setVisibleId] = useState(media[0].id);
const [visibleId, setVisibleId] = useState<string | undefined>(
media[0]?.id,
);
const latestMedia = useLatest(media);
const latestVisibleId = useLatest(visibleId);
const visibleIndex = media.findIndex((vm) => vm.id === visibleId);