diff --git a/src/components/MediaMuteAndSwitchButton.test.tsx b/src/components/MediaMuteAndSwitchButton.test.tsx index b11b02d55..9fd67dfdb 100644 --- a/src/components/MediaMuteAndSwitchButton.test.tsx +++ b/src/components/MediaMuteAndSwitchButton.test.tsx @@ -15,24 +15,16 @@ import { type RenderResult, } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { type JSX, useState, type ReactNode } from "react"; +import { Profiler, type JSX, useState, type ReactNode } from "react"; import { TooltipProvider } from "@vector-im/compound-web"; import { MediaMuteAndSwitchButton, type MenuOptions, } from "./MediaMuteAndSwitchButton"; -import { MediaDevicesContext, useMediaDevices } from "../MediaDevicesContext"; +import { MediaDevicesContext } from "../MediaDevicesContext"; import { type MediaDevices } from "../state/MediaDevices"; import { restoreAudioCapture, stubAudioCapture } from "../utils/test"; -import type * as MediaDevicesContextModule from "../MediaDevicesContext"; - -// The menu reads the devices on every render and the meter never does, so -// these calls count the menu's own renders. -vi.mock("../MediaDevicesContext", async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, useMediaDevices: vi.fn(actual.useMediaDevices) }; -}); interface RenderOptions { requestDeviceNames: () => void; @@ -488,15 +480,19 @@ describe("MediaMuteAndSwitchButton", () => { ).not.toHaveAttribute("aria-disabled", "true"); }); - test("redraws the meter and not the device rows around it", async () => { - // The level is held by the meter, so a moving level doesn't re-render the - // menu. + test("moves the level without re-rendering anything", async () => { + // The level is drawn into the DOM, so a moving level commits nothing. const capture = stubAudioCapture(); const user = userEvent.setup(); - const menuRenders = vi.mocked(useMediaDevices); + let commits = 0; const { getByRole } = renderComponent( - <> + { + commits++; + }} + > { selectedOutputOption="spk1" onSelectOutput={vi.fn()} /> - , + , ); await user.click(getByRole("button", { name: "Microphone" })); capture.grant(); + await vi.waitFor(() => expect(capture.contexts).toHaveLength(1)); const meter = await screen.findByRole("meter"); - const settled = menuRenders.mock.calls.length; + // The capture's arrival is one render: the idle level swapped for its own. + await act(async () => {}); + const settled = commits; // The meter smooths by elapsed time, so hand-driven frames need a clock. let elapsed = performance.now(); @@ -538,7 +537,7 @@ describe("MediaMuteAndSwitchButton", () => { clock.mockRestore(); expect(meter.getAttribute("aria-valuenow")).not.toBe("0"); - expect(menuRenders.mock.calls.length - settled).toBe(0); + expect(commits - settled).toBe(0); }); test("camera menu uses the same selection pattern and keeps the blur toggle", async () => { diff --git a/src/components/MicrophoneLevelMeter.stories.tsx b/src/components/MicrophoneLevelMeter.stories.tsx index 423472bc9..19aaa34dc 100644 --- a/src/components/MicrophoneLevelMeter.stories.tsx +++ b/src/components/MicrophoneLevelMeter.stories.tsx @@ -15,6 +15,7 @@ import { } from "./MicrophoneLevelMeter"; import styles from "./MicrophoneLevelMeter.module.css"; import { LEVEL_SCALE } from "../state/MicrophoneLevel"; +import { constant } from "../state/Behavior"; /** Roughly the menu's width. It only decides how many bars fit. */ const STORY_WIDTH = 256; @@ -41,18 +42,18 @@ type Story = StoryObj; /** A quiet room: hiss below the noise floor lights nothing. */ export const Silent: Story = { - args: { state: { type: "level", level: 0 } }, + args: { state: { type: "level", level: constant(0) } }, play: async ({ canvasElement }) => { await expect(litSegments(canvasElement)).toBe(0); }, }; export const QuietSpeech: Story = { - args: { state: { type: "level", level: 5 } }, + args: { state: { type: "level", level: constant(5) } }, }; export const NormalSpeech: Story = { - args: { state: { type: "level", level: 12 } }, + args: { state: { type: "level", level: constant(12) } }, play: async ({ canvasElement }) => { // A floor, not a count: the count follows from the design's bar and gap sizes. await expect( @@ -62,16 +63,20 @@ export const NormalSpeech: Story = { }; export const LoudSpeech: Story = { - args: { state: { type: "level", level: LEVEL_SCALE } }, + args: { state: { type: "level", level: constant(LEVEL_SCALE) } }, }; /** The three volumes differ in how many bars are lit, not only in colour. */ export const VolumesAreDistinguishable: Story = { - args: { state: { type: "level", level: 5 } }, + args: { state: { type: "level", level: constant(5) } }, play: async ({ canvasElement, mount }) => { const lit: number[] = []; for (const level of [5, 12, LEVEL_SCALE]) { - await mount(); + await mount( + , + ); lit.push(litSegments(canvasElement)); await expect(within(canvasElement).getByRole("meter")).toHaveAttribute( "aria-valuenow", @@ -116,7 +121,7 @@ export const NoDevice: Story = { /** The same meter at two widths: the bars keep their size and only their count changes. */ export const ShapeStaysTheSameAtAnyWidth: Story = { - args: { state: { type: "level", level: 12 } }, + args: { state: { type: "level", level: constant(12) } }, play: async ({ mount, args }) => { const narrow = await measureAt(mount, args, 180); const wide = await measureAt(mount, args, 400); diff --git a/src/components/MicrophoneLevelMeter.test.tsx b/src/components/MicrophoneLevelMeter.test.tsx index 8723791ae..3e4960540 100644 --- a/src/components/MicrophoneLevelMeter.test.tsx +++ b/src/components/MicrophoneLevelMeter.test.tsx @@ -10,10 +10,13 @@ import { render, screen } from "@testing-library/react"; import { MicrophoneLevelMeter } from "./MicrophoneLevelMeter"; import { LEVEL_SCALE } from "../state/MicrophoneLevel"; +import { constant } from "../state/Behavior"; describe("MicrophoneLevelMeter", () => { test("announces the level rather than relying on hue", () => { - render(); + render( + , + ); const meter = screen.getByRole("meter", { name: "Microphone level" }); expect(meter).toHaveAttribute("aria-valuenow", "6"); diff --git a/src/components/MicrophoneLevelMeter.tsx b/src/components/MicrophoneLevelMeter.tsx index 0382aa55f..fc274a34c 100644 --- a/src/components/MicrophoneLevelMeter.tsx +++ b/src/components/MicrophoneLevelMeter.tsx @@ -5,7 +5,14 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { useCallback, useState, type FC, type Ref } from "react"; +import { + useCallback, + useLayoutEffect, + useRef, + useState, + type FC, + type Ref, +} from "react"; import { Text } from "@vector-im/compound-web"; import { MicOnIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; import classNames from "classnames"; @@ -57,20 +64,45 @@ export const MicrophoneLevelMeter: FC = ({ // Bars keep one size, so their count follows the width. Starts full so the // first paint, and jsdom, draw a whole meter. const [barCount, setBarCount] = useState(LEVEL_SCALE); + const segments = useRef(null); const track = useCallback( (element: HTMLDivElement | null): (() => void) | undefined => { if (element === null) return; + segments.current = element; const subscription = observeElementSize$(element) .pipe( map(({ width }) => barsThatFit(element, width)), distinctUntilChanged(), ) .subscribe(setBarCount); - return (): void => subscription.unsubscribe(); + return (): void => { + subscription.unsubscribe(); + segments.current = null; + }; }, [], ); + // Drawn straight into the DOM: the level changes many times a second, and + // re-rendering for each change is what this avoids. + useLayoutEffect(() => { + const element = segments.current; + if (state.type !== "level" || element === null) return; + const subscription = state.level.subscribe((level) => { + element.setAttribute("aria-valuenow", String(level)); + element.setAttribute( + "aria-valuetext", + t("microphone_level.value", { level, max: LEVEL_SCALE }), + ); + // The level is a share of the scale, not a bar count. + const lit = Math.round((level / LEVEL_SCALE) * barCount); + Array.from(element.children).forEach((bar, i) => + bar.classList.toggle(styles.segmentLit, i < lit), + ); + }); + return (): void => subscription.unsubscribe(); + }, [state, barCount, t]); + return (
@@ -88,22 +120,15 @@ export const MicrophoneLevelMeter: FC = ({ aria-label={t("microphone_level.label")} aria-valuemin={0} aria-valuemax={LEVEL_SCALE} - aria-valuenow={state.level} + // The first paint's value; the effect above keeps it current. + aria-valuenow={state.level.value} aria-valuetext={t("microphone_level.value", { - level: state.level, + level: state.level.value, max: LEVEL_SCALE, })} > {Array.from({ length: barCount }, (_, i) => ( - + ))}
)} diff --git a/src/components/useMicrophoneLevel.test.tsx b/src/components/useMicrophoneLevel.test.tsx index 2c651457a..cef896a78 100644 --- a/src/components/useMicrophoneLevel.test.tsx +++ b/src/components/useMicrophoneLevel.test.tsx @@ -35,6 +35,7 @@ describe("useMicrophoneLevel", () => { rerender({ id: "mic2" }); // No level carried over from the previous device. - expect(result.current).toEqual({ type: "level", level: 0 }); + const state = result.current; + expect(state.type === "level" ? state.level.value : state.type).toBe(0); }); }); diff --git a/src/components/useMicrophoneLevel.ts b/src/components/useMicrophoneLevel.ts index f357744db..b2c17e012 100644 --- a/src/components/useMicrophoneLevel.ts +++ b/src/components/useMicrophoneLevel.ts @@ -11,8 +11,9 @@ import { type MicrophoneState, observeMicrophoneState$, } from "../state/MicrophoneLevel"; +import { constant } from "../state/Behavior"; -const IDLE: MicrophoneState = { type: "level", level: 0 }; +const IDLE: MicrophoneState = { type: "level", level: constant(0) }; /** The live level of a microphone, captured only while `active`. */ export function useMicrophoneLevel( diff --git a/src/state/MicrophoneLevel.test.ts b/src/state/MicrophoneLevel.test.ts index b99988f81..69d7c8a7b 100644 --- a/src/state/MicrophoneLevel.test.ts +++ b/src/state/MicrophoneLevel.test.ts @@ -157,16 +157,20 @@ describe("observeMicrophoneState$", () => { test("says nothing on a frame that did not change the level", async () => { const capture = stubAudioCapture(); - let emissions = 0; - const subscription = observeMicrophoneState$("mic1").subscribe( - () => emissions++, - ); + const states: string[] = []; + let levels = 0; + const subscription = observeMicrophoneState$("mic1").subscribe((state) => { + states.push(state.type); + if (state.type === "level") state.level.subscribe(() => levels++); + }); capture.grant(); - await vi.waitFor(() => expect(emissions).toBe(1)); + await vi.waitFor(() => expect(levels).toBe(1)); - // Read every frame, but a steady signal emits once. + // Read every frame, but a steady signal emits once, and the state itself + // arrives only once. capture.drawFrames(20); - expect(emissions).toBe(1); + expect(levels).toBe(1); + expect(states).toEqual(["level"]); subscription.unsubscribe(); }); diff --git a/src/state/MicrophoneLevel.ts b/src/state/MicrophoneLevel.ts index 8e88a5042..f34fcca2b 100644 --- a/src/state/MicrophoneLevel.ts +++ b/src/state/MicrophoneLevel.ts @@ -5,12 +5,15 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { distinctUntilChanged, Observable } from "rxjs"; +import { BehaviorSubject, Observable } from "rxjs"; import { logger } from "matrix-js-sdk/lib/logger"; +import { type Behavior } from "./Behavior"; + /** What the microphone picks up, or why it can't be read. */ export type MicrophoneState = - | { type: "level"; level: number } + // A Behavior, so a changing level can be drawn without re-rendering. + | { type: "level"; level: Behavior } | { type: "permission-denied" } | { type: "no-device" }; @@ -29,6 +32,7 @@ export function observeMicrophoneState$( let stream: MediaStream | undefined; let context: AudioContext | undefined; let frame: number | undefined; + let level: BehaviorSubject | undefined; // Idempotent: teardown and start can both call it. const release = (): void => { @@ -38,6 +42,8 @@ export function observeMicrophoneState$( frame = undefined; stream = undefined; context = undefined; + level?.complete(); + level = undefined; }; const start = async (): Promise => { @@ -59,6 +65,9 @@ export function observeMicrophoneState$( const samples = new Uint8Array(analyser.fftSize); let displayed = 0; let previousFrame = performance.now(); + const current = new BehaviorSubject(0); + level = current; + subscriber.next({ type: "level", level: current }); const read = (): void => { analyser.getByteTimeDomainData(samples); @@ -75,7 +84,9 @@ export function observeMicrophoneState$( now - previousFrame, ); previousFrame = now; - subscriber.next({ type: "level", level: segmentsForVolume(displayed) }); + // Frames that don't move the quantised level say nothing. + const next = segmentsForVolume(displayed); + if (next !== current.value) current.next(next); frame = requestAnimationFrame(read); }; read(); @@ -88,14 +99,7 @@ export function observeMicrophoneState$( }); return release; - }).pipe( - // Frames that don't move the quantised level don't reach React. - distinctUntilChanged( - (a, b) => - a.type === b.type && - (a.type !== "level" || b.type !== "level" || a.level === b.level), - ), - ); + }); } /** What a failure to open the microphone means for the person using it. */