mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-11 02:46:16 -08:00
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
4281194cb4
commit
ea7efdbd9b
8 files changed
+349
-1
No files matched your search
@@ -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.
|
||||
@@ -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<RowRing> {
|
||||
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<string> {
|
||||
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);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
"prefix": "lib",
|
||||
"projectType": "library",
|
||||
"tags": ["scope:playlist", "domain:m3u", "type:ui"],
|
||||
"implicitDependencies": ["ui-styles"],
|
||||
"targets": {
|
||||
"test": {
|
||||
"executor": "@nx/jest:jest",
|
||||
|
||||
+14
-1
@@ -82,7 +82,20 @@
|
||||
}
|
||||
</div>
|
||||
</div>
|
||||
<div class="playlist-content">
|
||||
<!-- The keyboard activation element is its own element so the drag
|
||||
handle and the actions stay siblings: an interactive control nested
|
||||
inside a role="button" is an invalid accessibility structure. A
|
||||
pointer click anywhere on the row still opens it via the row. -->
|
||||
<div
|
||||
class="playlist-content"
|
||||
role="button"
|
||||
tabindex="0"
|
||||
[attr.aria-label]="item.title || item.filename"
|
||||
[attr.aria-current]="isSelected() || null"
|
||||
[attr.aria-disabled]="isBusy() || null"
|
||||
(keydown.enter)="onActivationKey($event)"
|
||||
(keydown.space)="onActivationKey($event)"
|
||||
>
|
||||
<div class="playlist-title">
|
||||
{{ item.title || item.filename }}
|
||||
</div>
|
||||
|
||||
+10
@@ -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 {
|
||||
|
||||
+114
@@ -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<HTMLElement>(
|
||||
'.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;
|
||||
|
||||
+11
@@ -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();
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user