Repository navigation
fix(player): include visible playback rate in accessible name - #6032
Conversation
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed exact head 84012a0f4aead32cc9f63478f608a58bbd4b6bec for #6021. No blocking source finding.
packages/player/src/controls.ts:104, :276, and :378 now keep the visible rate in the button's accessible name at initialization, menu selection and API updates. I traced the playback-rate attribute/property consumer into updateSpeed; existing callbacks and menu behavior remain intact.
Independent verification using immutable source copies and the existing environment: all four controls tests passed; a separate DOM probe passed seven initial/menu/API rate states (including non-preset 0.75x), callback and teardown checks. The same probe fails against the parent source's static label, so it discriminates the reported defect. This verifies the DOM accessible-label inputs, not an actual voice-control or platform accessibility-tree session.
At the 14:32Z exact-head check, Lint, Format, Build, Typecheck, support/runtime-contract tests and several regression/performance jobs passed; aggregate Test and other jobs remain pending. Studio and player captures failed: its actual log rejects the missing Before/After headings and permits a ## No visible change declaration for qualifying small nonvisual changes. This diff changes only aria-label attributes (under 20 production lines, no visual-file changes), so that truthful body declaration is the appropriate repair; rendered text remains unchanged. Please correct the body and obtain a green check. I am withholding formal approval while that gate is red.
The author's targeted build/type/lint/format claims remain attributed. Their broad workspace failures/interrupted checks are disclosed and are not treated as passing or proven upstream failures. Normal repository checks remain independent merge gates.
— Magi / desktop-review
Verdict: COMMENT
Reasoning: Source and independent controls support the fix, but the current capture-evidence check is failing and needs the truthful nonvisual declaration before approval.
miguel-heygen
left a comment
There was a problem hiding this comment.
Thanks for the fix, and welcome. I checked every writer of the button text in controls.ts (creation, the menu click handler, and updateSpeed) and all three now set the label from the same formatSpeed call, so the name cannot drift from the visible rate. The test drives both the menu click and the API update through createControls, which is the real entry point.
Two small notes, neither blocking. Studio has its own speed button with the same problem, in packages/studio/src/player/components/SpeedMenu.tsx, if you want a follow up. And a tiny setSpeedLabel helper writing text and label together would keep a future fourth writer from updating one and forgetting the other.
jrusso1020
left a comment
There was a problem hiding this comment.
Approving at 84012a0. Main's player speed button has the static label Playback speed, which hides the visible rate from voice-control users, so #6021 is real.
I checked every place the button text is written in controls.ts at this head: creation (:103), the menu click handler (:275) and updateSpeed (:377). Each now sets the label from the same formatSpeed value, so the name and the text can't drift. Nothing else in the repo matches on the old "Playback speed" label for this button (the Studio matches are its own separate SpeedMenu). The new test asserts Playback speed: 1x at creation, so it fails on main, and it drives both the menu click and the API path through createControls.
Adding to Miguel's approval, not repeating it: his follow-up note on Studio's SpeedMenu.tsx having the same static label still stands.
Need: yes · Size: ok · Reuse: uses the existing formatSpeed · Simpler: already minimal
— Rames
What
Include the visible playback rate in the player speed button's accessible name, initially and after menu or API speed changes.
Why
The static
Playback speedlabel overrides the visible1xtext. Voice-control users need the visible rate to remain part of the accessible name as it changes.Related work
Fixes #6021.
Issue discussion describes the source mismatch and intended approach. Open speed-related PRs #5528 and #5300 concern exported media and CLI audio auditing rather than this control.
How
Set
Playback speed: 1xon initialization and useformatSpeedwhen updating the name in the menu-selection andupdateSpeedpaths. The visible text and existing callbacks remain unchanged.Test plan
Tested commit:
84012a0f4aead32cc9f63478f608a58bbd4b6bec.bun run --cwd packages/player test src/controls.test.ts --maxWorkers=1passed all 4 tests.Also passed: player typecheck (source and perf configuration); player build including declarations and runtime-pin verification; changed-file oxlint and oxfmt; comment citations; comment ratchet;
git diff --check.The full workspace checks did not pass locally.
bun run testhit missing AWS dependencies, Node shim cache permission errors, temporary-file errors and worker startup timeouts across packages; I interrupted the remaining run after these failures. The first isolated controls attempts also timed out starting workers; after stopping the broad processes, the single-worker run above passed. Workspace typecheck reported Node shim cache permission failures and its remaining packages were interrupted. Workspace lint and formatting checks were interrupted; both checks passed for the two changed files. These are local results, without a claim that the failures are reproduced on upstream main.No visible change
This change updates only the playback-speed button’s
aria-labelat initialization, menu selection, and API updates. The visible rate text, layout, and styling are unchanged. The production diff changes four lines incontrols.ts; the remaining changes are regression assertions in the existing test file.