mirror of
https://github.com/vector-im/element-call.git
synced 2026-06-06 11:45:53 +00:00
feat(mute): add syncing state and disable toggle during async mute
This commit is contained in:
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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>
|
||||
);
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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"
|
||||
/>,
|
||||
);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -93,6 +93,27 @@ 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("requests device names when opened", async () => {
|
||||
const user = userEvent.setup();
|
||||
const requestDeviceNames = vi.fn();
|
||||
|
||||
@@ -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,14 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
|
||||
button = (
|
||||
<VideoButton
|
||||
enabled={enabled ?? false}
|
||||
busy={isBusy}
|
||||
onClick={(e) => {
|
||||
if (isBusy) return;
|
||||
onMuteClick?.();
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
}}
|
||||
disabled={onMuteClick === undefined}
|
||||
disabled={isBusy || onMuteClick === undefined}
|
||||
data-testid="incall_videomute"
|
||||
/>
|
||||
);
|
||||
@@ -106,12 +112,14 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
|
||||
button = (
|
||||
<MicButton
|
||||
enabled={enabled ?? false}
|
||||
busy={isBusy}
|
||||
onClick={(e) => {
|
||||
if (isBusy) return;
|
||||
onMuteClick?.();
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
}}
|
||||
disabled={onMuteClick === undefined}
|
||||
disabled={isBusy || onMuteClick === undefined}
|
||||
data-testid="incall_mute"
|
||||
/>
|
||||
);
|
||||
|
||||
@@ -6,6 +6,7 @@ exports[`MediaMuteAndSwitchButton > renders 1`] = `
|
||||
class="container"
|
||||
>
|
||||
<button
|
||||
aria-busy="false"
|
||||
aria-checked="false"
|
||||
aria-disabled="true"
|
||||
aria-labelledby="_r_0_"
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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>,
|
||||
|
||||
Reference in New Issue
Block a user