Review (everything except translation feedback)

This commit is contained in:
Timo K
2026-05-18 14:31:03 +02:00
parent 88f660e43f
commit e22ab9355c
16 changed files with 170 additions and 727 deletions

View File

@@ -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,

View File

@@ -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<FooterProps> = ({ 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<FooterProps> = ({ ref, children, vm }) => {
})}
>
<div className={styles.settingsLogoContainer}>
{showSettingsButton && (
{openSettings !== undefined && (
<SettingsIconButton
key="settings"
kind="secondary"

View File

@@ -104,10 +104,10 @@ describe("createCallFooterViewModel", () => {
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);
});

View File

@@ -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<boolean, "md" | "lg">((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),

View File

@@ -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<typeof meta>;
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" },

View File

@@ -36,18 +36,6 @@ export interface ToggleOption {
id: string;
}
export interface IconsAndLabels {
/** The Icon used if the mute button is enabled */
IconEnabled: ComponentType<React.SVGAttributes<SVGElement>>;
/** The Icon used if the mute button is disabled */
IconDisabled: ComponentType<React.SVGAttributes<SVGElement>>;
/** The icon used for the different options */
IconOptions?: ComponentType<React.SVGAttributes<SVGElement>>;
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<MediaMuteAndSwitchButtonProps> = ({
data-testid="incall_mute"
/>
);
break;
default:
button = (
<Button
iconOnly
role="switch"
Icon={
enabled ? iconsAndLabels?.IconEnabled : iconsAndLabels?.IconDisabled
}
onClick={(e) => {
onMuteClick?.();
e.preventDefault();
e.stopPropagation();
}}
kind={enabled ? "secondary" : "primary"}
size="lg"
className={styles.button}
aria-label={
enabled
? iconsAndLabels?.disabledLabel
: iconsAndLabels?.enabledLabel
}
/>
);
break;
}
@@ -153,14 +118,6 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
IconOptions = MicOnIcon;
optionsButtonLabel = t("settings.devices.microphone");
break;
case undefined:
IconOptions = undefined;
optionsButtonLabel = "undefined";
break;
default:
IconOptions = iconsAndLabels.IconOptions;
optionsButtonLabel = iconsAndLabels.optionsButtonLabel;
break;
}
return (
<div

View File

@@ -5,18 +5,10 @@ exports[`MediaMuteAndSwitchButton > renders 1`] = `
<div
class="container"
>
<button
class="_button_1nw83_8 button _icon-only_1nw83_53"
data-kind="primary"
data-size="lg"
role="switch"
tabindex="0"
/>
<button
aria-disabled="false"
aria-expanded="false"
aria-haspopup="menu"
aria-label="undefined"
class="_button_1nw83_8 menuButton _has-icon_1nw83_60 _icon-only_1nw83_53"
data-kind="tertiary"
data-size="lg"