Merge pull request #4013 from element-hq/valere/fix_repeated_click_to_unmute

feat(mute): add syncing state and disable toggle during async mute
This commit is contained in:
Valere Fedronic
2026-06-05 12:58:23 +02:00
committed by GitHub
15 changed files with 386 additions and 34 deletions

View File

@@ -8,3 +8,17 @@ Please see LICENSE in the repository root for full details.
.endCall > svg {
color: var(--stopgap-color-on-solid-accent);
}
.rotate > svg {
animation: spin 1s linear infinite;
}
@keyframes spin {
from {
transform: rotate(0deg);
}
to {
transform: rotate(360deg);
}
}

113
src/button/Button.test.tsx Normal file
View File

@@ -0,0 +1,113 @@
/*
Copyright 2026 Element Creations Ltd.
SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE in the repository root for full details.
*/
import { describe, expect, test, vi } from "vitest";
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { TooltipProvider } from "@vector-im/compound-web";
import { MicButton, VideoButton } from "./Button";
describe("MicButton", () => {
test("calls onClick when not busy", async () => {
const user = userEvent.setup();
const onClick = vi.fn();
render(
<TooltipProvider>
<MicButton enabled={true} onClick={onClick} />
</TooltipProvider>,
);
const button = screen.getByRole("switch");
await user.click(button);
expect(onClick).toHaveBeenCalled();
});
test("does not call onClick when busy", async () => {
const user = userEvent.setup();
const onClick = vi.fn();
render(
<TooltipProvider>
<MicButton enabled={true} busy={true} onClick={onClick} />
</TooltipProvider>,
);
const button = screen.getByRole("switch");
expect(button).toHaveAttribute("aria-disabled", "true");
expect(button).toHaveAttribute("aria-busy", "true");
await user.click(button);
expect(onClick).not.toHaveBeenCalled();
});
test("does not call onClick when disabled", async () => {
const user = userEvent.setup();
const onClick = vi.fn();
render(
<TooltipProvider>
<MicButton enabled={true} disabled={true} onClick={onClick} />
</TooltipProvider>,
);
const button = screen.getByRole("switch");
expect(button).toHaveAttribute("aria-disabled", "true");
await user.click(button);
expect(onClick).not.toHaveBeenCalled();
});
});
describe("VideoButton", () => {
test("calls onClick when not busy", async () => {
const user = userEvent.setup();
const onClick = vi.fn();
render(
<TooltipProvider>
<VideoButton enabled={true} onClick={onClick} />
</TooltipProvider>,
);
const button = screen.getByRole("switch");
await user.click(button);
expect(onClick).toHaveBeenCalled();
});
test("does not call onClick when busy", async () => {
const user = userEvent.setup();
const onClick = vi.fn();
render(
<TooltipProvider>
<VideoButton enabled={true} busy={true} onClick={onClick} />
</TooltipProvider>,
);
const button = screen.getByRole("switch");
expect(button).toHaveAttribute("aria-disabled", "true");
expect(button).toHaveAttribute("aria-busy", "true");
await user.click(button);
expect(onClick).not.toHaveBeenCalled();
});
test("does not call onClick when disabled", async () => {
const user = userEvent.setup();
const onClick = vi.fn();
render(
<TooltipProvider>
<VideoButton enabled={true} disabled={true} onClick={onClick} />
</TooltipProvider>,
);
const button = screen.getByRole("switch");
expect(button).toHaveAttribute("aria-disabled", "true");
await user.click(button);
expect(onClick).not.toHaveBeenCalled();
});
});

View File

@@ -16,6 +16,7 @@ import {
import {
MicOnSolidIcon,
MicOffSolidIcon,
SpinnerIcon,
VideoCallSolidIcon,
VideoCallOffSolidIcon,
EndCallIcon,
@@ -32,12 +33,13 @@ import { platform } from "../Platform";
interface MicButtonProps extends ComponentPropsWithoutRef<"button"> {
enabled: boolean;
busy?: boolean;
size?: "md" | "lg";
}
export const MicButton: FC<MicButtonProps> = ({ enabled, ...props }) => {
export const MicButton: FC<MicButtonProps> = ({ enabled, busy, ...props }) => {
const { t } = useTranslation();
const Icon = enabled ? MicOnSolidIcon : MicOffSolidIcon;
const Icon = busy ? SpinnerIcon : enabled ? MicOnSolidIcon : MicOffSolidIcon;
const label = enabled
? t("mute_microphone_button_label")
: t("unmute_microphone_button_label");
@@ -51,6 +53,11 @@ export const MicButton: FC<MicButtonProps> = ({ enabled, ...props }) => {
role="switch"
aria-checked={enabled}
{...props}
aria-busy={busy}
className={classNames(props.className, {
[styles.rotate]: !!busy,
})}
disabled={props.disabled || busy}
/>
</Tooltip>
);
@@ -58,12 +65,21 @@ export const MicButton: FC<MicButtonProps> = ({ enabled, ...props }) => {
interface VideoButtonProps extends ComponentPropsWithoutRef<"button"> {
enabled: boolean;
busy?: boolean;
size?: "md" | "lg";
}
export const VideoButton: FC<VideoButtonProps> = ({ enabled, ...props }) => {
export const VideoButton: FC<VideoButtonProps> = ({
enabled,
busy,
...props
}) => {
const { t } = useTranslation();
const Icon = enabled ? VideoCallSolidIcon : VideoCallOffSolidIcon;
const Icon = busy
? SpinnerIcon
: enabled
? VideoCallSolidIcon
: VideoCallOffSolidIcon;
const label = enabled
? t("stop_video_button_label")
: t("start_video_button_label");
@@ -77,6 +93,11 @@ export const VideoButton: FC<VideoButtonProps> = ({ enabled, ...props }) => {
role="switch"
aria-checked={enabled}
{...props}
aria-busy={busy}
className={classNames(props.className, {
[styles.rotate]: !!busy,
})}
disabled={props.disabled || busy}
/>
</Tooltip>
);

View File

@@ -80,7 +80,9 @@ export const Default: Story = {
showLogo: false,
layoutMode: "grid",
audioEnabled: true,
audioBusy: false,
videoEnabled: true,
videoBusy: false,
setLayoutMode: fn(),
openSettings: fn(),
toggleAudio: fn(),
@@ -152,6 +154,26 @@ export const WithAudioAndVideoOptions: Story = {
selectedVideo: "1",
},
};
export const AudioBusy: Story = {
...Default,
args: {
...Default.args,
audioEnabled: true,
audioBusy: true,
videoEnabled: true,
},
};
export const VideoBusy: Story = {
...Default,
args: {
...Default.args,
audioEnabled: true,
videoEnabled: true,
videoBusy: true,
},
};
export const WithLogo: Story = {
...Default,
args: {

View File

@@ -72,7 +72,9 @@ export interface FooterActions {
// we do not use any ? optional properties so that the vm type is including all fields.
export interface FooterState {
audioEnabled: boolean;
audioBusy: boolean;
videoEnabled: boolean;
videoBusy: boolean;
videoBlurEnabled: boolean;
showFooter: boolean;
@@ -122,7 +124,9 @@ export const CallFooter: FC<FooterProps> = ({ ref, children, vm }) => {
const setLayoutMode = useBehavior(vm.setLayoutMode$);
const openSettings = useBehavior(vm.openSettings$);
const audioEnabled = useBehavior(vm.audioEnabled$);
const audioBusy = useBehavior(vm.audioBusy$);
const videoEnabled = useBehavior(vm.videoEnabled$);
const videoBusy = useBehavior(vm.videoBusy$);
const toggleAudio = useBehavior(vm.toggleAudio$);
const toggleVideo = useBehavior(vm.toggleVideo$);
const sharingScreen = useBehavior(vm.sharingScreen$);
@@ -167,6 +171,7 @@ export const CallFooter: FC<FooterProps> = ({ ref, children, vm }) => {
key="audio"
iconsAndLabels="audio"
enabled={audioEnabled ?? false}
busy={audioBusy ?? false}
onMuteClick={toggleAudio}
data-testid="incall_mute"
options={audioOptions}
@@ -180,8 +185,9 @@ export const CallFooter: FC<FooterProps> = ({ ref, children, vm }) => {
size={buttonSize}
key="audio"
enabled={audioEnabled ?? false}
busy={audioBusy ?? false}
onClick={toggleAudio}
disabled={toggleAudio === undefined}
disabled={(audioBusy ?? false) || toggleAudio === undefined}
data-testid="incall_mute"
/>,
);
@@ -194,6 +200,7 @@ export const CallFooter: FC<FooterProps> = ({ ref, children, vm }) => {
key="video"
iconsAndLabels="video"
enabled={videoEnabled ?? false}
busy={videoBusy ?? false}
onMuteClick={toggleVideo}
options={videoOptions}
selectedOption={selectedVideo}
@@ -208,8 +215,9 @@ export const CallFooter: FC<FooterProps> = ({ ref, children, vm }) => {
size={buttonSize}
key="video"
enabled={videoEnabled ?? false}
busy={videoBusy ?? false}
onClick={toggleVideo}
disabled={toggleVideo === undefined}
disabled={(videoBusy ?? false) || toggleVideo === undefined}
data-testid="incall_videomute"
/>,
);

View File

@@ -32,14 +32,21 @@ function buildMuteBehaviors(
muteStates: MuteStates,
): Pick<
ViewModel<FooterSnapshot>,
"audioEnabled$" | "toggleAudio$" | "videoEnabled$" | "toggleVideo$"
| "audioEnabled$"
| "audioBusy$"
| "toggleAudio$"
| "videoEnabled$"
| "videoBusy$"
| "toggleVideo$"
> {
return {
audioEnabled$: muteStates.audio.enabled$,
audioBusy$: muteStates.audio.syncing$,
toggleAudio$: scope.behavior(
muteStates.audio.toggle$.pipe(map((t) => t ?? undefined)),
),
videoEnabled$: muteStates.video.enabled$,
videoBusy$: muteStates.video.syncing$,
toggleVideo$: scope.behavior(
muteStates.video.toggle$.pipe(map((t) => t ?? undefined)),
),
@@ -252,7 +259,9 @@ export function createLobbyFooterViewModel(
setLayoutMode: undefined,
toggleScreenSharing: undefined,
audioEnabled: undefined,
audioBusy: false,
videoEnabled: undefined,
videoBusy: false,
layoutMode: undefined,
sharingScreen: false,
audioOutputSwitcher: undefined,

View File

@@ -93,6 +93,48 @@ describe("MediaMuteAndSwitchButton", () => {
expect(onMute).toHaveBeenCalled();
});
test("disables mute button while busy", async () => {
const user = userEvent.setup();
const onMute = vi.fn();
const { getByRole } = renderComponent(
<MediaMuteAndSwitchButton
title={"Switcher"}
onMuteClick={onMute}
iconsAndLabels="audio"
enabled={true}
busy={true}
/>,
);
const muteButton = getByRole("switch", { name: "Mute microphone" });
expect(muteButton).toHaveAttribute("aria-disabled", "true");
expect(muteButton).toHaveAttribute("aria-busy", "true");
await user.click(muteButton);
expect(onMute).not.toHaveBeenCalled();
});
test("disables video button while busy", async () => {
const user = userEvent.setup();
const onMute = vi.fn();
const { getByRole } = renderComponent(
<MediaMuteAndSwitchButton
title={"Switcher"}
onMuteClick={onMute}
iconsAndLabels="video"
enabled={true}
busy={true}
/>,
);
const videoButton = getByRole("switch", { name: "Stop video" });
expect(videoButton).toHaveAttribute("aria-disabled", "true");
expect(videoButton).toHaveAttribute("aria-busy", "true");
await user.click(videoButton);
expect(onMute).not.toHaveBeenCalled();
});
test("requests device names when opened", async () => {
const user = userEvent.setup();
const requestDeviceNames = vi.fn();

View File

@@ -40,6 +40,8 @@ export interface MediaMuteAndSwitchButtonProps {
enabled?: boolean;
/** Callback if the mute button is clicked */
onMuteClick?: () => void;
/** True while mute/unmute operation is syncing. */
busy?: boolean;
iconsAndLabels: "video" | "audio";
/** The options available for the media device selector modal */
options?: MenuOptions[];
@@ -59,6 +61,7 @@ const BLUR_ID = "blur";
export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
title,
enabled,
busy,
onMuteClick,
iconsAndLabels,
options,
@@ -69,6 +72,7 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
}) => {
const [plannedSelection, setPlannedSelection] = useState<string | null>(null);
const [menuOpen, setMenuOpen] = useState(false);
const isBusy = busy ?? false;
const { t } = useTranslation();
const devices = useMediaDevices();
@@ -83,12 +87,13 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
button = (
<VideoButton
enabled={enabled ?? false}
busy={isBusy}
onClick={(e) => {
onMuteClick?.();
e.preventDefault();
e.stopPropagation();
}}
disabled={onMuteClick === undefined}
disabled={isBusy || onMuteClick === undefined}
data-testid="incall_videomute"
/>
);
@@ -106,12 +111,13 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
button = (
<MicButton
enabled={enabled ?? false}
busy={isBusy}
onClick={(e) => {
onMuteClick?.();
e.preventDefault();
e.stopPropagation();
}}
disabled={onMuteClick === undefined}
disabled={isBusy || onMuteClick === undefined}
data-testid="incall_mute"
/>
);

View File

@@ -6,6 +6,7 @@ exports[`MediaMuteAndSwitchButton > renders 1`] = `
class="container"
>
<button
aria-busy="false"
aria-checked="false"
aria-disabled="true"
aria-labelledby="_r_0_"

View File

@@ -328,6 +328,7 @@ exports[`InCallView > rendering > renders 1`] = `
</svg>
</button>
<button
aria-busy="false"
aria-checked="false"
aria-disabled="true"
aria-labelledby="_r_i_"
@@ -352,6 +353,7 @@ exports[`InCallView > rendering > renders 1`] = `
</svg>
</button>
<button
aria-busy="false"
aria-checked="false"
aria-disabled="true"
aria-labelledby="_r_n_"

View File

@@ -304,6 +304,7 @@ exports[`LobbyView > renders with header and participant count 1`] = `
</svg>
</button>
<button
aria-busy="false"
aria-checked="false"
aria-disabled="true"
aria-labelledby="_r_g_"
@@ -328,6 +329,7 @@ exports[`LobbyView > renders with header and participant count 1`] = `
</svg>
</button>
<button
aria-busy="false"
aria-checked="false"
aria-disabled="true"
aria-labelledby="_r_l_"

View File

@@ -63,6 +63,7 @@ function createMockLocalTrack(source: Track.Source): LocalTrack {
function createMockMuteState(enabled$: BehaviorSubject<boolean>): {
enabled$: BehaviorSubject<boolean>;
syncing$: BehaviorSubject<boolean>;
setHandler: (h: (enabled: boolean) => void) => void;
unsetHandler: () => void;
} {
@@ -70,6 +71,7 @@ function createMockMuteState(enabled$: BehaviorSubject<boolean>): {
const ms = {
enabled$,
syncing$: new BehaviorSubject(false),
setHandler: vi.fn().mockImplementation((h: (enabled: boolean) => void) => {
currentHandler = h;
}),

View File

@@ -112,27 +112,49 @@ export class Publisher {
this.logger.info("Local track published", localTrackPublication);
const lkRoom = this.connection.livekitRoom;
if (!this.shouldPublish) {
this.logger.debug("Not publishing, pausing upstream");
this.pauseUpstreams(lkRoom, [localTrackPublication.source]).catch((e) => {
this.logger.error(`Failed to pause upstreams`, e);
});
}
// also check the mute state and apply it
if (localTrackPublication.source === Track.Source.Microphone) {
const enabled = this.muteStates.audio.enabled$.value;
lkRoom.localParticipant.setMicrophoneEnabled(enabled).catch((e) => {
this.logger.error(
`Failed to enable microphone track, enabled:${enabled}`,
e,
);
});
const muteState = this.muteStates.audio;
// skip this if a sync is in progress: enabled$ still reflects the old
// state while the handler is mid-flight, so the handler itself will apply
// the correct mute state once it completes.
if (!muteState.syncing$.value) {
const enabled = muteState.enabled$.value;
if (!enabled) {
this.logger.info(
"Local audio track just published but muted meanwhile, setting enabled to false",
);
lkRoom.localParticipant.setMicrophoneEnabled(false).catch((e) => {
this.logger.error(
`Failed to enable microphone track, enabled:${enabled}`,
e,
);
});
}
}
} else if (localTrackPublication.source === Track.Source.Camera) {
const enabled = this.muteStates.video.enabled$.value;
lkRoom.localParticipant.setCameraEnabled(enabled).catch((e) => {
this.logger.error(
`Failed to enable camera track, enabled:${enabled}`,
e,
);
});
const muteState = this.muteStates.video;
// skip this if a sync is in progress: enabled$ still reflects the old
// state while the handler is mid-flight, so the handler itself will apply
// the correct mute state once it completes.
if (!muteState.syncing$.value) {
const enabled = muteState.enabled$.value;
if (!enabled) {
this.logger.info(
"Local video track just published but muted meanwhile, setting enabled to false",
);
lkRoom.localParticipant.setCameraEnabled(false).catch((e) => {
this.logger.error(
`Failed to enable camera track, enabled:${enabled}`,
e,
);
});
}
}
}
}
/**

View File

@@ -92,6 +92,75 @@ describe("MuteState", () => {
await flushPromises();
expect(lastEnabled).toBe(true);
});
test("should ignore toggle while syncing but still process set requests", async () => {
const deviceStub = {
available$: constant(
new Map<string, DeviceLabel>([
["mic", { type: "name", name: "Microphone" }],
]),
),
selected$: constant({ id: "mic" }),
select(): void {},
} as unknown as MediaDevice<DeviceLabel, SelectedDevice>;
const muteState = new MuteState(
testScope,
deviceStub,
false,
constant(false),
);
const first = Promise.withResolvers<boolean>();
const second = Promise.withResolvers<boolean>();
const handler = vi
.fn<(desired: boolean) => Promise<boolean>>()
.mockImplementationOnce(async () => first.promise)
.mockImplementationOnce(async () => second.promise);
muteState.setHandler(handler);
const syncingValues: boolean[] = [];
muteState.syncing$.subscribe((syncing) => {
syncingValues.push(syncing);
});
let setEnabled: ((enabled: boolean) => void) | null = null;
muteState.setEnabled$.subscribe((setter) => {
setEnabled = setter;
});
let toggle: (() => void) | null = null;
muteState.toggle$.subscribe((toggleFn) => {
toggle = toggleFn;
});
await flushPromises();
// Start syncing by requesting unmute.
toggle!();
await flushPromises();
expect(handler).toHaveBeenCalledTimes(1);
expect(handler).toHaveBeenNthCalledWith(1, true);
// Toggle requests are ignored while syncing.
toggle!();
await flushPromises();
expect(handler).toHaveBeenCalledTimes(1);
// setEnabled still updates latest desired state while syncing (push-to-talk).
setEnabled!(false);
await flushPromises();
expect(handler).toHaveBeenCalledTimes(1);
first.resolve(true);
await flushPromises();
expect(handler).toHaveBeenCalledTimes(2);
expect(handler).toHaveBeenNthCalledWith(2, false);
second.resolve(false);
await flushPromises();
expect(syncingValues).toContain(true);
expect(syncingValues.at(-1)).toBe(false);
});
});
describe("MuteStates", () => {

View File

@@ -30,6 +30,7 @@ import { type Behavior, constant } from "./Behavior";
interface MuteStateData {
enabled$: Observable<boolean>;
syncing$: Observable<boolean>;
set: ((enabled: boolean) => void) | null;
toggle: (() => void) | null;
}
@@ -79,33 +80,40 @@ export class MuteState<Label, Selected> {
this.handler$.value(false).catch((err) => {
logger.error("MuteState-disable: handler error", err);
});
return { enabled$: of(false), set: null, toggle: null };
return {
enabled$: of(false),
syncing$: of(false),
set: null,
toggle: null,
};
}
// Assume the default value only once devices are actually connected
let enabled = this.enabledByDefault;
const set$ = new Subject<boolean>();
const toggle$ = new Subject<void>();
const syncing$ = new BehaviorSubject(false);
const desired$ = merge(set$, toggle$.pipe(map(() => !enabled)));
const enabled$ = new Observable<boolean>((subscriber) => {
subscriber.next(enabled);
let latestDesired = this.enabledByDefault;
let syncing = false;
const sync = async (): Promise<void> => {
if (enabled === latestDesired) syncing = false;
else {
if (enabled === latestDesired) {
syncing$.next(false);
} else {
const previouslyEnabled = enabled;
syncing$.next(true);
enabled = await firstValueFrom(
this.handler$.pipe(
switchMap(async (handler) => handler(latestDesired)),
),
);
if (enabled === previouslyEnabled) {
syncing = false;
syncing$.next(false);
} else {
subscriber.next(enabled);
syncing = true;
syncing$.next(true);
sync().catch((err) => {
// TODO: better error handling
logger.error("MuteState: handler error", err);
@@ -116,21 +124,28 @@ export class MuteState<Label, Selected> {
const s = desired$.subscribe((desired) => {
latestDesired = desired;
if (syncing === false) {
syncing = true;
if (syncing$.value === false) {
syncing$.next(true);
sync().catch((err) => {
// TODO: better error handling
logger.error("MuteState: handler error", err);
});
}
});
return (): void => s.unsubscribe();
return (): void => {
s.unsubscribe();
syncing$.complete();
};
});
return {
set: (enabled: boolean): void => set$.next(enabled),
toggle: (): void => toggle$.next(),
toggle: (): void => {
if (syncing$.value) return;
toggle$.next();
},
enabled$,
syncing$,
};
}),
),
@@ -147,6 +162,10 @@ export class MuteState<Label, Selected> {
this.data$.pipe(map(({ toggle }) => toggle)),
);
public readonly syncing$: Behavior<boolean> = this.scope.behavior(
this.data$.pipe(switchMap(({ syncing$ }) => syncing$)),
);
public constructor(
private readonly scope: ObservableScope,
private readonly device: MediaDevice<Label, Selected>,