From 1425279a0a8ba30e012cff484c8d7a08e5938a17 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 6 Sep 2026 15:19:38 +0200 Subject: [PATCH] fix(epg): keep guide keys on the grid, reconcile focus with filtered rows and wrap the toolbar Co-Authored-By: Claude Fable 5.1 --- .../epg-guide-keyboard.controller.spec.ts | 16 +++ .../epg-guide-keyboard.controller.ts | 24 +++- .../epg-guide-toolbar.component.scss | 53 +++++++- .../lib/epg-guide/epg-guide.component.html | 114 +++++++++--------- .../lib/epg-guide/epg-guide.component.scss | 14 ++- .../lib/epg-guide/epg-guide.component.spec.ts | 69 +++++++++++ .../src/lib/epg-guide/epg-guide.component.ts | 76 +++++++++++- 7 files changed, 296 insertions(+), 70 deletions(-) diff --git a/libs/ui/epg/src/lib/epg-guide/epg-guide-keyboard.controller.spec.ts b/libs/ui/epg/src/lib/epg-guide/epg-guide-keyboard.controller.spec.ts index 91fe6da80..cc0c2aa37 100644 --- a/libs/ui/epg/src/lib/epg-guide/epg-guide-keyboard.controller.spec.ts +++ b/libs/ui/epg/src/lib/epg-guide/epg-guide-keyboard.controller.spec.ts @@ -24,6 +24,7 @@ describe('EpgGuideKeyboardController', () => { blockCount: jest.fn(() => 3), activeRow: jest.fn(() => 2), isBlocked: jest.fn(() => false), + isOwnedTarget: jest.fn(() => true), play: jest.fn(), details: jest.fn(), jumpNow: jest.fn(), @@ -122,6 +123,21 @@ describe('EpgGuideKeyboardController', () => { expect(controller.focus()).toEqual({ row: 3, block: null }); }); + it('only closes when the target is outside the guide', () => { + host.isOwnedTarget.mockReturnValue(false); + + expect(controller.handle(key('ArrowDown'))).toBe(false); + expect(controller.focus()).toBeNull(); + expect(controller.handle(key('Enter'))).toBe(false); + expect(host.play).not.toHaveBeenCalled(); + expect(controller.handle(key('n'))).toBe(false); + expect(host.jumpNow).not.toHaveBeenCalled(); + + // Escape is the documented close key from anywhere on the page. + expect(controller.handle(key('Escape'))).toBe(true); + expect(host.close).toHaveBeenCalled(); + }); + it('leaves the catch-up button inside a card to the button', () => { const card = document.createElement('div'); card.setAttribute('role', 'button'); diff --git a/libs/ui/epg/src/lib/epg-guide/epg-guide-keyboard.controller.ts b/libs/ui/epg/src/lib/epg-guide/epg-guide-keyboard.controller.ts index 898b45049..63be8d988 100644 --- a/libs/ui/epg/src/lib/epg-guide/epg-guide-keyboard.controller.ts +++ b/libs/ui/epg/src/lib/epg-guide/epg-guide-keyboard.controller.ts @@ -14,6 +14,12 @@ export interface EpgGuideKeyboardHost { activeRow(): number; /** True while a dialog owns the keyboard. */ isBlocked(): boolean; + /** + * True when the key belongs to the guide: the listener is on the document, + * so a target that is neither inside the guide nor an unfocused page keeps + * its own keys (the docked player, the header). + */ + isOwnedTarget(target: EventTarget | null): boolean; play(row: number): void; details(row: number, block: number): void; jumpNow(): void; @@ -60,8 +66,9 @@ function clamp(value: number, min: number, max: number): number { /** * Roving focus for the guide grid. ↑/↓ move between rows, ←/→ between the * focused row's programmes, Enter plays the row, I opens details, N jumps to - * now, PgUp/PgDn change the day, Esc closes. Returns whether the event was - * consumed so the caller can `preventDefault()`. + * now, PgUp/PgDn change the day, Esc closes. Every key but Esc requires the + * host to own the event's target. Returns whether the event was consumed so + * the caller can `preventDefault()`. */ export class EpgGuideKeyboardController { readonly focus = signal(null); @@ -79,10 +86,17 @@ export class EpgGuideKeyboardController { ) { return false; } + // Escape is the documented close key and works from anywhere on the + // page; every other guide key applies only while the guide owns the + // target, or it would be stolen from whatever else has the focus. + if (event.key === 'Escape') { + this.host.close(); + return true; + } + if (!this.host.isOwnedTarget(event.target)) { + return false; + } switch (event.key) { - case 'Escape': - this.host.close(); - return true; case 'ArrowDown': return this.moveRow(1); case 'ArrowUp': diff --git a/libs/ui/epg/src/lib/epg-guide/epg-guide-toolbar.component.scss b/libs/ui/epg/src/lib/epg-guide/epg-guide-toolbar.component.scss index f4f39476a..ac94f1c29 100644 --- a/libs/ui/epg/src/lib/epg-guide/epg-guide-toolbar.component.scss +++ b/libs/ui/epg/src/lib/epg-guide/epg-guide-toolbar.component.scss @@ -4,12 +4,18 @@ display: block; } +// The guide lives in a lane that clips its overflow (the player dock, a split +// pane), so a toolbar that cannot fit one row must wrap: pushed-out controls +// would simply be unreachable. Everything but the two text fields keeps its +// size; the fields shrink first, then the row breaks. .guide-toolbar { display: flex; + flex-wrap: wrap; align-items: center; gap: 10px; + row-gap: 8px; min-height: 52px; - padding: 0 18px; + padding: 6px 18px; border-bottom: 1px solid $line; background: var(--app-content-bg); color: $text-primary; @@ -21,6 +27,15 @@ } } +.guide-toolbar__day, +.guide-toolbar__now, +.guide-toolbar__scope, +.guide-toolbar__toggle, +.guide-toolbar__density, +.guide-toolbar__zoom { + flex: 0 0 auto; +} + .guide-toolbar__day { display: inline-flex; align-items: center; @@ -178,7 +193,12 @@ gap: 6px; height: 30px; padding: 0 4px 0 10px; - width: 190px; + // Grows to its old fixed width when there is room and shrinks before the + // row wraps; `max-width` keeps the wide-window layout unchanged. + flex: 1 1 160px; + width: auto; + min-width: 120px; + max-width: 190px; border: 1px solid $line-strong; border-radius: 8px; background: $surface-1; @@ -217,7 +237,34 @@ } .guide-toolbar__input--search { - width: 230px; + flex-basis: 200px; + max-width: 230px; +} + +@media (max-width: 720px) { + // Too narrow for one row: the flexible spacer goes, the toggle keeps only + // its switch (the label stays in the DOM for assistive technology and the + // tooltip still names it) and the programme search takes the second row. + .guide-toolbar__spacer { + display: none; + } + + .guide-toolbar__toggle span { + position: absolute; + width: 1px; + height: 1px; + margin: -1px; + padding: 0; + overflow: hidden; + clip-path: inset(50%); + white-space: nowrap; + border: 0; + } + + .guide-toolbar__input--search { + flex: 1 1 100%; + max-width: none; + } } .guide-toolbar__count { diff --git a/libs/ui/epg/src/lib/epg-guide/epg-guide.component.html b/libs/ui/epg/src/lib/epg-guide/epg-guide.component.html index ca50ff270..b4a2df337 100644 --- a/libs/ui/epg/src/lib/epg-guide/epg-guide.component.html +++ b/libs/ui/epg/src/lib/epg-guide/epg-guide.component.html @@ -1,59 +1,63 @@ - + +
+ -@if (searchPanelVisible()) { -
- @for ( - hit of searchResults(); - track hit.program.channel + hit.program.start + $index - ) { - - } @empty { -
- {{ 'EPG.GUIDE.NO_RESULTS' | translate }} -
- } -
-} + @if (searchPanelVisible()) { +
+ @for ( + hit of searchResults(); + track hit.program.channel + hit.program.start + $index + ) { + + } @empty { +
+ {{ 'EPG.GUIDE.NO_RESULTS' | translate }} +
+ } +
+ } +
): Promise { fixture.detectChanges(); await fixture.whenStable(); @@ -244,6 +252,67 @@ describe('EpgGuideComponent', () => { expect(event.defaultPrevented).toBe(false); }); + it('handles grid keys only for its own targets, but closes from anywhere', async () => { + await settle(fixture); + const viewportEl: HTMLElement = fixture.debugElement.query( + By.css('cdk-virtual-scroll-viewport') + ).nativeElement; + viewportEl.scrollTo = jest.fn() as unknown as HTMLElement['scrollTo']; + const close = jest.fn(); + component.close.subscribe(close); + const outside = document.createElement('div'); + document.body.appendChild(outside); + + // A non-interactive surface outside the guide — the docked player, the + // header — keeps its keys: nothing moves, nothing is consumed. + const foreign = keydown('ArrowDown', outside); + component.onKeydown(foreign); + expect(component.focus()).toBeNull(); + expect(foreign.defaultPrevented).toBe(false); + + // Nothing focused: the state right after the `G` shortcut opened it. + component.onKeydown(keydown('ArrowDown', document.body)); + expect(component.focus()).toEqual({ row: 1, block: null }); + + const cell: HTMLElement = fixture.debugElement.query( + By.css('.epg-guide-row__channel') + ).nativeElement; + const own = keydown('ArrowDown', cell); + component.onKeydown(own); + expect(component.focus()).toEqual({ row: 2, block: null }); + expect(own.defaultPrevented).toBe(true); + + // Escape is the documented close key wherever the focus is. + component.onKeydown(keydown('Escape', outside)); + expect(close).toHaveBeenCalled(); + outside.remove(); + }); + + it('moves the roving focus with its channel and drops it with the row', async () => { + await settle(fixture); + component.focusCell(2, 0); + await settle(fixture); + expect(component.focus()).toEqual({ row: 2, block: 0 }); + + // The first channel disappears: the focus follows 'c' to its new index. + channels.set([channel('b', null, 2), channel('c', 'c', 3)]); + await settle(fixture); + expect(component.rows().map((row) => row.id)).toEqual(['b', 'c']); + expect(component.focus()).toEqual({ row: 1, block: null }); + + // A filter that hides the focused channel clears the focus instead of + // leaving it pointing at whatever moved into that row. + component.setFilter('channel b'); + await settle(fixture); + expect(component.rows().map((row) => row.id)).toEqual(['b']); + expect(component.focus()).toBeNull(); + + // Undoing the filter must not bring the stale focus back. + component.setFilter(''); + await settle(fixture); + expect(component.focus()).toBeNull(); + }); + it('opens a search hit with its own channel and focuses that row', async () => { await settle(fixture); // Revealing the row scrolls the CDK viewport; jsdom has no scrollTo. diff --git a/libs/ui/epg/src/lib/epg-guide/epg-guide.component.ts b/libs/ui/epg/src/lib/epg-guide/epg-guide.component.ts index 0525ceafa..ae10e55de 100644 --- a/libs/ui/epg/src/lib/epg-guide/epg-guide.component.ts +++ b/libs/ui/epg/src/lib/epg-guide/epg-guide.component.ts @@ -10,6 +10,7 @@ import { computed, DestroyRef, effect, + ElementRef, HostListener, inject, Injector, @@ -82,6 +83,7 @@ export class EpgGuideComponent implements OnDestroy { private readonly translate = inject(TranslateService); private readonly destroyRef = inject(DestroyRef); private readonly injector = inject(Injector); + private readonly hostElement = inject>(ElementRef); readonly close = output(); /** @@ -145,6 +147,7 @@ export class EpgGuideComponent implements OnDestroy { blockCount: (row) => this.blocksFor(row).length, activeRow: () => this.activeRowIndex(), isBlocked: () => this.dialog.openDialogs.length > 0, + isOwnedTarget: (target) => this.ownsTarget(target), play: (row) => this.commitRow(this.rows()[row]), details: (row, block) => this.openDetails(this.rows()[row], this.blocksFor(row)[block]), @@ -153,6 +156,13 @@ export class EpgGuideComponent implements OnDestroy { close: () => this.close.emit(), }); readonly focus = this.keyboard.focus; + /** + * The channel the roving focus sits on. `rows()` is rebuilt by the filters, + * by a scope switch and by coverage arriving late, so the focused index + * alone goes stale — Enter would then play a different channel. Recorded + * whenever the focus moves and reconciled whenever the rows change. + */ + private focusedRowId: string | null = null; /** * The row that carries the grid's single `tabindex="0"`: the focused one, * else the playing one, else the first — so Tab always reaches the grid, @@ -200,8 +210,19 @@ export class EpgGuideComponent implements OnDestroy { }); }); effect(() => { - this.rows(); - untracked(() => this.viewportController.loadRenderedRange()); + const focused = this.focus(); + untracked(() => { + this.focusedRowId = focused + ? (this.rows()[focused.row]?.id ?? null) + : null; + }); + }); + effect(() => { + const rows = this.rows(); + untracked(() => { + this.reconcileFocus(rows); + this.viewportController.loadRenderedRange(); + }); }); effect(() => { const viewport = this.viewport(); @@ -288,9 +309,12 @@ export class EpgGuideComponent implements OnDestroy { this.view.setZoom(value); } + /** + * The focus is not cleared here: `reconcileFocus` moves it to the filtered + * row's new index, or drops it when the filter hid that channel. + */ setFilter(value: string): void { this.view.filter.set(value); - this.focus.set(null); } onSearchQueryChange(query: string): void { @@ -337,6 +361,52 @@ export class EpgGuideComponent implements OnDestroy { this.close.emit(); } + /** + * Whether a key aimed at `target` belongs to the guide. The listener is on + * the document, so anything focused elsewhere — the docked player, the + * header — keeps its own keys; only the guide's own subtree and an + * unfocused page are ours. `document.body` is the target right after the + * `G` shortcut opened the guide, and a synthetic event carries no target + * at all. + */ + private ownsTarget(target: EventTarget | null): boolean { + if (target === null) { + return true; + } + if (!(target instanceof Node)) { + return false; + } + return ( + target === document.body || + target === document.documentElement || + this.hostElement.nativeElement.contains(target) + ); + } + + /** + * Keep the roving focus on the channel it was put on rather than on a row + * number the list has moved under it, and drop it when that channel is no + * longer shown — `tabbableRow` then falls back to the playing row. A focus + * that follows its channel loses its block: the list changed underneath, so + * the programmes behind that index may have moved with it, and the row is + * what Enter acts on. + */ + private reconcileFocus(rows: readonly EpgGuideChannel[]): void { + const focused = this.focus(); + const channelId = this.focusedRowId; + if (!focused || channelId === null) { + return; + } + const row = rows.findIndex((channel) => channel.id === channelId); + if (row < 0) { + this.focus.set(null); + return; + } + if (row !== focused.row) { + this.focus.set({ row, block: null }); + } + } + private revealRow(rowIndex: number): void { this.focus.set({ row: rowIndex, block: null }); this.viewportController.revealFocus(this.focus());