Make the volume slider less silly

Previously, dragging it all the way to the left would *not* mute the participant but rather bottom out at 10% volume, and people have found this unintuitive. Let's make it less silly by giving the slider a range of 0% to 100%, and making the mute toggle button have the same effect as dragging the slider to zero. When unmuting, it will reset to the last non-zero "committed" volume, similar to how the volume sliders in desktop environments work.
This commit is contained in:
Robin
2024-10-18 17:51:37 -04:00
parent 75c7516f0a
commit 0c6e53cda4
4 changed files with 85 additions and 41 deletions

View File

@@ -16,6 +16,7 @@ interface Props {
label: string;
value: number;
onValueChange: (value: number) => void;
onValueCommit?: (value: number) => void;
min: number;
max: number;
step: number;
@@ -30,6 +31,7 @@ export const Slider: FC<Props> = ({
label,
value,
onValueChange: onValueChangeProp,
onValueCommit: onValueCommitProp,
min,
max,
step,
@@ -39,12 +41,17 @@ export const Slider: FC<Props> = ({
([v]: number[]) => onValueChangeProp(v),
[onValueChangeProp],
);
const onValueCommit = useCallback(
([v]: number[]) => onValueCommitProp?.(v),
[onValueCommitProp],
);
return (
<Root
className={classNames(className, styles.slider)}
value={[value]}
onValueChange={onValueChange}
onValueCommit={onValueCommit}
min={min}
max={max}
step={step}

View File

@@ -13,43 +13,47 @@ import {
withTestScheduler,
} from "../utils/test";
test("set a participant's volume", async () => {
test("control a participant's volume", async () => {
const setVolumeSpy = vi.fn();
await withRemoteMedia({}, { setVolume: setVolumeSpy }, (vm) =>
withTestScheduler(({ expectObservable, schedule }) => {
schedule("-a|", {
a() {
vm.setLocalVolume(0.8);
expect(setVolumeSpy).toHaveBeenLastCalledWith(0.8);
},
});
expectObservable(vm.localVolume).toBe("ab", { a: 1, b: 0.8 });
}),
);
});
test("mute and unmute a participant", async () => {
const setVolumeSpy = vi.fn();
await withRemoteMedia({}, { setVolume: setVolumeSpy }, (vm) =>
withTestScheduler(({ expectObservable, schedule }) => {
schedule("-abc|", {
schedule("-ab---c---d|", {
a() {
// Try muting by toggling
vm.toggleLocallyMuted();
expect(setVolumeSpy).toHaveBeenLastCalledWith(0);
},
b() {
// Try unmuting by dragging the slider back up
vm.setLocalVolume(0.6);
vm.setLocalVolume(0.8);
expect(setVolumeSpy).toHaveBeenLastCalledWith(0);
vm.commitLocalVolume();
expect(setVolumeSpy).toHaveBeenCalledWith(0.6);
expect(setVolumeSpy).toHaveBeenLastCalledWith(0.8);
},
c() {
// Try muting by dragging the slider back down
vm.setLocalVolume(0.2);
vm.setLocalVolume(0);
vm.commitLocalVolume();
expect(setVolumeSpy).toHaveBeenCalledWith(0.2);
expect(setVolumeSpy).toHaveBeenLastCalledWith(0);
},
d() {
// Try unmuting by toggling
vm.toggleLocallyMuted();
// The volume should return to the last non-zero committed volume
expect(setVolumeSpy).toHaveBeenLastCalledWith(0.8);
},
});
expectObservable(vm.locallyMuted).toBe("ab-c", {
a: false,
b: true,
c: false,
expectObservable(vm.localVolume).toBe("ab(cd)(ef)g", {
a: 1,
b: 0,
c: 0.6,
d: 0.8,
e: 0.2,
f: 0,
g: 0.8,
});
}),
);

View File

@@ -26,10 +26,12 @@ import { RoomMember, RoomMemberEvent } from "matrix-js-sdk/src/matrix";
import {
BehaviorSubject,
Observable,
Subject,
combineLatest,
distinctUntilKeyChanged,
fromEvent,
map,
merge,
of,
startWith,
switchMap,
@@ -39,6 +41,7 @@ import { useEffect } from "react";
import { ViewModel } from "./ViewModel";
import { useReactiveState } from "../useReactiveState";
import { alwaysShowSelf } from "../settings/settings";
import { accumulate } from "../utils/observable";
// TODO: Move this naming logic into the view model
export function useDisplayName(vm: MediaViewModel): string {
@@ -232,18 +235,45 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel {
* A remote participant's user media.
*/
export class RemoteUserMediaViewModel extends BaseUserMediaViewModel {
private readonly _locallyMuted = new BehaviorSubject(false);
/**
* Whether we've disabled this participant's audio.
*/
public readonly locallyMuted: Observable<boolean> = this._locallyMuted;
private readonly locallyMutedToggle = new Subject<void>();
private readonly localVolumeAdjustment = new Subject<number>();
private readonly localVolumeCommit = new Subject<void>();
private readonly _localVolume = new BehaviorSubject(1);
/**
* The volume to which we've set this participant's audio, as a scalar
* The volume to which this participant's audio is set, as a scalar
* multiplier.
*/
public readonly localVolume: Observable<number> = this._localVolume;
public readonly localVolume: Observable<number> = merge(
this.locallyMutedToggle.pipe(map(() => "toggle mute" as const)),
this.localVolumeAdjustment,
this.localVolumeCommit.pipe(map(() => "commit" as const)),
).pipe(
accumulate(
{ muted: false, volume: 1, committedVolume: 1 },
(state, event) =>
event === "toggle mute"
? { ...state, muted: !state.muted }
: event === "commit"
? { ...state, committedVolume: state.volume }
: // Volume adjustment
event === 0
? // Dragging the slider to zero should have the same effect as
// muting: reset the volume to the committed volume, as if it were
// never dragged
{ ...state, muted: true, volume: state.committedVolume }
: { ...state, muted: false, volume: event },
),
map(({ muted, volume }) => (muted ? 0 : volume)),
this.scope.state(),
);
/**
* Whether this participant's audio is disabled.
*/
public readonly locallyMuted: Observable<boolean> = this.localVolume.pipe(
map((volume) => volume === 0),
this.scope.state(),
);
public constructor(
id: string,
@@ -253,22 +283,24 @@ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel {
) {
super(id, member, participant, callEncrypted);
// Sync the local mute state and volume with LiveKit
combineLatest([this._locallyMuted, this._localVolume], (muted, volume) =>
muted ? 0 : volume,
)
// Sync the local volume with LiveKit
this.localVolume
.pipe(this.scope.bind())
.subscribe((volume) => {
(this.participant as RemoteParticipant).setVolume(volume);
});
.subscribe((volume) =>
(this.participant as RemoteParticipant).setVolume(volume),
);
}
public toggleLocallyMuted(): void {
this._locallyMuted.next(!this._locallyMuted.value);
this.locallyMutedToggle.next();
}
public setLocalVolume(value: number): void {
this._localVolume.next(value);
this.localVolumeAdjustment.next(value);
}
public commitLocalVolume(): void {
this.localVolumeCommit.next();
}
}

View File

@@ -227,6 +227,7 @@ const RemoteUserMediaTile = forwardRef<
(v: number) => vm.setLocalVolume(v),
[vm],
);
const onCommitLocalVolume = useCallback(() => vm.commitLocalVolume(), [vm]);
const VolumeIcon = locallyMuted ? VolumeOffIcon : VolumeOnIcon;
@@ -250,10 +251,10 @@ const RemoteUserMediaTile = forwardRef<
label={t("video_tile.volume")}
value={localVolume}
onValueChange={onChangeLocalVolume}
min={0.1}
onValueCommit={onCommitLocalVolume}
min={0}
max={1}
step={0.01}
disabled={locallyMuted}
/>
</MenuItem>
</>