From e22ab9355ce137947c66a6e2ba46f90f5dc44ab8 Mon Sep 17 00:00:00 2001 From: Timo K Date: Mon, 18 May 2026 14:31:03 +0200 Subject: [PATCH] Review (everything except translation feedback) --- src/components/CallFooter.stories.tsx | 4 +- src/components/CallFooter.tsx | 7 +- src/components/CallFooterViewModel.test.ts | 4 +- src/components/CallFooterViewModel.tsx | 40 +- .../MediaMuteAndSwitchButton.stories.tsx | 9 +- src/components/MediaMuteAndSwitchButton.tsx | 47 +- .../MediaMuteAndSwitchButton.test.tsx.snap | 8 - src/room/GroupCallView.test.tsx | 10 +- src/room/InCallView.test.tsx | 3 +- src/room/InCallView.tsx | 80 ++- src/room/LobbyView.test.tsx | 19 +- .../GroupCallErrorBoundary.test.tsx.snap | 12 +- .../__snapshots__/InCallView.test.tsx.snap | 41 +- .../__snapshots__/LobbyView.test.tsx.snap | 576 +----------------- .../DeveloperSettingsTab.test.tsx.snap | 16 +- src/utils/test-viewmodel.ts | 21 +- 16 files changed, 170 insertions(+), 727 deletions(-) diff --git a/src/components/CallFooter.stories.tsx b/src/components/CallFooter.stories.tsx index 14f82506..f46f656f 100644 --- a/src/components/CallFooter.stories.tsx +++ b/src/components/CallFooter.stories.tsx @@ -30,7 +30,7 @@ const reactionData = { * - Add additional react context * The paraeters are all params from the FooterSnapshot, * the Snapshot of the vm, the wrapper will create a mocked vm from it and pass it to the CallFooter. - * children used for the "Back to Recents" button in the lobby stories, but can be used for anything really + * `children` is used for the "Back to Recents" button in the lobby stories, but can be used for anything really. * @returns A component that renders the CallFooter based on primitive snapshot params (not a view model). Which is what we want for storybook. */ function CallFooterStoryWrapper({ @@ -71,7 +71,6 @@ const fnArgType = { export const Default: Story = { args: { showLogo: false, - showSettingsButton: true, layoutMode: "grid", audioEnabled: true, videoEnabled: true, @@ -85,7 +84,6 @@ export const Default: Story = { showFooter: true, hideControls: false, asOverlay: false, - showLayoutSwitcher: false, sharingScreen: false, audioOutputSwitcher: undefined, reactionIdentifier: undefined, diff --git a/src/components/CallFooter.tsx b/src/components/CallFooter.tsx index bd309941..dfec25a9 100644 --- a/src/components/CallFooter.tsx +++ b/src/components/CallFooter.tsx @@ -82,8 +82,6 @@ export interface FooterState { asOverlay: boolean; buttonSize: "md" | "lg"; - showSettingsButton: boolean; - showLayoutSwitcher: boolean; showLogo: boolean; layoutMode: GridMode | undefined; @@ -143,12 +141,11 @@ export const CallFooter: FC = ({ ref, children, vm }) => { const selectVideoButtonOption = useBehavior(vm.selectVideoButtonOption$); const videoToggles = useBehavior(vm.videoToggles$); const buttonSize = useBehavior(vm.buttonSize$); - const showSettingsButton = useBehavior(vm.showSettingsButton$); const showLogo = useBehavior(vm.showLogo$); const buttons: JSX.Element[] = []; - if (showSettingsButton) { + if (openSettings !== undefined) { // Add the settings button to the center group so it's visible on small // screens. On larger screens the SettingsIconButton with // showForScreenWidth="wide" in the settingsLogoContainer is used instead. @@ -291,7 +288,7 @@ export const CallFooter: FC = ({ ref, children, vm }) => { })} >
- {showSettingsButton && ( + {openSettings !== undefined && ( { expect(vm.audioOptions$.value).toEqual([]); expect(vm.videoOptions$.value).toEqual([]); } - it("are empty when both the platform is iOS", () => { + it("are both empty when the platform is iOS", () => { checkEmptyFor("ios", gridLayout); }); - it("are empty when both the layout is pip", () => { + it("are both empty when the layout is pip", () => { checkEmptyFor("desktop", pipLayout); }); diff --git a/src/components/CallFooterViewModel.tsx b/src/components/CallFooterViewModel.tsx index c2cacd8c..4be52f0d 100644 --- a/src/components/CallFooterViewModel.tsx +++ b/src/components/CallFooterViewModel.tsx @@ -188,30 +188,35 @@ export function createCallFooterViewModel( callModel.windowMode$.pipe(map((mode) => mode === "flat")), ), buttonSize$: scope.behavior( - isPip$.pipe(map((pip) => (pip ? "md" : "lg") as "md" | "lg")), + isPip$.pipe(map((pip) => (pip ? "md" : "lg"))), ), - showSettingsButton$: scope.behavior( + + openSettings$: scope.behavior( combineLatest([ isPip$, callModel.showHeader$, - callModel.settingsOpen$, + callModel.setSettingsOpen$, ]).pipe( - map( - ([isPip, showHeader, settingsOpen]) => - settingsOpen !== undefined && - !isPip && - showControls && - !(headerStyle === HeaderStyle.AppBar && showHeader), + map(([isPip, showHeader, setSettingsOpen]) => + !isPip && + !(headerStyle === HeaderStyle.AppBar && showHeader) && + showControls + ? (): void => setSettingsOpen(true) + : undefined, ), ), ), - showLayoutSwitcher$: scope.behavior( - isPip$.pipe(map((l) => !isPip$ && showControls)), - ), + showLogo$: scope.behavior(isPip$.pipe(map((isPip) => showLogo && !isPip))), layoutMode$: callModel.gridMode$, - setLayoutMode$: constant(callModel.setGridMode), + setLayoutMode$: scope.behavior( + isPip$.pipe( + map((isPip) => + !isPip && showControls ? callModel.setGridMode : undefined, + ), + ), + ), sharingScreen$: callModel.sharingScreen$, toggleScreenSharing$: constant(callModel.toggleScreenSharing ?? undefined), @@ -222,15 +227,6 @@ export function createCallFooterViewModel( ), ), - openSettings$: scope.behavior( - combineLatest([callModel.showHeader$, callModel.setSettingsOpen$]).pipe( - map(([showHeader, setSettingsOpen]) => - headerStyle === HeaderStyle.AppBar && showHeader - ? undefined - : (): void => setSettingsOpen(true), - ), - ), - ), hangup$: constant(callModel.hangup), reactionIdentifier$: constant(reactionIdentifier), diff --git a/src/components/MediaMuteAndSwitchButton.stories.tsx b/src/components/MediaMuteAndSwitchButton.stories.tsx index a109dd4c..fe68fc9b 100644 --- a/src/components/MediaMuteAndSwitchButton.stories.tsx +++ b/src/components/MediaMuteAndSwitchButton.stories.tsx @@ -5,7 +5,6 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { AdvancedSettingsIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; import { fn, userEvent, within, expect } from "storybook/test"; import type { Meta, StoryObj } from "@storybook/react-vite"; @@ -21,13 +20,7 @@ type Story = StoryObj; export const Default: Story = { args: { title: "SomeMenu", - iconsAndLabels: { - IconEnabled: AdvancedSettingsIcon, - IconDisabled: AdvancedSettingsIcon, - enabledLabel: "Enabled", - disabledLabel: "Disabled", - optionsButtonLabel: "Options", - }, + iconsAndLabels: "audio", enabled: true, options: [ { label: "option 1", id: "1" }, diff --git a/src/components/MediaMuteAndSwitchButton.tsx b/src/components/MediaMuteAndSwitchButton.tsx index e25f23f1..4a6737f7 100644 --- a/src/components/MediaMuteAndSwitchButton.tsx +++ b/src/components/MediaMuteAndSwitchButton.tsx @@ -36,18 +36,6 @@ export interface ToggleOption { id: string; } -export interface IconsAndLabels { - /** The Icon used if the mute button is enabled */ - IconEnabled: ComponentType>; - /** The Icon used if the mute button is disabled */ - IconDisabled: ComponentType>; - /** The icon used for the different options */ - IconOptions?: ComponentType>; - enabledLabel: string; - disabledLabel: string; - optionsButtonLabel: string; -} - export interface MediaMuteAndSwitchButtonProps { /** The title used in the Switcher modal. */ title: string; @@ -55,7 +43,7 @@ export interface MediaMuteAndSwitchButtonProps { enabled?: boolean; /** Callback if the mute button is clicked */ onMuteClick?: () => void; - iconsAndLabels?: "video" | "audio" | IconsAndLabels; + iconsAndLabels: "video" | "audio"; /** The options available for the media device selector modal */ options?: MenuOptions[]; /** The option that will currently be rendered as the selected option */ @@ -115,30 +103,7 @@ export const MediaMuteAndSwitchButton: FC = ({ data-testid="incall_mute" /> ); - break; - default: - button = ( - +
renders with header and participant count 1`] = `
`; - -exports[`LobbyView > renders with waiting for invite state 1`] = ` -
-
-
-
-
- - T - -
-

- Test Room -

- - - - - -
-
- - - - - - - 3 - -
-
-
-
-
-
-
-
- - Back to recents - -
-
-
- - -
-
- - - - -
-
-
-
-`; - -exports[`LobbyView > renders without header 1`] = ` -
-
-
-
-
- - Back to recents - -
-
-
- -
-
- - - - -
-
-
-
-`; diff --git a/src/settings/__snapshots__/DeveloperSettingsTab.test.tsx.snap b/src/settings/__snapshots__/DeveloperSettingsTab.test.tsx.snap index af38685a..7d66f0ac 100644 --- a/src/settings/__snapshots__/DeveloperSettingsTab.test.tsx.snap +++ b/src/settings/__snapshots__/DeveloperSettingsTab.test.tsx.snap @@ -372,9 +372,7 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = ` local )

- 
+    
       {
   "region": "local",
   "version": "1.2.3"
@@ -384,9 +382,7 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = `
     

       Local Participant
     

-    
+    
       localParticipantIdentity
     

     

@@ -413,9 +409,7 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = `
       remote
       )
     

-    
+    
       {
   "region": "remote",
   "version": "4.5.6"
@@ -425,9 +419,7 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = `
     

       Local Participant
     

-    
+    
       localParticipantIdentity
     

     

diff --git a/src/utils/test-viewmodel.ts b/src/utils/test-viewmodel.ts
index b5438371..7c670308 100644
--- a/src/utils/test-viewmodel.ts
+++ b/src/utils/test-viewmodel.ts
@@ -39,6 +39,9 @@ import { aliceRtcMember, localRtcMember } from "./test-fixtures";
 import { type RaisedHandInfo, type ReactionInfo } from "../reactions";
 import { constant } from "../state/Behavior";
 import { MatrixRTCMode } from "../settings/settings";
+import { createCallFooterViewModel } from "../components/CallFooterViewModel";
+import { type FooterSnapshot } from "../components/CallFooter";
+import { type ViewModel } from "../state/ViewModel";
 
 mockConfig({ livekit: { livekit_service_url: "https://example.com" } });
 
@@ -136,6 +139,7 @@ export function getBasicCallViewModelEnvironment(
   callViewModelOptions: Partial = {},
 ): {
   vm: CallViewModel;
+  footerVm: ViewModel;
   rtcMemberships$: BehaviorSubject;
   rtcSession: MockRTCSession;
   handRaisedSubject$: BehaviorSubject>;
@@ -148,12 +152,15 @@ export function getBasicCallViewModelEnvironment(
   const handRaisedSubject$ = new BehaviorSubject({});
   const reactionsSubject$ = new BehaviorSubject({});
 
+  const scope = testScope();
+  const muteStates = mockMuteStates();
+  const mediaDevices = mediaDevicesOverride ?? mockMediaDevices({});
   const vm = createCallViewModel$(
-    testScope(),
+    scope,
     rtcSession.asMockedSession(),
     matrixRoom,
-    mediaDevicesOverride ?? mockMediaDevices({}),
-    mockMuteStates(),
+    mediaDevices,
+    muteStates,
     {
       encryptionSystem: { kind: E2eeType.PER_PARTICIPANT },
       livekitRoomFactory: (): LivekitRoom =>
@@ -171,8 +178,16 @@ export function getBasicCallViewModelEnvironment(
     reactionsSubject$,
     constant({ processor: undefined, supported: false }),
   );
+  const footerVm = createCallFooterViewModel(
+    testScope(),
+    vm,
+    muteStates,
+    mediaDevices,
+    "reactionId",
+  );
   return {
     vm,
+    footerVm,
     rtcMemberships$,
     rtcSession,
     handRaisedSubject$: handRaisedSubject$,