From 17a2b1b3b043279d7a574d63d095a5d73c494f1f Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sat, 10 Oct 2026 17:10:24 +0200 Subject: [PATCH] fix(ui): visible keyboard focus everywhere via a global focus-visible fallback (#1866) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ui): visible keyboard focus everywhere via a global focus-visible fallback The global stylesheet removed the outline from every input, button, textarea and `:focus`, so Tab users saw no focus on about 110 raw buttons: detail actions, season tabs, chips, title results, list rows. - Remove the outline only under `:focus:not(:focus-visible)` and draw a fallback ring on every other `:focus-visible` element. Both rules sit in `:where()`, so any component that styles its own focus still wins. - One recipe: `focus-ring-declarations` moves to `libs/ui/styles/_focus-ring.scss` and reads `--app-focus-ring`, declared per theme (light #1d63e0, dark #8cbaff) with at least 3:1 on every app surface and selection tint; `m3-theme.spec.ts` measures it. The card ring, the detail actions, the portal sidebar and both context panels use the mixin (the panels' selection-blue ring fell to 2.97:1 on the active item). - Material Tab stops (buttons, switches, button toggles, checkboxes) take the ring over their 12% focus state layer; menu items and options keep Material's highlight. - Surfaces over video (player controls, vendor chrome, fullscreen panels, Up Next rail) point the ring at the player's text colour. - The selected season tab drew its border with `outline`, which outranks the fallback; it is a spread shadow now. - Guard: `pnpm run styles:focus-visible:validate` (CI) rejects a global `outline: none` on an unscoped selector and a missing fallback. - Electron E2E `keyboard-focus-ring.e2e.ts` tabs through the detail actions and season tabs, a catalog grid with its refinement chips, the Sources list and Settings in both themes: one ring per stop, 3:1 where measurable, none after a click. Co-Authored-By: Claude Opus 5.5 * fix(ui): keep the player ring in the fullscreen panel; tighten the focus guard and E2E Local review round 1 (Codex P2, Greptile 3×P2): - The fullscreen channel panel carries `dark-theme`, whose context declares the theme ring again, so its own controls ringed in #8cbaff. Point the token back at the player's text colour on `.fullscreen-channel-panel .dark-theme`; the web series-playback E2E checks the close button's ring. - The guard took a container-scoped rule (`.panel :focus-visible`), a mixin body or a media query as the global fallback. The fallback is now the whole selector, outside any at-rule. - The keyboard-focus E2E now fails a ring that an `overflow: hidden` ancestor (up to the scroll container) cuts. Co-Authored-By: Claude Opus 5.5 * fix(ui): show focus where a component removes its outline; catch invisible rings in the guard GitHub review round on #1866 (Codex P2, Greptile P2): - A component rule that sets `outline: none` outranks the zero-specificity fallback. Re-audited every one: two hid a Tab stop with nothing else to show. The EPG list row (`role="button"`) now draws an inset ring, and the command palette underlines its search row while the field has focus. The others already show focus on the field's wrapper, on another element, or as a highlight inside an arrow-key composite (the "…" menu, the guide's search listbox); the guidelines now state the rule. - The guard read only a bare zero or `none` as a removal. It now reads a zero width, a `none`/`hidden` style or a transparent colour anywhere in the shorthand or longhands, so `outline: 0 solid transparent` is reported and `outline: 2px solid transparent` is no fallback. - The keyboard-focus E2E walks the live EPG list and the command palette too, and reads an inset or offset shadow line as a ring. Co-Authored-By: Claude Opus 5.5 * fix(player): keep --pc-text a literal palette colour Master's palette spec (#1854) reads `--pc-text` from the controls host as a literal; this branch had made it `#{palette.$text}`, failing "sets the timeline label on the dense glass" on the merge ref. The host declares the literal again, and a spec keeps the palette's `$text` (the focus ring token for surfaces beside the host) equal to it. Co-Authored-By: Claude Opus 5.5 * build(epg): declare the ui-styles dependency of the list row's focus ring The EPG list row now `@use`s `libs/ui/styles/_focus-ring.scss`; Nx cannot infer a Sass import, so `ui-epg` declares `ui-styles` like the other consumers of the shared partials. Co-Authored-By: Claude Opus 5.5 * fix(tooling): a styleless outline shorthand hides focus too `outline: 2px` or `outline: red` resets the style to `none`, yet the guard counted either as a visible fallback. A shorthand without a drawn style (or a `var()` that may carry one) now counts as removing the outline, as do `initial` and `unset`. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- .changes/ui-visible-keyboard-focus.md | 9 + .github/workflows/ci.yml | 3 + .../src/keyboard-focus-ring.e2e.ts | 566 ++++++++++++++++++ .../web-e2e/src/xtream-series-playback.e2e.ts | 18 + apps/web/src/m3-theme.scss | 8 + apps/web/src/m3-theme.spec.ts | 81 +++ apps/web/src/styles.scss | 39 +- docs/architecture/iptvnator-ui-guidelines.md | 42 ++ .../content-card/content-card.component.scss | 2 +- libs/ui/epg/project.json | 1 + .../epg-list-view-row.component.scss | 8 +- .../lib/art-player/art-player.component.scss | 4 + .../fullscreen-channel-panel.component.scss | 10 + .../fullscreen-episode-panel.component.scss | 1 + .../lib/player-controls/_player-palette.scss | 17 +- .../player-controls.component.scss | 3 + .../player-controls.palette.spec.ts | 16 + .../up-next-rail.component.scss | 1 + .../lib/vjs-player/vjs-player.component.scss | 4 + libs/ui/styles/_content-grid.scss | 14 +- libs/ui/styles/_detail-view-actions.scss | 8 +- libs/ui/styles/_focus-ring.scss | 11 + libs/ui/styles/_index.scss | 1 + libs/ui/styles/_portal-sidebar.scss | 5 +- .../workspace-command-palette.component.scss | 6 + .../workspace-context-panel.component.scss | 5 +- ...pace-settings-context-panel.component.scss | 6 +- package.json | 3 + tools/nx/check-focus-visible.mjs | 319 ++++++++++ tools/nx/check-focus-visible.test.mjs | 287 +++++++++ 30 files changed, 1475 insertions(+), 23 deletions(-) create mode 100644 .changes/ui-visible-keyboard-focus.md create mode 100644 apps/electron-backend-e2e/src/keyboard-focus-ring.e2e.ts create mode 100644 libs/ui/styles/_focus-ring.scss create mode 100644 tools/nx/check-focus-visible.mjs create mode 100644 tools/nx/check-focus-visible.test.mjs diff --git a/.changes/ui-visible-keyboard-focus.md b/.changes/ui-visible-keyboard-focus.md new file mode 100644 index 000000000..98245e936 --- /dev/null +++ b/.changes/ui-visible-keyboard-focus.md @@ -0,0 +1,9 @@ +--- +type: fix +area: ui +--- + +Moving through the app with Tab now always shows where focus is: buttons, +season tabs, filter chips, list rows, switches and other controls get a +clear focus ring in both themes. Clicking with the mouse still leaves no +ring. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd5d050e2..d7692558c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -409,6 +409,9 @@ jobs: - name: Validate Angular Material token overrides run: pnpm run styles:material-tokens:validate + - name: Validate the global keyboard focus ring + run: pnpm run styles:focus-visible:validate + # The icon font renders an unknown ligature name as plain text. - name: Validate Material icon names run: pnpm run styles:icon-ligatures:validate diff --git a/apps/electron-backend-e2e/src/keyboard-focus-ring.e2e.ts b/apps/electron-backend-e2e/src/keyboard-focus-ring.e2e.ts new file mode 100644 index 000000000..5df2e7d7b --- /dev/null +++ b/apps/electron-backend-e2e/src/keyboard-focus-ring.e2e.ts @@ -0,0 +1,566 @@ +import type { APIRequestContext, Locator, Page } from '@playwright/test'; +import { + addXtreamPortal, + channelItemByTitle, + clickCategoryByNameExact, + clickFirstGridListCard, + closeElectronApp, + defaultXtreamPassword, + defaultXtreamUsername, + expect, + launchElectronApp, + openSettings, + openSettingsSection, + openSources, + openWorkspaceSection, + resetMockServers, + saveSettings, + test, + waitForXtreamWorkspaceReady, +} from './electron-test-fixtures'; +import { fetchXtreamLiveFixture, getXtreamTitle } from './portal-mock-fixtures'; +import { applyTheme } from './theme-contrast'; + +// --------------------------------------------------------------------------- +// Keyboard focus is visible everywhere. The global stylesheet used to strip +// the outline from every input, button and `:focus`; it now draws a fallback +// ring on `:focus-visible`, and a component with an indicator of its own +// keeps it. Every Tab stop below must show exactly one ring, on itself or on +// the ancestor or sibling that draws it for it (a content card, a checkbox, +// a select's form-field outline), in both themes, at 3:1 against what it +// sits on where that is a flat colour, and not cut by an ancestor's +// `overflow: hidden`. A mouse click leaves no ring. +// --------------------------------------------------------------------------- + +type Ring = { + kind: 'outline' | 'box-shadow' | 'field-outline'; + on: string; + color: string; + /** Null where the ring sits on artwork or a gradient. */ + contrast: number | null; + /** The `overflow: hidden` ancestor that cuts the outline, if any. */ + clippedBy: string | null; +}; + +type Stop = { element: string; label: string; rings: Ring[] }; + +type Probe = { + capture(): { element: string; label: string }; + settle(): Ring[]; + clicked(): { element: string; focusVisible: boolean; outline: string }; +}; + +declare global { + interface Window { + __focusRingProbe?: Probe; + } +} + +/** Calls one method of the probe installed in the page. */ +function callProbe( + page: Page, + method: M +): Promise> { + return page.evaluate((name) => { + const probe = window.__focusRingProbe; + if (!probe) throw new Error('The focus ring probe is not installed.'); + return probe[name]() as ReturnType; + }, method); +} + +/** + * Installs the in-page probe. `capture` snapshots the rings around the + * focused element; `settle`, run once focus has moved on, reports the rings + * that went away with it, i.e. the ones that focus drew. + */ +async function installProbe(page: Page): Promise { + await page.evaluate(() => { + if (window.__focusRingProbe) return; + const canvas = document.createElement('canvas').getContext('2d', { + willReadFrequently: true, + }) as CanvasRenderingContext2D; + const transparent = (color: string) => + color === 'transparent' || /,\s*0\)$/.test(color); + const ringOf = (el: Element) => { + const style = getComputedStyle(el); + const outline = + style.outlineStyle !== 'none' && + parseFloat(style.outlineWidth) > 0 && + !transparent(style.outlineColor) + ? style.outlineColor + : ''; + // A focus ring or line drawn as an unblurred shadow: a spread + // ring (` 0 0 0 2px`) or an offset line, inset or not. + const line = + /(rgba?\([^)]*\)) (-?[\d.]+)px (-?[\d.]+)px 0px(?: (-?[\d.]+)px)?( inset)?/.exec( + style.boxShadow + ); + const shadow = + line && + [line[2], line[3], line[4] ?? '0'].some( + (length) => parseFloat(length) !== 0 + ) + ? line[0] + : ''; + return { + outline, + offset: parseFloat(style.outlineOffset) || 0, + shadow, + // A Material form field shows focus on its outline's border. + border: el.classList.contains('mdc-notched-outline__leading') + ? `${style.borderTopWidth} ${style.borderTopColor}` + : '', + }; + }; + const name = (el: Element) => + el.tagName.toLowerCase() + + [...el.classList] + .filter((c) => !c.startsWith('ng-') && !c.startsWith('cdk-')) + .slice(0, 2) + .map((c) => `.${c}`) + .join(''); + /** Luminance of `colors` painted in order over white. */ + const paint = (colors: string[]) => { + canvas.fillStyle = '#fff'; + canvas.fillRect(0, 0, 1, 1); + for (const color of colors) { + canvas.fillStyle = color; + canvas.fillRect(0, 0, 1, 1); + } + const [r, g, b] = Array.from( + canvas.getImageData(0, 0, 1, 1).data.slice(0, 3) + ).map((channel) => { + const c = channel / 255; + return c <= 0.03928 ? c / 12.92 : ((c + 0.055) / 1.055) ** 2.4; + }); + return 0.2126 * r + 0.7152 * g + 0.0722 * b; + }; + /** Background layers from the root down to `el`, or null on art. */ + const layersUnder = (el: Element | null) => { + const layers: string[] = []; + for (let node = el; node; node = node.parentElement) { + const style = getComputedStyle(node); + if (style.backgroundImage !== 'none') return null; + layers.unshift(style.backgroundColor); + } + return layers; + }; + /** + * The ancestor whose `overflow: hidden` cuts `el`'s outline, looking + * up to the nearest scroll container: past it, whether a ring at the + * edge shows depends on the scroll position, not on the styles. + */ + const clipperOf = (el: Element) => { + const style = getComputedStyle(el); + if (style.outlineStyle === 'none') return null; + const reach = Math.max( + 0, + (parseFloat(style.outlineOffset) || 0) + + parseFloat(style.outlineWidth) + ); + const box = el.getBoundingClientRect(); + for (let node = el.parentElement; node; node = node.parentElement) { + const clip = getComputedStyle(node); + const axes = [clip.overflowX, clip.overflowY]; + if (axes.some((axis) => axis === 'auto' || axis === 'scroll')) { + return null; + } + if ( + !axes.some((axis) => axis === 'hidden' || axis === 'clip') + ) { + continue; + } + const edge = node.getBoundingClientRect(); + const inset = (side: string) => + parseFloat(clip.getPropertyValue(`border-${side}-width`)) - + 0.5; + if ( + box.left - reach < edge.left + inset('left') || + box.top - reach < edge.top + inset('top') || + box.right + reach > edge.right - inset('right') || + box.bottom + reach > edge.bottom - inset('bottom') + ) { + return name(node); + } + } + return null; + }; + const contrastOf = (on: Element, color: string, inside: boolean) => { + const layers = layersUnder(inside ? on : on.parentElement); + if (!layers) return null; + const back = paint(layers); + const ring = paint([...layers, color]); + return ( + (Math.max(back, ring) + 0.05) / (Math.min(back, ring) + 0.05) + ); + }; + + let pending: { + candidates: Element[]; + before: (ReturnType & { + clippedBy: string | null; + })[]; + } | null = null; + + window.__focusRingProbe = { + capture() { + const el = document.activeElement ?? document.body; + const candidates = [el]; + for ( + let node = el.parentElement, depth = 0; + node && depth < 3; + node = node.parentElement, depth++ + ) { + candidates.push(node); + } + for (const sibling of el.parentElement?.children ?? []) { + if (sibling !== el) candidates.push(sibling); + } + const fieldOutline = el + .closest('.mat-mdc-form-field') + ?.querySelector('.mdc-notched-outline__leading'); + if (fieldOutline) candidates.push(fieldOutline); + // Geometry is read while focused: moving on may scroll. + pending = { + candidates, + before: candidates.map((candidate) => ({ + ...ringOf(candidate), + clippedBy: clipperOf(candidate), + })), + }; + return { + element: name(el), + label: ( + el.getAttribute('aria-label') ?? + el.textContent ?? + '' + ) + .replace(/\s+/g, ' ') + .trim() + .slice(0, 40), + }; + }, + settle() { + if (!pending) return []; + const { candidates, before } = pending; + pending = null; + return candidates.flatMap((on, index) => { + const was = before[index]; + const now = ringOf(on); + const rings: Ring[] = []; + if (was.outline && was.outline !== now.outline) { + rings.push({ + kind: 'outline', + on: name(on), + color: was.outline, + contrast: contrastOf( + on, + was.outline, + was.offset < 0 + ), + clippedBy: was.clippedBy, + }); + } + if (was.shadow && was.shadow !== now.shadow) { + const color = was.shadow.replace(/\) .*$/, ')'); + rings.push({ + kind: 'box-shadow', + on: name(on), + color, + contrast: contrastOf( + on, + color, + was.shadow.endsWith(' inset') + ), + clippedBy: null, + }); + } + if (was.border && was.border !== now.border) { + const color = getComputedStyle(on).borderTopColor; + const focused = was.border.replace(/^\S+ /, ''); + rings.push({ + kind: 'field-outline', + on: name(on), + color: focused, + // Against the field's own fill, inside the line. + contrast: + focused === color + ? null + : contrastOf(on, focused, true), + clippedBy: null, + }); + } + return rings; + }); + }, + clicked() { + const el = document.activeElement ?? document.body; + return { + element: name(el), + focusVisible: el.matches(':focus-visible'), + outline: ringOf(el).outline, + }; + }, + }; + }); +} + +/** + * Tabs from `from` through `area`, up to `limit` stops, and reports the + * rings each stop drew. `from` is entered from the keyboard (Tab, then + * Shift+Tab) so it matches `:focus-visible` like the stops after it. + */ +async function tabThrough( + page: Page, + area: Locator, + from: Locator, + limit: number +): Promise { + await installProbe(page); + await from.focus(); + await page.keyboard.press('Tab'); + await page.keyboard.press('Shift+Tab'); + await expect(from).toBeFocused(); + + const stops: Stop[] = []; + while (stops.length < limit) { + const inside = await area.evaluate((root) => + root.contains(document.activeElement) + ); + if (!inside) break; + const focused = await callProbe(page, 'capture'); + await page.keyboard.press('Tab'); + const rings = await callProbe(page, 'settle'); + stops.push({ ...focused, rings }); + } + return stops; +} + +/** Every stop shows one uncut ring with 3:1 against a flat background. */ +function expectOneVisibleRing(stops: Stop[], context: string): void { + expect(stops.length, `${context}: Tab stops`).toBeGreaterThan(1); + for (const stop of stops) { + const where = `${context}: ${stop.element} "${stop.label}"`; + expect(stop.rings, where).toHaveLength(1); + const [ring] = stop.rings; + expect(ring.clippedBy, `${where} ring clipped`).toBeNull(); + if (ring.contrast !== null) { + expect( + ring.contrast, + `${where} ring contrast` + ).toBeGreaterThanOrEqual(3); + } + } +} + +/** A mouse click focuses `target` without a ring. */ +async function expectNoRingAfterClick( + page: Page, + target: Locator, + context: string +): Promise { + await installProbe(page); + await target.click(); + const clicked = await callProbe(page, 'clicked'); + expect(clicked, `${context}: focus after a click`).toEqual({ + element: clicked.element, + focusVisible: false, + outline: '', + }); +} + +async function openFirstSeries(page: Page): Promise { + await openWorkspaceSection(page, 'Series'); + await clickFirstGridListCard(page); + await expect(page.locator('[data-test-id="episodes-heading"]')).toBeVisible( + { timeout: 20_000 } + ); +} + +async function openMoviesWithRatingFilter(page: Page): Promise { + await openWorkspaceSection(page, 'Movies'); + const view = page.locator('app-category-content-view'); + await expect(view.locator('mat-card').first()).toBeVisible({ + timeout: 20_000, + }); + await view.locator('.refine-action').click(); + await page.getByRole('menuitem', { name: /^\s*5\.0 and higher/ }).click(); + await expect(view.locator('.rating-refinement-chip')).toBeVisible(); +} + +/** Switches the guide to its list view and opens a live channel's EPG. */ +async function openLiveEpgList(page: Page, request: APIRequestContext) { + const fixture = await fetchXtreamLiveFixture(request, { + username: defaultXtreamUsername, + password: defaultXtreamPassword, + }); + const channel = fixture.items[0]; + if (!channel) throw new Error('Expected a live channel in the mock.'); + // The mock's live streams redirect to an external demo stream; the + // guide needs none of it, so keep that request pending. + await page.route('https://test-streams.mux.dev/**', () => undefined); + await openSettings(page); + await openSettingsSection(page, 'epg'); + await page.getByTestId('epg-view-mode-list').click(); + await saveSettings(page); + await openWorkspaceSection(page, 'Live TV'); + await clickCategoryByNameExact(page, fixture.categoryName); + await channelItemByTitle(page, getXtreamTitle(channel)).first().click(); + const guide = page.locator('app-epg-list-view'); + await expect(guide.locator('app-epg-list-view-row').first()).toBeVisible({ + timeout: 20_000, + }); + return guide; +} + +/** Opens the command palette from the keyboard. */ +async function openCommandPalette(page: Page): Promise { + await page.locator('body').focus(); + await page.keyboard.press( + `${process.platform === 'darwin' ? 'Meta' : 'Control'}+K` + ); + const palette = page.locator( + 'mat-dialog-container app-workspace-command-palette' + ); + await expect(palette).toBeVisible(); + return palette; +} + +test.describe('Keyboard focus ring', () => { + test('@electron @theme every Tab stop shows one visible ring in both themes, a click none', async ({ + dataDir, + request, + }) => { + await resetMockServers(request, ['xtream']); + const app = await launchElectronApp(dataDir); + const page = app.mainWindow; + const report: Record = {}; + try { + await addXtreamPortal(page); + await waitForXtreamWorkspaceReady(page); + + const walk = async ( + context: string, + area: Locator, + from: Locator, + limit: number + ) => { + for (const theme of ['light', 'dark'] as const) { + await applyTheme(page, theme); + const stops = await tabThrough(page, area, from, limit); + report[`${context} (${theme})`] = stops; + expectOneVisibleRing(stops, `${context} in ${theme}`); + } + }; + + // Detail page: the hero's action row, then the season tabs. + await openFirstSeries(page); + const shell = page.locator('app-portal-detail-shell'); + await walk( + 'detail actions and season tabs', + shell, + shell.locator('app-detail-action-button button').first(), + 14 + ); + expect( + report['detail actions and season tabs (dark)'].map( + (stop) => stop.element + ) + ).toContain('button.season-tabs__pill'); + + const pill = shell.locator('.season-tabs__pill').first(); + await expectNoRingAfterClick(page, pill, 'season tab'); + await expectNoRingAfterClick( + page, + shell.locator('.mat-button-toggle-button').last(), + 'episode view toggle' + ); + + // A catalog grid and its refinement chips. Closing the refine + // menu with a click hands focus back to its trigger by script: + // still pointer focus, so no ring. + await openMoviesWithRatingFilter(page); + await installProbe(page); + expect(await callProbe(page, 'clicked')).toEqual({ + element: expect.stringContaining('mat-mdc-button-base'), + focusVisible: false, + outline: '', + }); + const view = page.locator('app-category-content-view'); + await walk( + 'catalog refinements and grid', + view, + view.locator('.rating-refinement-chip'), + 6 + ); + + // Sources: the type filters, then the list's row actions. + await openSources(page); + await expect( + page.locator('app-playlist-item').first() + ).toBeVisible(); + await walk( + 'sources list', + page.locator('app-workspace-shell'), + page + .locator('app-workspace-sources-filters-panel button') + .first(), + 12 + ); + expect( + report['sources list (dark)'].map((stop) => stop.label) + ).toContain('Check again'); + + // The live EPG list: its rows are Tab stops (`role="button"`) + // with their own inset ring, then the row's info button. + const guide = await openLiveEpgList(page, request); + await walk( + 'epg list', + guide, + guide.locator('app-epg-list-view-row').first(), + 4 + ); + expect( + report['epg list (dark)'].map((stop) => stop.element) + ).toContainEqual(expect.stringMatching(/^app-epg-list-view-row\b/)); + + // Settings: the section navigation, then the first page's + // controls (selects, switches, the theme switcher). + await openSettings(page); + const firstSection = page + .locator('[data-test-id^="settings-section-"]') + .first(); + await walk( + 'settings', + page.locator('app-workspace-shell'), + firstSection, + 24 + ); + const toggle = page + .getByTestId('settings-container') + .locator('mat-slide-toggle [role="switch"]') + .first(); + await expectNoRingAfterClick(page, toggle, 'settings switch'); + await toggle.click(); + + // The command palette: its search field draws no outline, so the + // row underlines it; then the commands. + const palette = await openCommandPalette(page); + await walk( + 'command palette', + palette, + palette.locator('.palette-search input'), + 3 + ); + await page.keyboard.press('Escape'); + await expect(palette).toBeHidden(); + } finally { + // Every stop and the rings it drew, for a failure on CI. + await test.info().attach('focus-stops.json', { + body: JSON.stringify(report, null, 2), + contentType: 'application/json', + }); + await closeElectronApp(app); + } + }); +}); diff --git a/apps/web-e2e/src/xtream-series-playback.e2e.ts b/apps/web-e2e/src/xtream-series-playback.e2e.ts index 80a5e7159..e1ec9a424 100644 --- a/apps/web-e2e/src/xtream-series-playback.e2e.ts +++ b/apps/web-e2e/src/xtream-series-playback.e2e.ts @@ -223,6 +223,24 @@ test.describe('@xtream inline series fullscreen', () => { await expect(rows).toHaveCount(8); await expect(rows.first()).toHaveAttribute('aria-current', 'true'); + // Keyboard focus in the panel wears the player's ring, not the + // theme's: the panel carries `dark-theme` for its tokens, and that + // context declares the app's ring colour again. + await page.keyboard.press('Tab'); + expect( + await page.evaluate(() => { + const focused = document.activeElement as HTMLElement; + const style = getComputedStyle(focused); + return { + control: focused.dataset['testId'], + ring: `${style.outlineStyle} ${style.outlineColor}`, + }; + }) + ).toEqual({ + control: 'fullscreen-channel-panel-close', + ring: 'solid rgb(231, 236, 243)', + }); + // Picking an episode plays it inline through the same path as the Up // Next rail: fullscreen survives the engine remount, the panel closes. await rows.nth(2).click(); diff --git a/apps/web/src/m3-theme.scss b/apps/web/src/m3-theme.scss index 3f4f4e52d..acd62acd0 100644 --- a/apps/web/src/m3-theme.scss +++ b/apps/web/src/m3-theme.scss @@ -107,6 +107,11 @@ html { // Watch progress in app chrome (rail cards, catalog capsules, the hero). // Over video the player's fixed `--pc-progress` applies instead. --app-progress-color: var(--app-selection-color); + // Keyboard focus ring (`libs/ui/styles/_focus-ring.scss`), drawn 2px + // outside the focused element. A deeper blue than the selection colour: + // at least 3.6:1 on every light surface and selection tint, where + // #2f7bff falls to 2.6:1 on the stronger tint. + --app-focus-ring: #1d63e0; // Live / on-air accent (broadcast indicator) --app-live-color: #d12c2c; @@ -210,6 +215,9 @@ html { // Re-declared so it resolves against this context's selection colour // rather than inheriting the light value computed on `html`. --app-progress-color: var(--app-selection-color); + // Keyboard focus ring: at least 4.1:1 on every dark surface and + // selection tint. + --app-focus-ring: #8cbaff; // Live / on-air accent (broadcast indicator) --app-live-color: #ff8f8f; diff --git a/apps/web/src/m3-theme.spec.ts b/apps/web/src/m3-theme.spec.ts index bdacc25f6..d30e68502 100644 --- a/apps/web/src/m3-theme.spec.ts +++ b/apps/web/src/m3-theme.spec.ts @@ -51,3 +51,84 @@ describe('m3-theme watch-progress token', () => { ); }); }); + +/** WCAG relative luminance of a `#rrggbb` colour. */ +function luminance(hex: string): number { + const [r, g, b] = [1, 3, 5].map((start) => { + const channel = parseInt(hex.slice(start, start + 2), 16) / 255; + return channel <= 0.03928 + ? channel / 12.92 + : ((channel + 0.055) / 1.055) ** 2.4; + }); + return 0.2126 * r + 0.7152 * g + 0.0722 * b; +} + +function contrast(a: string, b: string): number { + const [light, dark] = [luminance(a), luminance(b)].sort((x, y) => y - x); + return (light + 0.05) / (dark + 0.05); +} + +/** `top` at `percent`% over an opaque `base`, as color-mix(in srgb) does. */ +function mix(top: string, percent: number, base: string): string { + return `#${[1, 3, 5] + .map((start) => { + const over = parseInt(top.slice(start, start + 2), 16); + const under = parseInt(base.slice(start, start + 2), 16); + return Math.round((over * percent + under * (100 - percent)) / 100) + .toString(16) + .padStart(2, '0'); + }) + .join('')}`; +} + +function hexToken(context: string, name: string): string { + const value = new RegExp(`${name}:\\s*(#[0-9a-f]{6});`, 'i').exec( + context + )?.[1]; + expect(value).toBeDefined(); + return (value ?? '').toLowerCase(); +} + +describe('m3-theme focus ring token', () => { + // The ring sits 2px outside the focused element, on whatever the element + // sits on: an app surface, or one tinted by the selection colour. + const surfaces = [ + '--app-shell-bg', + '--app-rail-bg', + '--app-header-bg', + '--app-content-bg', + '--app-widget-bg', + '--app-widget-header-bg', + '--app-card-hover-bg', + ]; + + it('declares a ring with at least 3:1 on every surface in both themes', () => { + const { light, dark } = themeContexts(); + + for (const [theme, context] of Object.entries({ light, dark })) { + const ring = hexToken(context, '--app-focus-ring'); + const selection = hexToken(context, '--app-selection-color'); + const strongTint = Number( + /--app-selection-surface-strong:\s*color-mix\(\s*in srgb,\s*var\(--app-selection-color\)\s*(\d+)%/.exec( + context + )?.[1] + ); + expect(strongTint).toBeGreaterThan(0); + + for (const surface of surfaces) { + const base = hexToken(context, surface); + for (const background of [ + base, + mix(selection, strongTint, base), + ]) { + expect({ + theme, + surface, + background, + ratio: contrast(ring, background) >= 3, + }).toEqual({ theme, surface, background, ratio: true }); + } + } + } + }); +}); diff --git a/apps/web/src/styles.scss b/apps/web/src/styles.scss index 9afd95269..83a12c6fd 100644 --- a/apps/web/src/styles.scss +++ b/apps/web/src/styles.scss @@ -1,5 +1,6 @@ @use '@angular/material' as mat; @use '../../../libs/ui/components/src/lib/resizable/resizable.scss'; +@use '../../../libs/ui/styles/focus-ring'; @use './settings-theme'; @use './cover-size'; @@ -164,13 +165,43 @@ textarea { opacity: 1 !important; } -input, -button, -textarea, -:focus { +// ─── Keyboard focus ─────────────────────────────────────────────────────────── +// Pointer focus draws no ring; keyboard focus always does. Both rules have +// zero specificity (`:where`), so a component that styles its own focus — an +// outline, a ring drawn on another element, or `outline: none` beside an +// indicator of its own — wins whatever the stylesheet order. +// `pnpm run styles:focus-visible:check` keeps a blanket `outline: none` out. +:where(:focus:not(:focus-visible)) { outline: none; } +:where(:focus-visible) { + @include focus-ring.focus-ring-declarations; +} + +// Material sets `outline: none` on its controls and shows keyboard focus as a +// 12% state layer, well under the 3:1 a focus indicator needs, so its Tab +// stops take the ring too (over the state layer, which stays). Items inside +// arrow-key composites (menu items, select options) keep Material's +// highlight. A component that rings its own Material control (the player's +// `--pc-text` outline) is more specific and wins. +.mat-mdc-button-base:focus-visible, +.mat-mdc-slide-toggle .mdc-switch:focus-visible { + @include focus-ring.focus-ring-declarations; +} + +// The group clips its toggles (`overflow: hidden`): draw the ring inside. +.mat-button-toggle-button:focus-visible { + @include focus-ring.focus-ring-declarations($offset: -3px); +} + +// The native checkbox is transparent; ring the box drawn beside it. +.mat-mdc-checkbox + .mdc-checkbox__native-control:focus-visible + ~ .mdc-checkbox__background { + @include focus-ring.focus-ring-declarations; +} + // ─── Dark Theme ─────────────────────────────────────────────────────────────── // Only the app host takes the dark surface: components such as the fullscreen // channel panel and the playback diagnostic's alternative sources carry diff --git a/docs/architecture/iptvnator-ui-guidelines.md b/docs/architecture/iptvnator-ui-guidelines.md index 7a4773f5e..ed0a9446d 100644 --- a/docs/architecture/iptvnator-ui-guidelines.md +++ b/docs/architecture/iptvnator-ui-guidelines.md @@ -62,6 +62,7 @@ in `apps/web/src/m3-theme.scss`): - `--app-shell-bg` / `--app-rail-bg` / `--app-header-bg` / `--app-content-bg` - `--app-widget-bg` / `--app-widget-header-bg` — panels and popovers - `--app-card-hover-bg` — raised or hovered rows +- `--app-focus-ring` — the keyboard focus ring (see Keyboard Focus) - `--app-widget-border` / `--app-rail-border` — hairlines - `--app-on-surface` — primary text - `--app-eyebrow-color` — secondary/muted text @@ -233,6 +234,47 @@ context panels and the Favorites sidebar) or `border-box` (live-layout and groups rails); starting from the border box widened a padded `content-box` host by its padding and border on the first move. +## Keyboard Focus + +- **Always a ring for the keyboard, none for the pointer.** + `apps/web/src/styles.scss` removes the outline only under + `:focus:not(:focus-visible)` and draws a fallback ring on every other + `:focus-visible` element: 2px of `--app-focus-ring` at a 2px offset, from + `focus-ring-declarations` in `libs/ui/styles/_focus-ring.scss`. Both rules + sit in `:where()`, so any component rule that styles focus wins. Never + bring back a global `outline: none`: `pnpm run styles:focus-visible:validate` + (CI) rejects one on an unscoped selector, and a missing fallback. +- **Token.** `--app-focus-ring` is declared per theme in `m3-theme.scss`, + deeper than the selection blue in light, so it keeps 3:1 on every app + surface and selection tint; `apps/web/src/m3-theme.spec.ts` measures it. +- **A ring of your own.** A component that needs a different ring styles its + `:focus-visible` with the mixin rather than a literal colour. Draw it on + the element that owns `overflow: hidden` (the cards' `card-focus-ring`), + or inside the element (a negative offset) when a clipping parent would cut + it. When an ancestor or a box shadow draws the ring, set `outline: none` on + the focused element so it never shows two. Never use `outline` as a + decorative border on something focusable: any outline outranks the + fallback, so the element loses its ring (use `border` or a spread + `box-shadow`, as the selected season tab does). +- **Removing the outline.** A component rule that sets `outline: none` also + outranks the fallback, so it must show focus another way: a `:focus-within` + change on a search field's wrapper (the command palette underlines its + row), a ring drawn on another element, or a highlight on an item inside an + arrow-key composite. +- **Material.** Buttons, switches, button toggles and checkboxes take the + ring over their 12% focus state layer (global rules in `styles.scss`). + Menu items and select options keep Material's highlight, since arrow keys, + not Tab, move within them. +- **Over video** the ring is the player's text colour, not the theme's: + the player controls, vendor chrome, the fullscreen channel and episode + panels and the Up Next rail include `focus-ring-token` from + `_player-palette.scss`. +- **Check.** `keyboard-focus-ring.e2e.ts` (Electron) tabs through the detail + actions and season tabs, a catalog grid with its refinement chips, the + Sources list and Settings in both themes. Each stop must draw exactly one + ring, uncut by an ancestor's `overflow: hidden`, at 3:1 where it sits on a + flat colour, and a click must draw none. + ## Keyboard Scrolling and Channel Focus `ChannelScrollFocusDirective` belongs on the actual channel scroll owner, diff --git a/libs/portal/shared/ui/src/lib/components/content-card/content-card.component.scss b/libs/portal/shared/ui/src/lib/components/content-card/content-card.component.scss index 00e628bd8..cb3c1134c 100644 --- a/libs/portal/shared/ui/src/lib/components/content-card/content-card.component.scss +++ b/libs/portal/shared/ui/src/lib/components/content-card/content-card.component.scss @@ -14,7 +14,7 @@ } &:has(> .content-card__activation:focus-visible) { - @include grid.focus-ring-declarations; + @include grid.card-focus-ring-declarations; } .remove-button { diff --git a/libs/ui/epg/project.json b/libs/ui/epg/project.json index 5dcfa878d..55323c45f 100644 --- a/libs/ui/epg/project.json +++ b/libs/ui/epg/project.json @@ -5,6 +5,7 @@ "prefix": "lib", "projectType": "library", "tags": ["scope:shared", "domain:playback", "type:ui"], + "implicitDependencies": ["ui-styles"], "targets": { "test": { "executor": "nx:run-commands", diff --git a/libs/ui/epg/src/lib/epg-list-view/epg-list-view-row/epg-list-view-row.component.scss b/libs/ui/epg/src/lib/epg-list-view/epg-list-view-row/epg-list-view-row.component.scss index 4f08f17a9..e56ab0669 100644 --- a/libs/ui/epg/src/lib/epg-list-view/epg-list-view-row/epg-list-view-row.component.scss +++ b/libs/ui/epg/src/lib/epg-list-view/epg-list-view-row/epg-list-view-row.component.scss @@ -1,4 +1,5 @@ @use '../../epg-theme' as *; +@use '../../../../../styles/focus-ring'; :host { box-sizing: border-box; @@ -11,7 +12,12 @@ cursor: pointer; border-left: 3px solid transparent; transition: background 120ms ease; - outline: none; +} + +/* The row is a Tab stop (role=button). Its ring sits inside: the list's + * scroller would cut one drawn outside the full-width row. */ +:host(:focus-visible) { + @include focus-ring.focus-ring-declarations($offset: -2px); } :host *, :host *::before { diff --git a/libs/ui/playback/src/lib/art-player/art-player.component.scss b/libs/ui/playback/src/lib/art-player/art-player.component.scss index 62d96fdd5..2aa4a9a3c 100644 --- a/libs/ui/playback/src/lib/art-player/art-player.component.scss +++ b/libs/ui/playback/src/lib/art-player/art-player.component.scss @@ -1,4 +1,8 @@ +@use '../player-controls/player-palette' as palette; + :host { + // Vendor chrome over video takes the player's focus ring. + @include palette.focus-ring-token; display: block; width: 100%; height: 100%; diff --git a/libs/ui/playback/src/lib/fullscreen-channel-panel/fullscreen-channel-panel.component.scss b/libs/ui/playback/src/lib/fullscreen-channel-panel/fullscreen-channel-panel.component.scss index 6b75089d2..02f0d8561 100644 --- a/libs/ui/playback/src/lib/fullscreen-channel-panel/fullscreen-channel-panel.component.scss +++ b/libs/ui/playback/src/lib/fullscreen-channel-panel/fullscreen-channel-panel.component.scss @@ -1,4 +1,5 @@ @use '@angular/material' as mat; +@use '../player-controls/player-palette' as palette; // Global styles on purpose (ViewEncapsulation.None): the host page's list is // stamped into the panel body through an ng-template, so its nodes carry the // host's scoping attribute and an emulated `.body > *` rule could never reach @@ -8,6 +9,15 @@ // Never take layout space beside the engine; every child is absolutely // positioned inside the `.web-player-view` box. display: contents; + // On video: the edge tab and the panel take the player's focus ring. + @include palette.focus-ring-token; +} + +// The panel carries `dark-theme` for its tokens, and that context declares +// the theme's ring again (`html .dark-theme`); point it back at the player's +// here, more specifically, for the header and the stamped host lists. +.fullscreen-channel-panel.dark-theme { + @include palette.focus-ring-token; } // ─── Closed state ──────────────────────────────────────────────────────────── diff --git a/libs/ui/playback/src/lib/fullscreen-episode-panel/fullscreen-episode-panel.component.scss b/libs/ui/playback/src/lib/fullscreen-episode-panel/fullscreen-episode-panel.component.scss index 2ec56938c..bfbfaa51f 100644 --- a/libs/ui/playback/src/lib/fullscreen-episode-panel/fullscreen-episode-panel.component.scss +++ b/libs/ui/playback/src/lib/fullscreen-episode-panel/fullscreen-episode-panel.component.scss @@ -7,6 +7,7 @@ // declares the player's progress colour itself. :host { @include palette.progress-token; + @include palette.focus-ring-token; display: flex; flex-direction: column; min-height: 0; diff --git a/libs/ui/playback/src/lib/player-controls/_player-palette.scss b/libs/ui/playback/src/lib/player-controls/_player-palette.scss index d33d887c5..f596cc636 100644 --- a/libs/ui/playback/src/lib/player-controls/_player-palette.scss +++ b/libs/ui/playback/src/lib/player-controls/_player-palette.scss @@ -2,7 +2,9 @@ // controls declare their `--pc-*` palette on `app-player-controls`' `:host`; // the Up Next rail and the fullscreen episode panel render beside that host, // not inside it, so they include `progress-token` on their own `:host` to -// draw watch progress in the same colour as the dock. +// draw watch progress in the same colour as the dock. The surfaces drawn +// over video (those two, the controls, vendor chrome and the fullscreen +// channel panel) include `focus-ring-token`. // // ArtPlayer's vendor chrome takes the same value as a string at construction: // `PLAYER_PROGRESS_COLOR` in `player-palette.ts`, pinned to `$accent-blue` by @@ -11,9 +13,22 @@ // Accent blue: the primary action and watch progress over video. $accent-blue: #4f8eff; +// Overlay text, the keyboard focus ring over video. The controls host +// declares `--pc-text` with the same literal; the palette spec keeps the two +// in step. +$text: #e7ecf3; + // `--pc-progress`, the watch-progress fill over video. Literal like the rest // of the overlay palette: the player is theme-independent, and the app's // selection blue would change with the theme and fight the video. @mixin progress-token { --pc-progress: #{$accent-blue}; } + +// Points the app's `:focus-visible` fallback ring (`--app-focus-ring`, see +// `libs/ui/styles/_focus-ring.scss`) at the overlay text colour, so a control +// over video without a ring of its own gets the player's ring rather than +// the theme's blue. Included on every surface that sits on video. +@mixin focus-ring-token { + --app-focus-ring: #{$text}; +} diff --git a/libs/ui/playback/src/lib/player-controls/player-controls.component.scss b/libs/ui/playback/src/lib/player-controls/player-controls.component.scss index 07a4b47be..2b766250b 100644 --- a/libs/ui/playback/src/lib/player-controls/player-controls.component.scss +++ b/libs/ui/playback/src/lib/player-controls/player-controls.component.scss @@ -10,6 +10,7 @@ // which the Up Next rail and the fullscreen episode panel also read. --pc-accent-blue: #{palette.$accent-blue}; @include palette.progress-token; + @include palette.focus-ring-token; // Fills that carry a white glyph (play, active tune) use deeper blues: // white on #4f8eff is only 3.2:1, too close to the 3:1 floor once a // 1x display antialiases the glyph. These give 4.4:1 and 5.3:1. @@ -24,6 +25,8 @@ // text read 6.2:1 on the glass over a black frame. --pc-live: #d32f2f; --pc-danger: #ff5252; + // Literal, like the rest (the palette spec reads it); `palette.$text` + // repeats it for the focus ring over video. --pc-text: #e7ecf3; --pc-text-secondary: #9aa3b2; --pc-text-tertiary: #6b7384; diff --git a/libs/ui/playback/src/lib/player-controls/player-controls.palette.spec.ts b/libs/ui/playback/src/lib/player-controls/player-controls.palette.spec.ts index 148eacdd7..2952e4efd 100644 --- a/libs/ui/playback/src/lib/player-controls/player-controls.palette.spec.ts +++ b/libs/ui/playback/src/lib/player-controls/player-controls.palette.spec.ts @@ -200,6 +200,22 @@ describe('player controls overlay palette', () => { } }); + it('points the app focus ring over video at the overlay text colour', () => { + // Surfaces beside the controls host (the fullscreen panels, the Up + // Next rail, vendor chrome) have no `--pc-text`, so the ring token + // carries the colour itself. + const text = PALETTE_STYLES.match(/\$text:\s*(#[0-9a-f]{6});/i)?.[1]; + expect(text?.toLowerCase()).toBe( + HOST_STYLES.match( + /--pc-text:\s*(#[0-9a-f]{6});/i + )?.[1].toLowerCase() + ); + expect(PALETTE_STYLES).toMatch( + /@mixin focus-ring-token\s*\{\s*--app-focus-ring:\s*#\{\$text\};/ + ); + expect(HOST_STYLES).toMatch(/@include palette\.focus-ring-token;/); + }); + it('tells a focused selected swatch from one that is only selected', () => { const selected = ruleBody( SETTINGS_STYLES, diff --git a/libs/ui/playback/src/lib/portal-inline-player/up-next-rail.component.scss b/libs/ui/playback/src/lib/portal-inline-player/up-next-rail.component.scss index 88a754d13..ebe7b5a13 100644 --- a/libs/ui/playback/src/lib/portal-inline-player/up-next-rail.component.scss +++ b/libs/ui/playback/src/lib/portal-inline-player/up-next-rail.component.scss @@ -6,6 +6,7 @@ // player's `--pc-progress`, so an episode's bar matches the dock's Up next card. :host { @include palette.progress-token; + @include palette.focus-ring-token; display: block; height: 100%; min-height: 0; diff --git a/libs/ui/playback/src/lib/vjs-player/vjs-player.component.scss b/libs/ui/playback/src/lib/vjs-player/vjs-player.component.scss index 8fde05522..3362c9df8 100644 --- a/libs/ui/playback/src/lib/vjs-player/vjs-player.component.scss +++ b/libs/ui/playback/src/lib/vjs-player/vjs-player.component.scss @@ -1,4 +1,8 @@ +@use '../player-controls/player-palette' as palette; + :host { + // Vendor chrome over video takes the player's focus ring. + @include palette.focus-ring-token; display: block; width: 100%; height: 100%; diff --git a/libs/ui/styles/_content-grid.scss b/libs/ui/styles/_content-grid.scss index 110373d9d..8158a3367 100644 --- a/libs/ui/styles/_content-grid.scss +++ b/libs/ui/styles/_content-grid.scss @@ -7,6 +7,8 @@ // @include grid.content-grid; // @include grid.content-card; +@use 'focus-ring'; + @mixin content-grid( $min-width: var(--cover-grid-min-width, 148px), $gap: var(--cover-gap, 16px) @@ -154,16 +156,16 @@ } } -// Draw the ring on the element that owns `overflow: hidden` (or on one of -// its ancestors): an outline on a clipped descendant is cut on every edge. -@mixin focus-ring-declarations { - outline: 2px solid var(--app-selection-color, #2f7bff); - outline-offset: 3px; +// The app focus ring at the cards' 3px offset. Draw it on the element that +// owns `overflow: hidden` (or on one of its ancestors): an outline on a +// clipped descendant is cut on every edge. +@mixin card-focus-ring-declarations { + @include focus-ring.focus-ring-declarations($offset: 3px); } @mixin card-focus-ring { &:focus-visible { - @include focus-ring-declarations; + @include card-focus-ring-declarations; } } diff --git a/libs/ui/styles/_detail-view-actions.scss b/libs/ui/styles/_detail-view-actions.scss index b5d126d84..7ad770356 100644 --- a/libs/ui/styles/_detail-view-actions.scss +++ b/libs/ui/styles/_detail-view-actions.scss @@ -1,3 +1,5 @@ +@use 'focus-ring'; + @mixin play-button-copy($meta-color: currentColor) { &__copy { display: inline-flex; @@ -343,8 +345,7 @@ } &:focus-visible { - outline: 2px solid var(--app-selection-color, #78adff); - outline-offset: 2px; + @include focus-ring.focus-ring-declarations; } &:disabled { @@ -382,8 +383,7 @@ } &:focus-visible { - outline: 2px solid var(--app-selection-color, #78adff); - outline-offset: 2px; + @include focus-ring.focus-ring-declarations; } } diff --git a/libs/ui/styles/_focus-ring.scss b/libs/ui/styles/_focus-ring.scss new file mode 100644 index 000000000..bc0204a70 --- /dev/null +++ b/libs/ui/styles/_focus-ring.scss @@ -0,0 +1,11 @@ +// The app's keyboard focus ring. `apps/web/src/styles.scss` draws it on +// every `:focus-visible` element that sets no outline of its own; components +// include it where the ring belongs on another element (the card that owns +// `overflow: hidden`, for one). `--app-focus-ring` is declared per theme in +// `apps/web/src/m3-theme.scss`; surfaces over video point it at the +// player's text colour (`_player-palette.scss`). +@mixin focus-ring-declarations($offset: 2px) { + outline: 2px solid + var(--app-focus-ring, var(--app-selection-color, #2f7bff)); + outline-offset: $offset; +} diff --git a/libs/ui/styles/_index.scss b/libs/ui/styles/_index.scss index 88a0b1947..500453ade 100644 --- a/libs/ui/styles/_index.scss +++ b/libs/ui/styles/_index.scss @@ -11,3 +11,4 @@ @forward 'detail-view'; @forward 'detail-view-actions'; @forward 'nav-list'; +@forward 'focus-ring'; diff --git a/libs/ui/styles/_portal-sidebar.scss b/libs/ui/styles/_portal-sidebar.scss index 9d5da1750..aedd5c499 100644 --- a/libs/ui/styles/_portal-sidebar.scss +++ b/libs/ui/styles/_portal-sidebar.scss @@ -6,6 +6,8 @@ // depth. No alias is needed because this partial exports no callable members. // @use '../../../../../../ui/styles/portal-sidebar'; +@use 'focus-ring'; + .sidebar { // Width is controlled by the resizable directive // Default fallback if directive not applied @@ -126,8 +128,7 @@ } &:focus-visible { - outline: 2px solid var(--mat-sys-primary); - outline-offset: -2px; + @include focus-ring.focus-ring-declarations($offset: -2px); } } } diff --git a/libs/workspace/shell/feature/src/lib/workspace-command-palette/workspace-command-palette.component.scss b/libs/workspace/shell/feature/src/lib/workspace-command-palette/workspace-command-palette.component.scss index 9585c09df..8185789b2 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-command-palette/workspace-command-palette.component.scss +++ b/libs/workspace/shell/feature/src/lib/workspace-command-palette/workspace-command-palette.component.scss @@ -25,6 +25,12 @@ padding: 0 14px; min-height: 44px; cursor: text; + + // The field draws no outline of its own, so the row shows its focus: an + // underline in the focus ring colour (inset: the shell clips its edges). + &:focus-within { + box-shadow: inset 0 -2px 0 var(--app-focus-ring); + } } .palette-search__icon { diff --git a/libs/workspace/shell/feature/src/lib/workspace-context-panel/workspace-context-panel.component.scss b/libs/workspace/shell/feature/src/lib/workspace-context-panel/workspace-context-panel.component.scss index 07c02f2a2..ce08654ed 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-context-panel/workspace-context-panel.component.scss +++ b/libs/workspace/shell/feature/src/lib/workspace-context-panel/workspace-context-panel.component.scss @@ -1,3 +1,5 @@ +@use '../../../../../../ui/styles/focus-ring'; + :host { display: flex; flex-direction: column; @@ -289,8 +291,7 @@ app-workspace-context-category-view { &:focus-visible { color: var(--app-heading-color); - outline: 2px solid var(--app-selection-color); - outline-offset: -2px; + @include focus-ring.focus-ring-declarations($offset: -2px); } mat-icon { diff --git a/libs/workspace/shell/feature/src/lib/workspace-context-panel/workspace-settings-context-panel.component.scss b/libs/workspace/shell/feature/src/lib/workspace-context-panel/workspace-settings-context-panel.component.scss index 85044b901..f90beb0be 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-context-panel/workspace-settings-context-panel.component.scss +++ b/libs/workspace/shell/feature/src/lib/workspace-context-panel/workspace-settings-context-panel.component.scss @@ -1,3 +1,5 @@ +@use '../../../../../../ui/styles/focus-ring'; + // Settings navigation. Not the shared nav-list: the active item here is a // soft fill with tinted text and no border, the same treatment as a // selected channel row, so the list does not shift by the 2px an outlined @@ -95,9 +97,9 @@ color: var(--mat-sys-on-surface); } + // Inside the item, so the ring on the active item sits on its tint. &:focus-visible { - outline: 2px solid var(--app-selection-color); - outline-offset: -2px; + @include focus-ring.focus-ring-declarations($offset: -2px); } &.active { diff --git a/package.json b/package.json index 028ca4530..3c6b94f49 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,9 @@ "styles:material-tokens:test": "node --test tools/nx/check-material-token-overrides.test.mjs", "styles:material-tokens:check": "node tools/nx/check-material-token-overrides.mjs", "styles:material-tokens:validate": "pnpm run styles:material-tokens:test && pnpm run styles:material-tokens:check", + "styles:focus-visible:test": "node --test tools/nx/check-focus-visible.test.mjs", + "styles:focus-visible:check": "node tools/nx/check-focus-visible.mjs", + "styles:focus-visible:validate": "pnpm run styles:focus-visible:test && pnpm run styles:focus-visible:check", "styles:icon-ligatures:test": "node --test tools/nx/check-icon-ligatures.test.mjs", "styles:icon-ligatures:check": "node tools/nx/check-icon-ligatures.mjs", "styles:icon-ligatures:validate": "pnpm run styles:icon-ligatures:test && pnpm run styles:icon-ligatures:check", diff --git a/tools/nx/check-focus-visible.mjs b/tools/nx/check-focus-visible.mjs new file mode 100644 index 000000000..e051424bd --- /dev/null +++ b/tools/nx/check-focus-visible.mjs @@ -0,0 +1,319 @@ +import { execFileSync } from 'node:child_process'; +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { stripScssComments } from './check-stylesheet-inputs.mjs'; + +/** + * The web app's global stylesheets used to remove the outline from every + * `input`, `button`, `textarea` and `:focus`, so keyboard users saw no focus + * at all. A global rule may remove the outline only where focus is not + * visible (`:focus:not(:focus-visible)`), or on an element the selector + * scopes by class, id or attribute; and a bare `:focus-visible` rule must + * draw the fallback ring. Component styles are out of scope: Angular scopes + * them, and they may replace the ring with an indicator of their own. + */ + +/** The global entry stylesheets and the partials beside them. */ +const SCANNED_PATHSPECS = [':(glob)apps/web/src/*.scss']; + +const OUTLINE_DECLARATION = + /^(outline(?:-style|-width|-color)?)\s*:\s*(.+?)\s*(?:!important)?$/i; + +/** A zero length: `0`, `0px`, `0.0em`. */ +const ZERO_WIDTH = /^[+-]?0*\.?0+(?:[a-z]+)?$/i; +/** `none`, `hidden`, or a keyword that resets the outline to `none`. */ +const NO_STYLE = /^(?:none|hidden|initial|unset)$/i; +/** A style that draws, or a `var()` that may carry one. */ +const DRAWN_STYLE = + /^(?:auto|solid|dashed|dotted|double|groove|ridge|inset|outset|inherit|revert|revert-layer|var\(.*\))$/i; +/** `transparent`, or a colour whose alpha (4th or slashed) is zero. */ +const NO_COLOR = + /^(?:transparent|#[0-9a-f]{3}0|#[0-9a-f]{6}00|(?:rgb|hsl)a?\((?:[^,()]+,){3}\s*0*\.?0+%?\s*\)|(?:rgb|hsl)a?\([^/()]*\/\s*0*\.?0+%?\s*\))$/i; + +/** Splits `text` on `separator` outside parentheses and brackets. */ +function splitTopLevel(text, separator) { + const parts = []; + let depth = 0; + let start = 0; + for (let i = 0; i < text.length; i += 1) { + const char = text[i]; + if (char === '(' || char === '[') depth += 1; + else if (char === ')' || char === ']') depth -= 1; + else if (depth === 0 && separator.test(char)) { + parts.push(text.slice(start, i)); + start = i + 1; + } + } + parts.push(text.slice(start)); + return parts.map((part) => part.trim()).filter(Boolean); +} + +/** Resolves a nested selector list against its parent selectors. */ +function resolveSelectors(parents, selectorList) { + const own = splitTopLevel(selectorList, /,/); + if (parents.length === 0) return own; + return parents.flatMap((parent) => + own.map((selector) => + selector.includes('&') + ? selector.replaceAll('&', parent) + : `${parent} ${selector}` + ) + ); +} + +/** + * Whether an `outline*` declaration leaves no visible outline: a `none` or + * `hidden` style, a zero width or a transparent colour, in the longhands or + * anywhere in the shorthand (`outline: 0 solid transparent`). A shorthand + * without a style (`outline: 2px`, `outline: red`) resets it to `none`. + */ +export function removesOutline(declaration) { + const match = OUTLINE_DECLARATION.exec(declaration); + if (!match) return false; + const property = match[1].toLowerCase(); + const tokens = splitTopLevel(match[2], /\s/); + const invisible = { + 'outline-style': (token) => NO_STYLE.test(token), + 'outline-width': (token) => ZERO_WIDTH.test(token), + 'outline-color': (token) => NO_COLOR.test(token), + outline: (token) => + NO_STYLE.test(token) || + ZERO_WIDTH.test(token) || + NO_COLOR.test(token), + }[property]; + const styleless = + property === 'outline' && + !tokens.some((token) => DRAWN_STYLE.test(token)); + return styleless || tokens.some(invisible); +} + +/** Whether a declaration draws a visible outline. */ +function drawsOutline(declaration) { + return /^outline\s*:/i.test(declaration) && !removesOutline(declaration); +} + +/** Expands `:is()` / `:where()` / `:matches()` lists into plain branches. */ +export function expandBranches(selector) { + const match = /:(?:is|where|matches)\(/i.exec(selector); + if (!match) return [selector]; + const open = match.index + match[0].length; + let depth = 1; + let close = open; + while (close < selector.length && depth > 0) { + if (selector[close] === '(') depth += 1; + else if (selector[close] === ')') depth -= 1; + close += 1; + } + const head = selector.slice(0, match.index); + const tail = selector.slice(close); + return splitTopLevel(selector.slice(open, close - 1), /,/).flatMap( + (branch) => expandBranches(`${head}${branch}${tail}`) + ); +} + +/** The last compound selector, the element the rule actually styles. */ +function subjectOf(selector) { + const compounds = splitTopLevel(selector, /[\s>+~]/); + return compounds[compounds.length - 1] ?? ''; +} + +/** A compound without its `:not(...)` arguments. */ +function withoutNegations(compound) { + let out = ''; + for (let i = 0; i < compound.length; i += 1) { + if (/^:not\(/i.test(compound.slice(i))) { + let depth = 0; + for (; i < compound.length; i += 1) { + if (compound[i] === '(') depth += 1; + else if (compound[i] === ')' && --depth === 0) break; + } + } else { + out += compound[i]; + } + } + return out; +} + +/** + * Whether removing the outline here strips it from elements nothing else + * styles: the subject has no class, id or attribute of its own, and does + * not limit the removal to focus that is not visible. + */ +export function isBlanketSelector(selector) { + return expandBranches(selector).some((branch) => { + const subject = subjectOf(branch); + if (subject.includes('::')) return false; + if (/:not\(\s*:focus-visible\s*\)/i.test(subject)) return false; + return !/[.#[]/.test(withoutNegations(subject)); + }); +} + +/** + * Whether a selector is the global `:focus-visible` fallback: the whole + * selector, not only its subject, so `.panel :focus-visible` (which leaves + * everything outside the panel without a ring) is not one. + */ +function isFallbackSelector(selector) { + return expandBranches(selector).some((branch) => + /^\*?:focus-visible$/i.test(branch.replace(/\s+/g, '')) + ); +} + +/** One line number per offset, for reports. */ +function lineAt(source, index) { + let line = 1; + for (let i = 0; i < index; i += 1) if (source[i] === '\n') line += 1; + return line; +} + +/** + * Walks the rules of a stylesheet: each declaration and `@include` with the + * fully resolved selectors it applies to. Mixin bodies are walked as if they + * were included at the top level, the widest place a caller could use them. + * `conditional` marks a declaration inside an at-rule (`@mixin`, `@media`, + * `@if`, an `@include` content block…), which may never reach the page. + */ +export function walkDeclarations(source) { + const text = stripScssComments(source); + const items = []; + const stack = [{ selectors: [], conditional: false }]; + let start = 0; + let quote = ''; + let interpolation = 0; + const statement = (end) => { + const raw = text.slice(start, end); + const value = raw.trim(); + const offset = start + raw.indexOf(value); + start = end + 1; + return { value, offset }; + }; + for (let i = 0; i < text.length; i += 1) { + const char = text[i]; + if (quote) { + if (char === '\\') i += 1; + else if (char === quote) quote = ''; + continue; + } + if (char === '"' || char === "'") quote = char; + else if (text.startsWith('#{', i)) { + interpolation += 1; + i += 1; + } else if (interpolation > 0 && char === '}') interpolation -= 1; + else if (interpolation > 0) continue; + else if (char === '{') { + const { value } = statement(i); + const parent = stack[stack.length - 1]; + if (/^@mixin\b/i.test(value)) + stack.push({ selectors: [], conditional: true }); + else if (value.startsWith('@')) + stack.push({ selectors: parent.selectors, conditional: true }); + else + stack.push({ + selectors: resolveSelectors(parent.selectors, value), + conditional: parent.conditional, + }); + } else if (char === ';' || char === '}') { + const { value, offset } = statement(i); + if (value) { + const { selectors, conditional } = stack[stack.length - 1]; + items.push({ + selectors, + conditional, + declaration: value.replace(/\s+/g, ' '), + line: lineAt(text, offset), + }); + } + if (char === '}' && stack.length > 1) stack.pop(); + } + } + return items; +} + +/** Blanket outline removals in one stylesheet. */ +export function findBlanketOutlineRemovals(file, source) { + return walkDeclarations(source).flatMap( + ({ selectors, declaration, line }) => + removesOutline(declaration) + ? selectors.filter(isBlanketSelector).map((selector) => ({ + file, + line, + selector, + declaration, + })) + : [] + ); +} + +/** + * Whether a stylesheet always draws the bare `:focus-visible` fallback ring: + * an unconditional rule, not one in a mixin body nobody may include or under + * a media query. + */ +export function hasFocusVisibleFallback(source) { + return walkDeclarations(source).some( + ({ selectors, conditional, declaration }) => + !conditional && + selectors.some(isFallbackSelector) && + (drawsOutline(declaration) || + /^@include\s+[\w.-]*focus-ring/i.test(declaration)) + ); +} + +/** Tracked global stylesheets under `rootDir`. */ +export function listScannedFiles(rootDir) { + // No shell: `cmd.exe` treats single quotes as literal characters, so a + // POSIX-quoted pathspec reaches git intact on Windows and matches nothing. + return execFileSync('git', ['ls-files', ...SCANNED_PATHSPECS], { + cwd: rootDir, + encoding: 'utf8', + }) + .trim() + .split('\n') + .filter(Boolean); +} + +/** Every problem across the global stylesheets, as printable lines. */ +export function checkFocusVisible(sources) { + const problems = sources.flatMap(({ file, source }) => + findBlanketOutlineRemovals(file, source).map( + ({ file: at, line, selector, declaration }) => + `${at}:${line} \`${selector}\` sets \`${declaration}\`` + ) + ); + if (!sources.some(({ source }) => hasFocusVisibleFallback(source))) { + problems.push( + 'No global `:focus-visible` rule draws the fallback outline (`@include focus-ring.focus-ring-declarations`).' + ); + } + return problems; +} + +const isMain = + process.argv[1] && + path.resolve(process.argv[1]) === + path.resolve(fileURLToPath(import.meta.url)); + +if (isMain) { + const rootDir = process.cwd(); + const files = listScannedFiles(rootDir); + const sources = await Promise.all( + files.map(async (file) => ({ + file, + source: await readFile(path.join(rootDir, file), 'utf8'), + })) + ); + const problems = checkFocusVisible(sources); + if (problems.length > 0) { + console.error( + 'Global styles hide keyboard focus. Remove the outline only under `:focus:not(:focus-visible)` or on a scoped selector, and keep the `:focus-visible` fallback ring:' + ); + for (const problem of problems) console.error(`- ${problem}`); + process.exitCode = 1; + } else { + console.log( + `Checked ${files.length} global stylesheets; keyboard focus keeps its fallback ring.` + ); + } +} diff --git a/tools/nx/check-focus-visible.test.mjs b/tools/nx/check-focus-visible.test.mjs new file mode 100644 index 000000000..01262ff22 --- /dev/null +++ b/tools/nx/check-focus-visible.test.mjs @@ -0,0 +1,287 @@ +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test } from 'node:test'; + +import { + checkFocusVisible, + expandBranches, + findBlanketOutlineRemovals, + hasFocusVisibleFallback, + isBlanketSelector, + removesOutline, + listScannedFiles, +} from './check-focus-visible.mjs'; + +const rootDir = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + '../..' +); + +const fallback = [ + ':where(:focus:not(:focus-visible)) {', + ' outline: none;', + '}', + ':where(:focus-visible) {', + ' @include focus-ring.focus-ring-declarations;', + '}', +].join('\n'); + +test('reports the old blanket rule once per unscoped selector', () => { + const source = [ + 'input,', + 'button,', + 'textarea,', + ':focus {', + ' outline: none;', + '}', + ].join('\n'); + + assert.deepEqual( + findBlanketOutlineRemovals('styles.scss', source).map( + ({ line, selector }) => `${line} ${selector}` + ), + ['5 input', '5 button', '5 textarea', '5 :focus'] + ); +}); + +test('reports every way of hiding the outline, nested or not', () => { + const source = [ + '*:focus { outline: 0 !important; }', + 'button:focus { outline-style: none; }', + 'a { outline-width: 0; }', + '[tabindex] :focus { outline-color: transparent; }', + '.dark-theme {', + ' :focus { outline: 0px; }', + ' @media (min-width: 600px) {', + ' & :focus-visible { outline: none; }', + ' }', + '}', + '@mixin hide { :focus { outline: none; } }', + ].join('\n'); + + assert.deepEqual( + findBlanketOutlineRemovals('a.scss', source).map( + ({ selector }) => selector + ), + [ + '*:focus', + 'button:focus', + 'a', + '[tabindex] :focus', + '.dark-theme :focus', + '.dark-theme :focus-visible', + ':focus', + ] + ); +}); + +test('reads an invisible outline from any part of the shorthand', () => { + for (const hidden of [ + 'outline: 0 solid transparent', + 'outline: 2px solid transparent', + 'outline: 1px solid rgba(0, 0, 0, 0)', + 'outline: 1px solid rgb(0 0 0 / 0%)', + 'outline: thin dotted #0000', + 'outline: 0.0em solid red', + 'outline-color: transparent', + 'outline-width: 0px', + 'outline-style: hidden', + // A shorthand without a style resets it to `none`. + 'outline: 2px', + 'outline: 2px #1d63e0', + 'outline: red', + 'outline: initial', + 'outline-style: unset', + ]) { + assert.equal(removesOutline(hidden), true, hidden); + } + for (const visible of [ + 'outline: 2px solid var(--app-focus-ring)', + // A zero blue channel is not a zero alpha. + 'outline: 2px solid rgb(255, 0, 0)', + 'outline: 2px solid rgba(0, 0, 0, 0.5)', + 'outline: 2px solid #ff000080', + 'outline: auto', + 'outline: dashed', + 'outline: var(--app-focus-outline)', + 'outline-offset: 0', + ]) { + assert.equal(removesOutline(visible), false, visible); + } + + // Both checks use it: a transparent ring hides focus and is no fallback. + const problems = checkFocusVisible([ + { + file: 'f.scss', + source: [ + ':focus { outline: 0 solid transparent; }', + ':focus-visible { outline: 2px solid transparent; }', + ':focus-visible { outline: 2px; }', + ].join('\n'), + }, + ]); + assert.deepEqual(problems, [ + 'f.scss:1 `:focus` sets `outline: 0 solid transparent`', + 'f.scss:2 `:focus-visible` sets `outline: 2px solid transparent`', + 'f.scss:3 `:focus-visible` sets `outline: 2px`', + 'No global `:focus-visible` rule draws the fallback outline (`@include focus-ring.focus-ring-declarations`).', + ]); +}); + +test('accepts removals that keep visible focus or target scoped elements', () => { + const source = [ + ':where(:focus:not(:focus-visible)) { outline: none; }', + ':focus:not(:focus-visible) { outline: 0; }', + '.setting-item:focus { outline: none; }', + 'input.search-field { outline: none; }', + 'button[mat-icon-button]:focus { outline: none; }', + '#player:focus { outline: none; }', + '.panel { &:focus { outline: none; } }', + '::-webkit-scrollbar { outline: none; }', + ':focus-visible { outline: 2px solid red; }', + ].join('\n'); + + assert.deepEqual(findBlanketOutlineRemovals('b.scss', source), []); +}); + +test('expands :is() and :where() so one unscoped branch is still caught', () => { + assert.deepEqual(expandBranches(':is(input, .x):focus'), [ + 'input:focus', + '.x:focus', + ]); + assert.equal(isBlanketSelector(':is(input, .x):focus'), true); + assert.equal(isBlanketSelector(':where(.x, [data-y]):focus'), false); + // A class inside :not() does not scope the element. + assert.equal(isBlanketSelector('button:not(.mat-button)'), true); +}); + +test('ignores commented-out rules and strings that look like rules', () => { + const source = [ + '// :focus { outline: none; }', + '/* button { outline: 0; } */', + ".icon { content: '{ outline: none; }'; }", + fallback, + ].join('\n'); + + assert.deepEqual(checkFocusVisible([{ file: 'c.scss', source }]), []); +}); + +test('requires a :focus-visible fallback that draws an outline', () => { + const missing = checkFocusVisible([ + { + file: 'd.scss', + source: ':where(:focus:not(:focus-visible)) { outline: none; }', + }, + ]); + assert.equal(missing.length, 1); + assert.match(missing[0], /No global `:focus-visible` rule/); + + assert.equal(hasFocusVisibleFallback(fallback), true); + assert.equal( + hasFocusVisibleFallback( + ':focus-visible { outline: 2px solid var(--ring); }' + ), + true + ); + // A scoped rule is not the global fallback: not on the focused element, + // not on a container's descendants, not nested in one. + for (const scoped of [ + '.card:focus-visible { outline: 2px solid red; }', + '.panel :focus-visible { outline: 2px solid red; }', + '.dark-theme { :focus-visible { outline: 2px solid red; } }', + '.panel { :where(:focus-visible) { @include focus-ring.ring; } }', + ]) { + assert.equal(hasFocusVisibleFallback(scoped), false, scoped); + } + // Nor is one that may never reach the page. + for (const conditional of [ + '@mixin ring { :focus-visible { outline: 2px solid red; } }', + '@media print { :focus-visible { outline: 2px solid red; } }', + ]) { + assert.equal(hasFocusVisibleFallback(conditional), false, conditional); + } + assert.equal( + hasFocusVisibleFallback('*:focus-visible { outline: 2px solid red; }'), + true + ); + // Neither is one that removes the outline, which is also reported. + const removed = checkFocusVisible([ + { file: 'e.scss', source: ':focus-visible { outline: none; }' }, + ]); + assert.equal(removed.length, 2); + assert.match( + removed[0], + /^e\.scss:1 `:focus-visible` sets `outline: none`$/ + ); +}); + +test('the fallback may live in another scanned stylesheet', () => { + assert.deepEqual( + checkFocusVisible([ + { + file: 'apps/web/src/m3-theme.scss', + source: 'html { color: red; }', + }, + { file: 'apps/web/src/styles.scss', source: fallback }, + ]), + [] + ); +}); + +test('selects only the tracked top-level web stylesheets', async () => { + const tempDir = await mkdtemp( + path.join(os.tmpdir(), 'focus-visible-guard-') + ); + const git = (...args) => + execFileSync('git', args, { cwd: tempDir, stdio: 'pipe' }); + try { + git('init', '-q'); + const files = [ + 'apps/web/src/styles.scss', + 'apps/web/src/_settings-theme.scss', + 'apps/web/src/app/settings/settings.component.scss', + 'apps/web/src/main.ts', + 'libs/ui/styles/_focus-ring.scss', + ]; + for (const file of files) { + await mkdir(path.dirname(path.join(tempDir, file)), { + recursive: true, + }); + await writeFile(path.join(tempDir, file), ''); + } + git('add', '.'); + await writeFile(path.join(tempDir, 'apps/web/src/untracked.scss'), ''); + + assert.deepEqual(listScannedFiles(tempDir).sort(), [ + 'apps/web/src/_settings-theme.scss', + 'apps/web/src/styles.scss', + ]); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } +}); + +test('the repository global stylesheets pass, and the old rule would not', async () => { + const sources = await Promise.all( + listScannedFiles(rootDir).map(async (file) => ({ + file, + source: await readFile(path.join(rootDir, file), 'utf8'), + })) + ); + assert.ok(sources.some(({ file }) => file === 'apps/web/src/styles.scss')); + assert.deepEqual(checkFocusVisible(sources), []); + + const reverted = sources.map(({ file, source }) => + file === 'apps/web/src/styles.scss' + ? { + file, + source: `${source}\ninput, button, textarea, :focus { outline: none; }\n`, + } + : { file, source } + ); + assert.equal(checkFocusVisible(reverted).length, 4); +});