From 6c050f3175ec575cd965322384e76a6afe8f885a Mon Sep 17 00:00:00 2001 From: 4gray Date: Wed, 30 Sep 2026 22:07:22 +0200 Subject: [PATCH] fix(player): arrows check settings radios; subtitle chip reads On Review follow-up: - Arrow keys, Home and End now check the settings radio they reach, as a native radio group does (the directive clicks it, so the template's handler applies the choice); an option the engine already reports as checked is not applied again. - With subtitles on but no track marked selected yet (the engine can report the switch before the track list), the subtitle chip reads and announces "On" (new SUBTITLES_ON key, 19 locales) instead of "Off". - The swatch row has 4px padding on every side, so the outer focus ring is not clipped at the scroll edge of the panel body. Co-Authored-By: Claude Opus 5.5 --- .../src/player-controls-keyboard.e2e.ts | 60 ++++++++------- apps/web/src/assets/i18n/ar.json | 1 + apps/web/src/assets/i18n/ary.json | 1 + apps/web/src/assets/i18n/by.json | 1 + apps/web/src/assets/i18n/de.json | 1 + apps/web/src/assets/i18n/el.json | 1 + apps/web/src/assets/i18n/en.json | 1 + apps/web/src/assets/i18n/es.json | 1 + apps/web/src/assets/i18n/fr.json | 1 + apps/web/src/assets/i18n/hu.json | 1 + apps/web/src/assets/i18n/it.json | 1 + apps/web/src/assets/i18n/ja.json | 1 + apps/web/src/assets/i18n/ko.json | 1 + apps/web/src/assets/i18n/nl.json | 1 + apps/web/src/assets/i18n/pl.json | 1 + apps/web/src/assets/i18n/pt.json | 1 + apps/web/src/assets/i18n/ru.json | 1 + apps/web/src/assets/i18n/tr.json | 1 + apps/web/src/assets/i18n/zh.json | 1 + apps/web/src/assets/i18n/zhtw.json | 1 + docs/architecture/player-controls-contract.md | 19 +++-- .../player-controls.component.html | 11 ++- .../player-settings-panel.a11y.spec.ts | 73 +++++++++++++------ .../player-settings-panel.component.scss | 4 +- .../settings-radio-group.directive.ts | 20 +++-- 25 files changed, 140 insertions(+), 66 deletions(-) diff --git a/apps/web-e2e/src/player-controls-keyboard.e2e.ts b/apps/web-e2e/src/player-controls-keyboard.e2e.ts index aa6326723..15a441f36 100644 --- a/apps/web-e2e/src/player-controls-keyboard.e2e.ts +++ b/apps/web-e2e/src/player-controls-keyboard.e2e.ts @@ -247,38 +247,38 @@ test('@web @playback keyboard reaches the dock and the settings radios with a vi ]); expect(new Set(radioStops.map((stop) => stop.group)).size).toBe(3); - // --- Arrows move focus without applying; Space applies. ----------- + // --- Arrows move focus and apply the option they reach. ---------- const speedGroup = panel.locator( '[data-test-id="player-settings-speed"] [role="radiogroup"]' ); - const rateBefore = await video.evaluate( - (el: HTMLVideoElement) => el.playbackRate - ); + const rate = () => + video.evaluate((el: HTMLVideoElement) => el.playbackRate); await page.keyboard.press('ArrowRight'); const moved = await focusStop(page); expect(moved?.role).toBe('radio'); - expect(moved?.checked).toBe('false'); - expect( - await video.evaluate((el: HTMLVideoElement) => el.playbackRate) - ).toBe(rateBefore); - await page.keyboard.press('Space'); - await expect - .poll(() => - video.evaluate((el: HTMLVideoElement) => el.playbackRate) - ) - .toBe(Number.parseFloat(moved?.label ?? '')); + await expect.poll(rate).toBe(Number.parseFloat(moved?.label ?? '')); await expect( speedGroup.locator('[role="radio"][aria-checked="true"]') ).toHaveText(moved?.label ?? ''); await page.keyboard.press('Home'); expect((await focusStop(page))?.label).toBe('0.5×'); + await expect.poll(rate).toBe(0.5); + await expect( + speedGroup.locator('[role="radio"][aria-checked="true"]') + ).toHaveText('0.5×'); + // The ends wrap, as in a native radio group. await page.keyboard.press('ArrowLeft'); expect((await focusStop(page))?.label).toBe('2×'); + await expect.poll(rate).toBe(2); await expect(speedGroup.locator('[tabindex="0"]')).toHaveText('2×'); - // Leaving and re-entering lands on the checked option again. + // Leaving and re-entering lands on the checked option — the one the + // engine reports, so wait for it to confirm the switch first. + await expect( + speedGroup.locator('[role="radio"][aria-checked="true"]') + ).toHaveText('2×'); await pressTab(page, browserName, 'backward'); await pressTab(page, browserName); - expect((await focusStop(page))?.label).toBe(moved?.label); + expect((await focusStop(page))?.label).toBe('2×'); // --- A focused selected swatch differs from a selected one. ------- await tabUntil(page, browserName, { @@ -286,14 +286,24 @@ test('@web @playback keyboard reaches the dock and the settings radios with a vi direction: 'backward', max: 5, }); - const focusedSelected = panel.locator('.player-settings__swatch:focus'); - await expect(focusedSelected).toHaveAttribute('aria-checked', 'true'); - await expect(focusedSelected).toHaveCSS('outline-style', 'solid'); + const focusedSwatch = panel.locator('.player-settings__swatch:focus'); + await expect(focusedSwatch).toHaveAttribute('aria-checked', 'true'); + await expect(focusedSwatch).toHaveCSS('outline-style', 'solid'); + // The arrow checks the next swatch, which takes focus and the ring. await page.keyboard.press('ArrowRight'); + await expect(focusedSwatch).toHaveAttribute('aria-checked', 'true'); + await expect(focusedSwatch).toHaveCSS('outline-style', 'solid'); + // Tab moves on to the speed group: the checked swatch keeps its + // selected border but loses the ring. + await pressTab(page, browserName); + expect( + await focusIsInside(page, '[data-test-id="player-settings-speed"]') + ).toBe(true); const selectedOnly = panel.locator( - '.player-settings__swatch[aria-checked="true"]:not(:focus)' + '.player-settings__swatch[aria-checked="true"]' ); await expect(selectedOnly).toHaveCount(1); + await expect(selectedOnly).not.toBeFocused(); await expect(selectedOnly).toHaveCSS('outline-style', 'none'); await expect(selectedOnly).toHaveCSS( 'border-top-color', @@ -313,13 +323,9 @@ test('@web @playback keyboard reaches the dock and the settings radios with a vi path: test.info().outputPath(`${theme}-panel-focus.png`), }); - // A keyboard-focused swatch shows its tooltip, and Material spends - // the next Escape on a visible tooltip: close from a speed radio, - // which has none, once the swatch's tooltip has gone. - await pressTab(page, browserName); - expect( - await focusIsInside(page, '[data-test-id="player-settings-speed"]') - ).toBe(true); + // Focus is on a speed radio, which has no tooltip. Material spends + // an Escape on any tooltip still showing (the swatch's fades out), + // so wait for it to go before closing. await expect(page.locator('.mat-mdc-tooltip')).toHaveCount(0); await page.keyboard.press('Escape'); await expect(panel).toHaveCount(0); diff --git a/apps/web/src/assets/i18n/ar.json b/apps/web/src/assets/i18n/ar.json index b559c76eb..4a85f01e3 100644 --- a/apps/web/src/assets/i18n/ar.json +++ b/apps/web/src/assets/i18n/ar.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "مسارات الصوت", "SUBTITLES": "الترجمات", "SUBTITLES_OFF": "إيقاف", + "SUBTITLES_ON": "تشغيل", "QUALITY": "الجودة", "QUALITY_AUTO": "تلقائي", "TRACK_DEFAULT": "افتراضي", diff --git a/apps/web/src/assets/i18n/ary.json b/apps/web/src/assets/i18n/ary.json index 18c7ee6c3..17ef48074 100644 --- a/apps/web/src/assets/i18n/ary.json +++ b/apps/web/src/assets/i18n/ary.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "مسارات الصوت", "SUBTITLES": "الترجمة", "SUBTITLES_OFF": "معطلة", + "SUBTITLES_ON": "مفعلة", "QUALITY": "الجودة", "QUALITY_AUTO": "تلقائي", "TRACK_DEFAULT": "افتراضي", diff --git a/apps/web/src/assets/i18n/by.json b/apps/web/src/assets/i18n/by.json index de9410dfd..5870ef6e5 100644 --- a/apps/web/src/assets/i18n/by.json +++ b/apps/web/src/assets/i18n/by.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Аўдыядарожкі", "SUBTITLES": "Субтытры", "SUBTITLES_OFF": "Выкл.", + "SUBTITLES_ON": "Укл.", "QUALITY": "Якасць", "QUALITY_AUTO": "Аўта", "TRACK_DEFAULT": "Па змаўчанні", diff --git a/apps/web/src/assets/i18n/de.json b/apps/web/src/assets/i18n/de.json index ba84b5898..31d6adc55 100644 --- a/apps/web/src/assets/i18n/de.json +++ b/apps/web/src/assets/i18n/de.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Tonspuren", "SUBTITLES": "Untertitel", "SUBTITLES_OFF": "Aus", + "SUBTITLES_ON": "An", "QUALITY": "Qualität", "QUALITY_AUTO": "Automatisch", "TRACK_DEFAULT": "Standard", diff --git a/apps/web/src/assets/i18n/el.json b/apps/web/src/assets/i18n/el.json index bf9600dce..5fd530bf1 100644 --- a/apps/web/src/assets/i18n/el.json +++ b/apps/web/src/assets/i18n/el.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Κομμάτια ήχου", "SUBTITLES": "Υπότιτλοι", "SUBTITLES_OFF": "Ανενεργοί", + "SUBTITLES_ON": "Ενεργοί", "QUALITY": "Ποιότητα", "QUALITY_AUTO": "Αυτόματη", "TRACK_DEFAULT": "Προεπιλογή", diff --git a/apps/web/src/assets/i18n/en.json b/apps/web/src/assets/i18n/en.json index 85b8e61d6..0f3e22e84 100644 --- a/apps/web/src/assets/i18n/en.json +++ b/apps/web/src/assets/i18n/en.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Audio tracks", "SUBTITLES": "Subtitles", "SUBTITLES_OFF": "Off", + "SUBTITLES_ON": "On", "QUALITY": "Quality", "QUALITY_AUTO": "Auto", "TRACK_DEFAULT": "Default", diff --git a/apps/web/src/assets/i18n/es.json b/apps/web/src/assets/i18n/es.json index 17d528980..c4f916a67 100644 --- a/apps/web/src/assets/i18n/es.json +++ b/apps/web/src/assets/i18n/es.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Pistas de audio", "SUBTITLES": "Subtítulos", "SUBTITLES_OFF": "Desactivados", + "SUBTITLES_ON": "Activados", "QUALITY": "Calidad", "QUALITY_AUTO": "Automática", "TRACK_DEFAULT": "Predeterminada", diff --git a/apps/web/src/assets/i18n/fr.json b/apps/web/src/assets/i18n/fr.json index c6f535722..0327cce04 100644 --- a/apps/web/src/assets/i18n/fr.json +++ b/apps/web/src/assets/i18n/fr.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Pistes audio", "SUBTITLES": "Sous-titres", "SUBTITLES_OFF": "Désactivés", + "SUBTITLES_ON": "Activés", "QUALITY": "Qualité", "QUALITY_AUTO": "Auto", "TRACK_DEFAULT": "Par défaut", diff --git a/apps/web/src/assets/i18n/hu.json b/apps/web/src/assets/i18n/hu.json index 0e23a6d29..787271394 100644 --- a/apps/web/src/assets/i18n/hu.json +++ b/apps/web/src/assets/i18n/hu.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Hangsávok", "SUBTITLES": "Feliratok", "SUBTITLES_OFF": "Kikapcsolva", + "SUBTITLES_ON": "Bekapcsolva", "QUALITY": "Minőség", "QUALITY_AUTO": "Automatikus", "TRACK_DEFAULT": "Alapértelmezett", diff --git a/apps/web/src/assets/i18n/it.json b/apps/web/src/assets/i18n/it.json index a540b578b..8c8df9cc6 100644 --- a/apps/web/src/assets/i18n/it.json +++ b/apps/web/src/assets/i18n/it.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Tracce audio", "SUBTITLES": "Sottotitoli", "SUBTITLES_OFF": "Disattivati", + "SUBTITLES_ON": "Attivati", "QUALITY": "Qualità", "QUALITY_AUTO": "Automatica", "TRACK_DEFAULT": "Predefinita", diff --git a/apps/web/src/assets/i18n/ja.json b/apps/web/src/assets/i18n/ja.json index 3a090d609..4ce2ff60e 100644 --- a/apps/web/src/assets/i18n/ja.json +++ b/apps/web/src/assets/i18n/ja.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "音声トラック", "SUBTITLES": "字幕", "SUBTITLES_OFF": "オフ", + "SUBTITLES_ON": "オン", "QUALITY": "画質", "QUALITY_AUTO": "自動", "TRACK_DEFAULT": "デフォルト", diff --git a/apps/web/src/assets/i18n/ko.json b/apps/web/src/assets/i18n/ko.json index bf53349cf..2cc0b14ff 100644 --- a/apps/web/src/assets/i18n/ko.json +++ b/apps/web/src/assets/i18n/ko.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "오디오 트랙", "SUBTITLES": "자막", "SUBTITLES_OFF": "끄기", + "SUBTITLES_ON": "켜기", "QUALITY": "화질", "QUALITY_AUTO": "자동", "TRACK_DEFAULT": "기본", diff --git a/apps/web/src/assets/i18n/nl.json b/apps/web/src/assets/i18n/nl.json index 4f454a903..8ea43b97c 100644 --- a/apps/web/src/assets/i18n/nl.json +++ b/apps/web/src/assets/i18n/nl.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Audiosporen", "SUBTITLES": "Ondertiteling", "SUBTITLES_OFF": "Uit", + "SUBTITLES_ON": "Aan", "QUALITY": "Kwaliteit", "QUALITY_AUTO": "Automatisch", "TRACK_DEFAULT": "Standaard", diff --git a/apps/web/src/assets/i18n/pl.json b/apps/web/src/assets/i18n/pl.json index c8f661b9b..34c745998 100644 --- a/apps/web/src/assets/i18n/pl.json +++ b/apps/web/src/assets/i18n/pl.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Ścieżki audio", "SUBTITLES": "Napisy", "SUBTITLES_OFF": "Wyłączone", + "SUBTITLES_ON": "Włączone", "QUALITY": "Jakość", "QUALITY_AUTO": "Automatycznie", "TRACK_DEFAULT": "Domyślna", diff --git a/apps/web/src/assets/i18n/pt.json b/apps/web/src/assets/i18n/pt.json index 6b83a2af9..3725ef482 100644 --- a/apps/web/src/assets/i18n/pt.json +++ b/apps/web/src/assets/i18n/pt.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Faixas de áudio", "SUBTITLES": "Legendas", "SUBTITLES_OFF": "Desativadas", + "SUBTITLES_ON": "Ativadas", "QUALITY": "Qualidade", "QUALITY_AUTO": "Automática", "TRACK_DEFAULT": "Padrão", diff --git a/apps/web/src/assets/i18n/ru.json b/apps/web/src/assets/i18n/ru.json index 4d9d51736..a4e5a1c19 100644 --- a/apps/web/src/assets/i18n/ru.json +++ b/apps/web/src/assets/i18n/ru.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Аудиодорожки", "SUBTITLES": "Субтитры", "SUBTITLES_OFF": "Выкл.", + "SUBTITLES_ON": "Вкл.", "QUALITY": "Качество", "QUALITY_AUTO": "Авто", "TRACK_DEFAULT": "По умолчанию", diff --git a/apps/web/src/assets/i18n/tr.json b/apps/web/src/assets/i18n/tr.json index 52643a332..cbdba5560 100644 --- a/apps/web/src/assets/i18n/tr.json +++ b/apps/web/src/assets/i18n/tr.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "Ses parçaları", "SUBTITLES": "Altyazılar", "SUBTITLES_OFF": "Kapalı", + "SUBTITLES_ON": "Açık", "QUALITY": "Kalite", "QUALITY_AUTO": "Otomatik", "TRACK_DEFAULT": "Varsayılan", diff --git a/apps/web/src/assets/i18n/zh.json b/apps/web/src/assets/i18n/zh.json index 2a71e12bc..1a8f50ad7 100644 --- a/apps/web/src/assets/i18n/zh.json +++ b/apps/web/src/assets/i18n/zh.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "音轨", "SUBTITLES": "字幕", "SUBTITLES_OFF": "关闭", + "SUBTITLES_ON": "开启", "QUALITY": "画质", "QUALITY_AUTO": "自动", "TRACK_DEFAULT": "默认", diff --git a/apps/web/src/assets/i18n/zhtw.json b/apps/web/src/assets/i18n/zhtw.json index 482d6cac0..c9c7fa333 100644 --- a/apps/web/src/assets/i18n/zhtw.json +++ b/apps/web/src/assets/i18n/zhtw.json @@ -619,6 +619,7 @@ "AUDIO_TRACKS": "音軌", "SUBTITLES": "字幕", "SUBTITLES_OFF": "關閉", + "SUBTITLES_ON": "開啟", "QUALITY": "畫質", "QUALITY_AUTO": "自動", "TRACK_DEFAULT": "預設", diff --git a/docs/architecture/player-controls-contract.md b/docs/architecture/player-controls-contract.md index 30a36c302..07fae5b23 100644 --- a/docs/architecture/player-controls-contract.md +++ b/docs/architecture/player-controls-contract.md @@ -473,20 +473,23 @@ the panel the moment the last group disappears. (audio, subtitles, quality) are rows with a check mark and a cyan selection; segmented groups (speed, aspect, subtitle size) have a violet selection, and a selected default (`1×`, the first aspect preset) stays - neutral. Each group is one Tab stop — the checked option, else the first — - and arrows, Home and End move focus with a CDK `FocusKeyManager` - (wrapping; the horizontal arrows follow `direction`). Moving focus does not - apply: a choice is applied to the playing stream at once and a track or - quality switch rebuffers, so Space / Enter checks, as in a toolbar radio - group. Focus changes also write the roving `tabindex` immediately, because - a quick Shift+Tab, Tab can arrive before change detection updates the - bindings. The dialog is named by its `h2` title (`aria-labelledby`), each + neutral. Each group is one Tab stop — the checked option as the engine + reports it, else the first — and arrows, Home and End move focus with a + CDK `FocusKeyManager` (wrapping; the horizontal arrows follow `direction`) + and check the option they reach, as a native radio group does: the + directive clicks it, so the template's handler applies the choice. An + option the engine already reports as checked is not applied again. Focus + changes also write the roving `tabindex` immediately, because a quick + Shift+Tab, Tab can arrive before change detection updates the bindings. The dialog is named by its `h2` title (`aria-labelledby`), each radio group by its `h3` group heading or `h4` subheading, and the delay buttons form a labelled `group`. Headings read `--pc-text-secondary` on `--pc-glass-bg-dense` (`rgba(12,16,23,.86)`), 4.5:1 or more even over a white frame. They wrap with `overflow-wrap: anywhere` and `hyphens: auto`, so one long word (German "Wiedergabegeschwindigkeit") breaks inside the sheet's 84px heading column. Hyphenation needs a `lang` on ``. + With subtitles on but no track marked selected yet (the engine can report + the switch before the track list), the subtitle chip reads "On" + (`SUBTITLES_ON`), never "Off". Every focused option shows the `--pc-text` ring; a selected swatch has a white border with a dark inner gap, and focus adds an outer ring. The subtitle group carries the load-file action (a plain button outside the diff --git a/libs/ui/playback/src/lib/player-controls/player-controls.component.html b/libs/ui/playback/src/lib/player-controls/player-controls.component.html index b65139dbe..332c1e4bf 100644 --- a/libs/ui/playback/src/lib/player-controls/player-controls.component.html +++ b/libs/ui/playback/src/lib/player-controls/player-controls.component.html @@ -366,11 +366,16 @@ settings.available() && layout.roomy() && !settings.isOpen() ) { @if (settings.groups().subtitles) { + @let subtitleValue = settings.subtitleLabel() ?? - ('EMBEDDED_MPV.PLAYER.SUBTITLES_OFF' | translate); - + ((settings.subtitlesOn() + ? 'EMBEDDED_MPV.PLAYER.SUBTITLES_ON' + : 'EMBEDDED_MPV.PLAYER.SUBTITLES_OFF' + ) | translate);