From 52d33c8ebba1f11f72dbe4f1235e8dcbb1b7e403 Mon Sep 17 00:00:00 2001 From: Jason Buckner Date: Mon, 14 Sep 2026 17:20:01 -0700 Subject: [PATCH] WEBDEV-9046: Address the ia-playback-controls review follow-ups Four items split out of the WEBDEV-8965 review so they didn't hold up the radio player release. Nothing covered the `color: inherit` rule on .unstyled-button. Buttons don't inherit colour from their parent, so without it the container's themed colour never reaches the icons and their currentColor falls back to the UA default. There's a test for that now, driven through the public theme variable. It skips the play/pause button on purpose, since that one sets its own colour and would pass either way. The decorative icons are hidden from assistive tech. Every button carries its own aria-label, and replay and skip-ahead each contain a 10 that gives the SVG accessible content of its own. All ten get the attribute, matching what ia-expandable-search-bar and ia-radio-player already do. The test walks the element through playing and the two quieter volume states as well, because pause, volume-medium and volume-mute never render otherwise, and it asserts which buttons it covered so a future icon can't slip past unchecked. MAX_PLAYBACK_RATE is documented. The one comment above the pair only described MIN_PLAYBACK_RATE. The playback rate goes through Intl.NumberFormat, in its own module alongside the one ia-transcript-view already has for durations. It was interpolated as a raw JS number, so it always formatted with a decimal point whatever the reader's locale. The test uses a third rather than a quarter step: formatting stops at three fraction digits in every locale, while the raw value spells out all sixteen, so the assertion tells the two apart without depending on the locale the tests run in. That formatter is also where an unusable rate now stops. `playbackRate` is public, so `NaN` could reach the display and the screen reader as "Playback speed, currently NaN". Rates outside the range the button steps through are left alone: a media element will still play at those. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DaEr5YT1xpjK5ydNZPvjwx --- .../assets/next-section.ts | 9 +- .../ia-playback-controls/assets/pause.ts | 9 +- .../ia-playback-controls/assets/play.ts | 9 +- .../assets/playback-speed.ts | 9 +- .../assets/previous-section.ts | 9 +- .../ia-playback-controls/assets/replay.ts | 9 +- .../ia-playback-controls/assets/skip-ahead.ts | 9 +- .../assets/volume-full.ts | 9 +- .../assets/volume-medium.ts | 9 +- .../assets/volume-mute.ts | 9 +- .../ia-playback-controls.test.ts | 94 +++++++++++++++++++ .../ia-playback-controls.ts | 38 +++++++- .../playback-rate-formatter.test.ts | 16 ++++ .../playback-rate-formatter.ts | 10 ++ src/elements/index.ts | 1 + 15 files changed, 215 insertions(+), 34 deletions(-) create mode 100644 src/elements/ia-playback-controls/playback-rate-formatter.test.ts create mode 100644 src/elements/ia-playback-controls/playback-rate-formatter.ts diff --git a/src/elements/ia-playback-controls/assets/next-section.ts b/src/elements/ia-playback-controls/assets/next-section.ts index 8e935ac5..2521dc00 100644 --- a/src/elements/ia-playback-controls/assets/next-section.ts +++ b/src/elements/ia-playback-controls/assets/next-section.ts @@ -1,13 +1,16 @@ /** * @file Jump to next section icon * - * Inline so the button can recolor it with `currentColor`, which is not - * possible on the contents of an ``. + * Decorative: the button around it carries the accessible name, so the icon + * is hidden from assistive tech. Inline so the button can recolor it with + * `currentColor`, which is not possible on the contents of an ``. */ import { svg } from 'lit'; export default svg` - +`. + * Decorative: the button around it carries the accessible name, so the icon + * is hidden from assistive tech. Inline so the button can recolor it with + * `currentColor`, which is not possible on the contents of an ``. */ import { svg } from 'lit'; export default svg` - +`. */ import { svg } from 'lit'; export default svg` - +`. */ import { svg } from 'lit'; export default svg` - +`. + * Decorative: the button around it carries the accessible name, so the icon + * is hidden from assistive tech. Inline so the button can recolor it with + * `currentColor`, which is not possible on the contents of an ``. */ import { svg } from 'lit'; export default svg` - +`. + * Decorative: the button around it carries the accessible name, so the icon + * is hidden from assistive tech. Inline so the button can recolor it with + * `currentColor`, which is not possible on the contents of an ``. */ import { svg } from 'lit'; export default svg` - +`. */ import { svg } from 'lit'; export default svg` - +`. */ import { svg } from 'lit'; export default svg` - +`. */ import { svg } from 'lit'; export default svg` - +`. */ import { svg } from 'lit'; export default svg` - + @@ -174,7 +186,25 @@ export class IAPlaybackControls extends LitElement { * label beside the button carries the "x". */ private get playbackRateLabel(): string { - return msg(str`Playback speed, currently ${this.playbackRate}`); + return msg(str`Playback speed, currently ${this.formattedPlaybackRate}`); + } + + /** + * The playback rate written for the reader's locale, so the quarter steps + * use whatever decimal separator they expect rather than always a point. + * + * `playbackRate` is a public property, so it can arrive as something that + * isn't a usable number. Everything that displays or announces the rate + * reads it through here, so `NaN` never reaches the screen reader. Rates + * outside the range the button steps through are shown as they are: they + * are still rates a media element will play at. + */ + private get formattedPlaybackRate(): string { + const rate = Number.isFinite(this.playbackRate) + ? this.playbackRate + : DEFAULT_PLAYBACK_RATE; + + return formatPlaybackRate(rate); } /** diff --git a/src/elements/ia-playback-controls/playback-rate-formatter.test.ts b/src/elements/ia-playback-controls/playback-rate-formatter.test.ts new file mode 100644 index 00000000..486ec420 --- /dev/null +++ b/src/elements/ia-playback-controls/playback-rate-formatter.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, test } from 'vitest'; + +import { formatPlaybackRate } from './playback-rate-formatter'; + +describe('formatPlaybackRate', () => { + test('uses the locale’s decimal separator', () => { + expect(formatPlaybackRate(1.5, 'en-US')).to.equal('1.5'); + expect(formatPlaybackRate(1.5, 'de-DE')).to.equal('1,5'); + expect(formatPlaybackRate(0.75, 'de-DE')).to.equal('0,75'); + }); + + test('leaves whole rates without a decimal part', () => { + expect(formatPlaybackRate(1, 'en-US')).to.equal('1'); + expect(formatPlaybackRate(2, 'de-DE')).to.equal('2'); + }); +}); diff --git a/src/elements/ia-playback-controls/playback-rate-formatter.ts b/src/elements/ia-playback-controls/playback-rate-formatter.ts new file mode 100644 index 00000000..73dd9fe1 --- /dev/null +++ b/src/elements/ia-playback-controls/playback-rate-formatter.ts @@ -0,0 +1,10 @@ +/** + * Formats a playback rate for display. + * + * The rate moves in quarter steps, and the decimal separator for those varies + * by locale, so the number goes through `Intl` rather than straight into a + * template. Omitting the locale uses the reader's own. + */ +export function formatPlaybackRate(rate: number, locale?: string): string { + return new Intl.NumberFormat(locale).format(rate); +} diff --git a/src/elements/index.ts b/src/elements/index.ts index 16ee6fe6..e9c7be6e 100644 --- a/src/elements/index.ts +++ b/src/elements/index.ts @@ -13,6 +13,7 @@ export * from './ia-item-navigator/menus/ia-itemnav-sort-files-button'; export * from './ia-item-navigator/menus/ia-itemnav-share-panel'; export * from './ia-playback-controls/ia-playback-controls'; export * from './ia-playback-controls/models'; +export * from './ia-playback-controls/playback-rate-formatter'; export * from './ia-radio-player/ia-radio-player'; export * from './ia-radio-player/ia-search-results-switcher'; export * from './ia-radio-player/models';