Skip to content

fix(player): include visible playback rate in accessible name - #6032

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
Dante-dan:fix/6021-speed-accessible-name
Oct 24, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
Dante-dan:fix/6021-speed-accessible-name

Conversation

@Dante-dan

@Dante-dan Dante-dan commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

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 speed label overrides the visible 1x text. 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: 1x on initialization and use formatSpeed when updating the name in the menu-selection and updateSpeed paths. The visible text and existing callbacks remain unchanged.

Test plan

Tested commit: 84012a0f4aead32cc9f63478f608a58bbd4b6bec.

  • Unit tests added/updated: existing player controls test now checks initial 1x, a 1.5x menu selection, and a 2x API update. bun run --cwd packages/player test src/controls.test.ts --maxWorkers=1 passed all 4 tests.
  • Manual testing performed
  • Documentation updated (not applicable)
  • Comments follow CONTRIBUTING.md (no new comments).

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 test hit 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-label at initialization, menu selection, and API updates. The visible rate text, layout, and styling are unchanged. The production diff changes four lines in controls.ts; the remaining changes are regression assertions in the existing test file.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
Merged via the queue into heygen-com:main with commit 8b1bf6b Oct 24, 2026
109 of 110 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Player: speed button's accessible name doesn't include its visible text

3 participants