From ea7efdbd9be727aa596d16c9311e9439482239bd Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sun, 11 Oct 2026 08:15:04 +0200 Subject: [PATCH] fix(workspace): open a source row from the keyboard (#1885) A Sources row opened only on a pointer click: the row div was not focusable and had no key handler, so a keyboard user could Tab to its actions but never open the playlist. Following app-content-card, the row's title and meta block becomes the keyboard activation element: role="button", tabindex="0", named by the source title, aria-current on the active source, aria-disabled while busy. Enter and Space open the source (Space prevents the page scroll). The drag handle, health indicator and actions stay its siblings, so no control is nested inside a role="button" and their keys never bubble into it. A click anywhere on the row still opens it via the row. The row draws the ring with the shared focus-ring mixin, inset, because the list's scroller would cut a ring drawn outside the full-width row. playlist-shared-ui now declares its ui-styles stylesheet dependency. Tests: unit spec for Enter/Space, bubbling from an action button, busy and selected states; Electron E2E tabs to a source row, checks the ring and opens it with Enter (fails on the previous component). Co-authored-by: Claude Opus 5.5 --- .changes/workspace-sources-keyboard-open.md | 8 + .../src/sources-keyboard.e2e.ts | 175 ++++++++++++++++++ docs/architecture/iptvnator-ui-guidelines.md | 16 ++ libs/playlist/shared/ui/project.json | 1 + .../playlist-item.component.html | 15 +- .../playlist-item.component.scss | 10 + .../playlist-item.component.spec.ts | 114 ++++++++++++ .../playlist-item/playlist-item.component.ts | 11 ++ 8 files changed, 349 insertions(+), 1 deletion(-) create mode 100644 .changes/workspace-sources-keyboard-open.md create mode 100644 apps/electron-backend-e2e/src/sources-keyboard.e2e.ts diff --git a/.changes/workspace-sources-keyboard-open.md b/.changes/workspace-sources-keyboard-open.md new file mode 100644 index 000000000..06c7ce752 --- /dev/null +++ b/.changes/workspace-sources-keyboard-open.md @@ -0,0 +1,8 @@ +--- +type: fix +area: workspace +--- + +Sources can now be opened from the keyboard: on the Sources page, Tab stops +on each source row before its buttons, and Enter or Space opens it just like +a click. The focused row shows the focus ring. diff --git a/apps/electron-backend-e2e/src/sources-keyboard.e2e.ts b/apps/electron-backend-e2e/src/sources-keyboard.e2e.ts new file mode 100644 index 000000000..83dc3802d --- /dev/null +++ b/apps/electron-backend-e2e/src/sources-keyboard.e2e.ts @@ -0,0 +1,175 @@ +import type { Page } from '@playwright/test'; +import { + closeElectronApp, + expect, + launchElectronApp, + openSources, + sourceRowByTitle, + test, + waitForM3uCatalog, + writeTemporaryM3uFile, +} from './electron-test-fixtures'; + +// --------------------------------------------------------------------------- +// A source row opens from the keyboard. Each row has its own Tab stop: the +// title and meta block (`role="button"`, named after the source). The drag +// handle and the row actions are its siblings, so Tab reaches the row before +// its actions, Enter opens it like a click, and the row draws one ring +// while that element has keyboard focus. +// --------------------------------------------------------------------------- + +type RowRing = { + focusVisible: boolean; + /** The focused element's own outline: the row draws its ring. */ + ownOutline: string; + row: { style: string; width: string; offset: string; color: string }; + /** `--app-focus-ring` resolved where the row sits. */ + token: string; +}; + +/** Reads the ring of the row that holds keyboard focus. */ +function readRowRing(page: Page): Promise { + return page.evaluate(() => { + const focused = document.activeElement as HTMLElement; + const row = focused.closest('.playlist-item') as HTMLElement; + const probe = document.createElement('span'); + probe.style.color = 'var(--app-focus-ring)'; + row.append(probe); + const token = getComputedStyle(probe).color; + probe.remove(); + const style = getComputedStyle(row); + return { + focusVisible: focused.matches(':focus-visible'), + ownOutline: getComputedStyle(focused).outlineStyle, + row: { + style: style.outlineStyle, + width: style.outlineWidth, + offset: style.outlineOffset, + color: style.outlineColor, + }, + token, + }; + }); +} + +/** Describes the focused element, for a failure message. */ +function describeFocus(page: Page): Promise { + return page.evaluate(() => { + const el = document.activeElement; + if (!el) return '(none)'; + const label = (el.getAttribute('aria-label') ?? el.textContent ?? '') + .replace(/\s+/g, ' ') + .trim() + .slice(0, 40); + return `${el.tagName.toLowerCase()}.${[...el.classList].join('.')} "${label}"`; + }); +} + +test.describe('Electron Sources keyboard', () => { + test('@m3u @electron Tab reaches a source row before its actions, shows a ring, and Enter switches to it', async ({ + dataDir, + }) => { + const alpha = writeTemporaryM3uFile(dataDir, 'keyboard-alpha.m3u', [ + { + groupTitle: 'News', + name: 'Keyboard Alpha News', + url: 'https://streams.example.test/keyboard-alpha.m3u8', + }, + ]); + const bravo = writeTemporaryM3uFile(dataDir, 'keyboard-bravo.m3u', [ + { + groupTitle: 'Sports', + name: 'Keyboard Bravo Sports', + url: 'https://streams.example.test/keyboard-bravo.m3u8', + }, + ]); + const app = await launchElectronApp(dataDir, { + appArgs: [alpha, bravo], + }); + const page = app.mainWindow; + + try { + await page.waitForURL(/\/workspace\/playlists\/.+/, { + timeout: 30_000, + }); + await openSources(page); + await expect(page.locator('app-playlist-item')).toHaveCount(2, { + timeout: 30_000, + }); + // Bravo opened last, so it is the active source; open the other. + const row = sourceRowByTitle(page, 'keyboard-alpha').first(); + await expect(row.locator('.playlist-item')).not.toHaveClass( + /\bselected\b/ + ); + const title = ( + await row.locator('.playlist-title').textContent() + )?.trim(); + expect(title).toBeTruthy(); + + // Enter the page from the keyboard ahead of the list, then Tab + // until focus lands in the row. + const start = page + .locator('app-workspace-sources-filters-panel button') + .first(); + await start.focus(); + await page.keyboard.press('Tab'); + await page.keyboard.press('Shift+Tab'); + await expect(start).toBeFocused(); + const visited: string[] = []; + for (let presses = 0; presses < 40; presses++) { + await page.keyboard.press('Tab'); + visited.push(await describeFocus(page)); + const inRow = await row.evaluate((el) => + el.contains(document.activeElement) + ); + if (inRow) break; + } + + // The row's first Tab stop opens it; its actions come after. + const open = row.getByRole('button', { + name: title, + exact: true, + }); + await expect( + open, + `Tab stops up to the row: ${visited.join(' → ')}` + ).toBeFocused(); + + const focused = await readRowRing(page); + expect(focused).toEqual({ + focusVisible: true, + ownOutline: 'none', + row: { + style: 'solid', + width: '2px', + offset: '-2px', + color: focused.token, + }, + token: expect.stringMatching(/^rgb/), + }); + + // The ring follows focus: it leaves with it. + await page.keyboard.press('Tab'); + await expect(open).not.toBeFocused(); + expect( + await row + .locator('.playlist-item') + .evaluate((el) => getComputedStyle(el).outlineStyle) + ).toBe('none'); + await page.keyboard.press('Shift+Tab'); + await expect(open).toBeFocused(); + + await page.keyboard.press('Enter'); + await waitForM3uCatalog(page); + const channels = page.getByTestId('channel-item'); + await expect( + channels.filter({ hasText: 'Keyboard Alpha News' }) + ).toBeVisible(); + await expect( + channels.filter({ hasText: 'Keyboard Bravo Sports' }) + ).toHaveCount(0); + } finally { + await closeElectronApp(app); + } + }); +}); diff --git a/docs/architecture/iptvnator-ui-guidelines.md b/docs/architecture/iptvnator-ui-guidelines.md index ae262cca8..8c79228c9 100644 --- a/docs/architecture/iptvnator-ui-guidelines.md +++ b/docs/architecture/iptvnator-ui-guidelines.md @@ -1252,6 +1252,22 @@ A visual change is not done until: 4. Scroll behavior is correct. 5. The result was checked in the running app for layout-sensitive work. +### Source row keyboard access + +A Sources row (`app-playlist-item`) opens on a click anywhere on it, and it +also holds a drag handle, a health indicator and actions. Like +`app-content-card`, it opens from the keyboard through a dedicated element +that is a sibling of those controls: the title and meta block +`.playlist-content` (`role="button"`, `tabindex="0"`, named by the source +title, `aria-current` on the active source, `aria-disabled` while busy). +Enter and Space open the source (Space prevents the page scroll), and Tab +reaches each row before its actions. Never put `role="button"` or key +handlers on the whole row: the actions would sit inside a button, and their +Enter and Space would bubble into it. The row draws the ring for that +element, inset (`$offset: -2px`), because the list's scroller would cut a +ring drawn outside the full-width row. `sources-keyboard.e2e.ts` (Electron) +tabs to a row, checks its ring and opens the source with Enter. + ### Network source indicators (Electron) The switcher and source rows use `SourceHealthIndicatorComponent`: green means diff --git a/libs/playlist/shared/ui/project.json b/libs/playlist/shared/ui/project.json index 81a410ea8..632af926b 100644 --- a/libs/playlist/shared/ui/project.json +++ b/libs/playlist/shared/ui/project.json @@ -5,6 +5,7 @@ "prefix": "lib", "projectType": "library", "tags": ["scope:playlist", "domain:m3u", "type:ui"], + "implicitDependencies": ["ui-styles"], "targets": { "test": { "executor": "@nx/jest:jest", diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.html b/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.html index 2037eb452..ff2d1ef16 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.html +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.html @@ -82,7 +82,20 @@ } -
+ +
{{ item.title || item.filename }}
diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.scss b/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.scss index 76b9f77a4..3129a13fa 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.scss +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.scss @@ -1,3 +1,5 @@ +@use '../../../../../../../ui/styles/focus-ring'; + :host { display: block; width: 100%; @@ -75,6 +77,14 @@ flex-direction: column; justify-content: center; gap: 2px; + // The row draws this element's focus ring. + outline: none; +} + +// The keyboard ring belongs to the whole row, the unit that opens. It sits +// inside the row: the list's scroller would cut one drawn outside it. +.playlist-item:has(> .playlist-content:focus-visible) { + @include focus-ring.focus-ring-declarations($offset: -2px); } .playlist-title { diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.spec.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.spec.ts index 064b3dae4..2ba2c712b 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.spec.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.spec.ts @@ -91,6 +91,120 @@ describe('PlaylistItemComponent', () => { expect(emitSpy).not.toHaveBeenCalled(); }); + describe('keyboard activation', () => { + const row = () => fixture.nativeElement as HTMLElement; + const activation = () => + row().querySelector('[role="button"]') as HTMLElement; + const keydown = (target: HTMLElement, key: string) => { + const event = new KeyboardEvent('keydown', { + key, + bubbles: true, + cancelable: true, + }); + target.dispatchEvent(event); + return event; + }; + + it('is a Tab stop named after the source that Enter and Space open', () => { + const emitSpy = jest.spyOn(component.playlistClicked, 'emit'); + const surface = activation(); + + expect(surface).not.toBeNull(); + expect(surface.getAttribute('tabindex')).toBe('0'); + expect(surface.getAttribute('aria-label')).toBe('Playlist'); + + const enter = keydown(surface, 'Enter'); + const space = keydown(surface, ' '); + + expect(emitSpy).toHaveBeenNthCalledWith(1, '1'); + expect(emitSpy).toHaveBeenNthCalledWith(2, '1'); + expect(enter.defaultPrevented).toBe(true); + // Space must not scroll the page. + expect(space.defaultPrevented).toBe(true); + }); + + it('keeps the drag handle and actions outside the activation element so their keys never open the source', () => { + fixture.componentRef.setInput('isDraggable', true); + fixture.detectChanges(); + const emitSpy = jest.spyOn(component.playlistClicked, 'emit'); + const editSpy = jest.spyOn(component.editPlaylistClicked, 'emit'); + const controls = Array.from( + row().querySelectorAll( + '.action-buttons button, .drag-icon' + ) + ); + + expect(activation()).not.toBeNull(); + expect(controls.length).toBeGreaterThan(1); + // No interactive control nested inside a role="button". + for (const control of controls) { + expect(control.closest('[role="button"]')).toBeNull(); + } + + const editButton = row().querySelector( + '.edit-btn' + ) as HTMLButtonElement; + keydown(editButton, 'Enter'); + const space = keydown(editButton, ' '); + editButton.click(); + + // The button's own activation edits; the row must neither open + // the source nor swallow the Space the button relies on. + expect(emitSpy).not.toHaveBeenCalled(); + expect(space.defaultPrevented).toBe(false); + expect(editSpy).toHaveBeenCalledTimes(1); + }); + + it('opens once for a pointer click anywhere on the row', () => { + const emitSpy = jest.spyOn(component.playlistClicked, 'emit'); + + activation().click(); + (row().querySelector('.playlist-item') as HTMLElement).click(); + (row().querySelector('.playlist-icon') as HTMLElement).click(); + + expect(emitSpy).toHaveBeenCalledTimes(3); + }); + + it('announces the selected source and ignores keys while busy', () => { + const emitSpy = jest.spyOn(component.playlistClicked, 'emit'); + expect(activation().hasAttribute('aria-current')).toBe(false); + expect(activation().hasAttribute('aria-disabled')).toBe(false); + + fixture.componentRef.setInput('isSelected', true); + fixture.componentRef.setInput('isRefreshing', true); + fixture.detectChanges(); + + expect(activation().getAttribute('aria-current')).toBe('true'); + expect(activation().getAttribute('aria-disabled')).toBe('true'); + // Still a Tab stop, so focus is not lost when a refresh starts. + expect(activation().getAttribute('tabindex')).toBe('0'); + + const space = keydown(activation(), ' '); + keydown(activation(), 'Enter'); + + expect(emitSpy).not.toHaveBeenCalled(); + expect(space.defaultPrevented).toBe(true); + }); + + it('falls back to the file name for the accessible name', () => { + fixture.destroy(); + fixture = TestBed.createComponent(PlaylistItemComponent); + fixture.componentInstance.item = { + title: '', + filename: 'channels.m3u', + _id: 'file-source', + count: 10, + importDate: Date.now().toString(), + autoRefresh: false, + }; + fixture.detectChanges(); + + expect(activation().getAttribute('aria-label')).toBe( + 'channels.m3u' + ); + }); + }); + it('renders a refresh action for file-backed M3U playlists', () => { fixture.destroy(); runtime.supportsPlaylistRefresh = true; diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.ts index 5193eefee..db537f5d8 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/playlist-item/playlist-item.component.ts @@ -159,4 +159,15 @@ export class PlaylistItemComponent implements OnInit { this.playlistClicked.emit(this.item._id); } + + /** + * Enter/Space open the source like a click on the row; Space also + * prevents the page scroll. The drag handle and the actions are siblings + * of the activation element, never descendants, so their keys cannot + * reach this handler. + */ + onActivationKey(event: Event): void { + event.preventDefault(); + this.onPlaylistClick(); + } }