From a2ab00d3cd9856db144fed34e12bb7535cf2186c Mon Sep 17 00:00:00 2001 From: 4gray Date: Thu, 1 Oct 2026 21:01:16 +0200 Subject: [PATCH] fix(epg): move the guide focus with the N jump and keep DOM focus N left the roving focus on its row while the jump scrolled to the playing channel, so the CDK could recycle the focused row and drop DOM focus to the page, and the next arrow key scrolled all the way back. - N moves the roving focus to the playing row; only the keys that move the focus (arrows) reveal it, so N's smooth jump is never cancelled. - The DOM focus is handed to the roving cell once its row is rendered, and never taken from a control outside the grid. Co-Authored-By: Claude Opus 5.5 --- .../electron-backend-e2e/src/epg-guide.e2e.ts | 4 ++ .../epg-guide-keyboard.controller.spec.ts | 27 ++++++++ .../epg-guide-keyboard.controller.ts | 33 ++++++++-- .../epg-guide-viewport.controller.spec.ts | 65 +++++++++++++++++++ .../epg-guide-viewport.controller.ts | 34 +++++++++- .../lib/epg-guide/epg-guide.component.spec.ts | 5 +- .../src/lib/epg-guide/epg-guide.component.ts | 15 ++--- 7 files changed, 163 insertions(+), 20 deletions(-) diff --git a/apps/electron-backend-e2e/src/epg-guide.e2e.ts b/apps/electron-backend-e2e/src/epg-guide.e2e.ts index 79c36c5dc..c8c394209 100644 --- a/apps/electron-backend-e2e/src/epg-guide.e2e.ts +++ b/apps/electron-backend-e2e/src/epg-guide.e2e.ts @@ -183,6 +183,10 @@ test('@epg @electron opens the programme guide with the playlist channels, switc ); await app.mainWindow.keyboard.press('n'); await expect.poll(() => nowLineInLane(app.mainWindow)).toBe(true); + // The keyboard focus follows the jump to the playing row. + await expect( + rows.nth(0).locator('[data-epg-guide-grid][tabindex="0"]') + ).toBeFocused(); // "Only with EPG" hides the silent channel once coverage is known. const toggle = guide.locator('.guide-toolbar__toggle input'); 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 61350d2ce..466edf229 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 @@ -27,6 +27,7 @@ describe('EpgGuideKeyboardController', () => { isOwnedTarget: jest.fn((_target: EventTarget | null) => true), play: jest.fn(), details: jest.fn(), + revealFocus: jest.fn(), jumpNow: jest.fn(), stepDay: jest.fn(), close: jest.fn(), @@ -71,6 +72,32 @@ describe('EpgGuideKeyboardController', () => { expect(host.play).toHaveBeenLastCalledWith(3); }); + it('reveals the focus only for the keys that move it', () => { + controller.handle(key('ArrowDown')); + controller.handle(key('ArrowRight')); + expect(host.revealFocus).toHaveBeenCalledTimes(2); + + host.revealFocus.mockClear(); + controller.handle(key('n')); + controller.handle(key('PageDown')); + controller.handle(key('Enter')); + expect(host.revealFocus).not.toHaveBeenCalled(); + }); + + it('moves the focus to the playing row on N, where the jump scrolls', () => { + controller.focus.set({ row: 4, block: 1 }); + controller.handle(key('n')); + expect(controller.focus()).toEqual({ row: 2, block: null }); + expect(host.jumpNow).toHaveBeenCalledTimes(1); + + // Nothing playing: the jump stays on the focused row, and so does + // the focus. + host.activeRow.mockReturnValue(-1); + controller.focus.set({ row: 4, block: 1 }); + controller.handle(key('n')); + expect(controller.focus()).toEqual({ row: 4, block: 1 }); + }); + it('maps N, PageUp/PageDown and Escape', () => { controller.handle(key('n')); expect(host.jumpNow).toHaveBeenCalled(); 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 63be8d988..5a6d26c39 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 @@ -22,6 +22,11 @@ export interface EpgGuideKeyboardHost { isOwnedTarget(target: EventTarget | null): boolean; play(row: number): void; details(row: number, block: number): void; + /** + * Scroll the focus moved by an arrow key into view. N and the day keys + * scroll on their own; a reveal after them would cancel their scroll. + */ + revealFocus(): void; jumpNow(): void; stepDay(direction: EpgDateNavigationDirection): void; close(): void; @@ -112,8 +117,7 @@ export class EpgGuideKeyboardController { return this.details(); case 'n': case 'N': - this.host.jumpNow(); - return true; + return this.jumpNow(); case 'PageUp': this.host.stepDay('prev'); return true; @@ -146,6 +150,7 @@ export class EpgGuideKeyboardController { : count - 1 : clamp(current + delta, 0, count - 1); this.focus.set({ row: next, block: null }); + this.host.revealFocus(); return true; } @@ -156,14 +161,28 @@ export class EpgGuideKeyboardController { } const row = clamp(Math.max(0, this.currentRow()), 0, count - 1); const blocks = this.host.blockCount(row); - if (blocks === 0) { - this.focus.set({ row, block: null }); - return true; - } const current = this.focus()?.row === row ? (this.focus()?.block ?? null) : null; const start = current ?? (delta > 0 ? -1 : blocks); - this.focus.set({ row, block: clamp(start + delta, 0, blocks - 1) }); + this.focus.set({ + row, + block: blocks === 0 ? null : clamp(start + delta, 0, blocks - 1), + }); + this.host.revealFocus(); + return true; + } + + /** + * The jump scrolls to the playing row, so the focus follows it there. Left + * on a far row it would be recycled during the scroll, dropping the DOM + * focus to the page, and the next arrow key would scroll all the way back. + */ + private jumpNow(): boolean { + const row = this.host.activeRow(); + if (row >= 0 && row < this.host.rowCount()) { + this.focus.set({ row, block: null }); + } + this.host.jumpNow(); return true; } diff --git a/libs/ui/epg/src/lib/epg-guide/epg-guide-viewport.controller.spec.ts b/libs/ui/epg/src/lib/epg-guide/epg-guide-viewport.controller.spec.ts index 5bbfe867b..f0b36f7a5 100644 --- a/libs/ui/epg/src/lib/epg-guide/epg-guide-viewport.controller.spec.ts +++ b/libs/ui/epg/src/lib/epg-guide/epg-guide-viewport.controller.spec.ts @@ -106,6 +106,7 @@ function harness(rowCount = 100): Harness { activeRow: () => 40, ensureLoaded, setScrollLeft, + afterRender: (callback) => callback(), }; return { controller: new EpgGuideViewportController(host), @@ -287,6 +288,70 @@ describe('EpgGuideViewportController', () => { test.element.remove(); }); + it('focuses the roving target only once its row is rendered', () => { + const test = harness(); + test.controller.watch(test.viewport, test.destroyRef); + test.renderedRange$.next({ start: 0, end: 20 }); + const cell = document.createElement('button'); + cell.setAttribute('data-epg-guide-grid', ''); + cell.tabIndex = 0; + const focus = jest.spyOn(cell, 'focus'); + + // A smooth jump to row 40: the row is not rendered yet. + test.controller.focusRovingTargetOnRow(40); + expect(focus).not.toHaveBeenCalled(); + test.renderedRange$.next({ start: 20, end: 35 }); + expect(focus).not.toHaveBeenCalled(); + test.element.appendChild(cell); + test.renderedRange$.next({ start: 30, end: 50 }); + expect(focus).toHaveBeenCalledWith({ preventScroll: true }); + + // Already rendered: focused after the next render, and only once. + focus.mockClear(); + test.controller.focusRovingTargetOnRow(35); + expect(focus).toHaveBeenCalledTimes(1); + test.renderedRange$.next({ start: 30, end: 60 }); + expect(focus).toHaveBeenCalledTimes(1); + }); + + it('drops a pending roving focus when a newer one is requested', () => { + const test = harness(); + test.controller.watch(test.viewport, test.destroyRef); + test.renderedRange$.next({ start: 0, end: 20 }); + const cell = document.createElement('button'); + cell.setAttribute('data-epg-guide-grid', ''); + cell.tabIndex = 0; + test.element.appendChild(cell); + const focus = jest.spyOn(cell, 'focus'); + + test.controller.focusRovingTargetOnRow(40); + test.controller.focusRovingTargetOnRow(60); + focus.mockClear(); + test.renderedRange$.next({ start: 30, end: 50 }); + expect(focus).not.toHaveBeenCalled(); + test.renderedRange$.next({ start: 50, end: 70 }); + expect(focus).toHaveBeenCalledTimes(1); + }); + + it('does not take the focus from a control outside the grid', () => { + const test = harness(); + const cell = document.createElement('button'); + cell.setAttribute('data-epg-guide-grid', ''); + cell.tabIndex = 0; + test.element.appendChild(cell); + const focus = jest.spyOn(cell, 'focus'); + const field = document.createElement('input'); + document.body.appendChild(field); + field.focus(); + try { + test.controller.focusRovingTarget(); + expect(focus).not.toHaveBeenCalled(); + expect(document.activeElement).toBe(field); + } finally { + field.remove(); + } + }); + it('reveals the focused row and block, and ignores a null focus', () => { const test = harness(); test.controller.revealFocus({ row: 40, block: 1 }); diff --git a/libs/ui/epg/src/lib/epg-guide/epg-guide-viewport.controller.ts b/libs/ui/epg/src/lib/epg-guide/epg-guide-viewport.controller.ts index 187131b6b..406e0c830 100644 --- a/libs/ui/epg/src/lib/epg-guide/epg-guide-viewport.controller.ts +++ b/libs/ui/epg/src/lib/epg-guide/epg-guide-viewport.controller.ts @@ -2,7 +2,7 @@ import { ListRange } from '@angular/cdk/collections'; import { DestroyRef } from '@angular/core'; import { CdkVirtualScrollViewport } from '@angular/cdk/scrolling'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; -import { filter, take } from 'rxjs'; +import { filter, Subscription, take } from 'rxjs'; import { TimelineRenderBlock } from '../epg-timeline/epg-timeline-render.util'; import { EpgGuideFocus } from './epg-guide-keyboard.controller'; import { EPG_GUIDE_ROW_BUFFER } from './epg-guide-layout.util'; @@ -31,6 +31,8 @@ export interface EpgGuideViewportHost { ensureLoaded(channels: readonly EpgGuideChannel[]): void; /** Reports the viewport's horizontal offset; drives the ruler and now-line. */ setScrollLeft(left: number): void; + /** Run `callback` after the next render (`afterNextRender`). */ + afterRender(callback: () => void): void; } /** @@ -41,6 +43,7 @@ export interface EpgGuideViewportHost { */ export class EpgGuideViewportController { private renderedRange: ListRange | null = null; + private pendingFocus: Subscription | null = null; constructor(private readonly host: EpgGuideViewportHost) {} @@ -161,6 +164,12 @@ export class EpgGuideViewportController { */ focusRovingTarget(): void { const element = this.host.viewport()?.elementRef.nativeElement; + const active = document.activeElement; + // Only a focus inside the grid, or one already lost to the page, is + // moved: a deferred call must not take it from a control used since. + if (active && active !== document.body && !element?.contains(active)) { + return; + } const target = element?.querySelector( '[data-epg-guide-grid][tabindex="0"]' ); @@ -169,6 +178,29 @@ export class EpgGuideViewportController { } } + /** + * `focusRovingTarget` once `row` is rendered. A smooth jump renders a far + * row only towards its end, and only a rendered cell can take the focus; + * the CDK may recycle the previously focused one meanwhile. Before the + * viewport has reported a range (jsdom), the next render is used. + */ + focusRovingTargetOnRow(row: number): void { + this.pendingFocus?.unsubscribe(); + this.pendingFocus = null; + const viewport = this.host.viewport(); + const focus = () => + this.host.afterRender(() => this.focusRovingTarget()); + const rendered = (range: ListRange | null) => + range === null || (range.start <= row && row < range.end); + if (!viewport || rendered(this.renderedRange)) { + focus(); + return; + } + this.pendingFocus = viewport.renderedRangeStream + .pipe(filter(rendered), take(1)) + .subscribe(focus); + } + /** Keep the keyboard focus target inside the viewport, both axes. */ revealFocus(focused: EpgGuideFocus | null): void { const viewport = this.host.viewport(); diff --git a/libs/ui/epg/src/lib/epg-guide/epg-guide.component.spec.ts b/libs/ui/epg/src/lib/epg-guide/epg-guide.component.spec.ts index e00e6e91c..49044789a 100644 --- a/libs/ui/epg/src/lib/epg-guide/epg-guide.component.spec.ts +++ b/libs/ui/epg/src/lib/epg-guide/epg-guide.component.spec.ts @@ -461,12 +461,13 @@ describe('EpgGuideComponent', () => { component.onKeydown(keydown('n')); - // One combined smooth scroll; a reveal after it would cancel it. + // One combined smooth scroll; a reveal after it would cancel it. The + // focus follows the jump to the playing row. expect(scrollTo).toHaveBeenCalledTimes(1); expect(scrollTo).toHaveBeenCalledWith( expect.objectContaining({ top: 0, behavior: 'smooth' }) ); - expect(component.focus()).toEqual({ row: 0, block: 0 }); + expect(component.focus()).toEqual({ row: 0, block: null }); }); it('moves the roving focus to a clicked programme card', async () => { 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 bf69caa5f..dfed6d379 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 @@ -151,6 +151,7 @@ export class EpgGuideComponent implements OnDestroy { play: (row) => this.commitRow(this.rows()[row]), details: (row, block) => this.openDetails(this.rows()[row], this.blocksFor(row)[block]), + revealFocus: () => this.viewportController.revealFocus(this.focus()), jumpNow: () => this.jumpNow(), stepDay: (direction) => this.stepDay(direction), close: () => this.close.emit(), @@ -184,6 +185,8 @@ export class EpgGuideComponent implements OnDestroy { activeRow: () => this.activeRowIndex(), ensureLoaded: (channels) => this.programsService.ensureLoaded(channels), setScrollLeft: (left) => this.view.scrollLeft.set(left), + afterRender: (callback) => + afterNextRender(callback, { injector: this.injector }), }); private readonly dialogs = new EpgGuideDialogController( @@ -253,23 +256,15 @@ export class EpgGuideComponent implements OnDestroy { * listener of its own — but it must own the DOM focus, or a screen reader * would still announce whatever the user tabbed from. The roving * `tabindex="0"` moves with the signal, so the element to focus only exists - * after the next render. + * after the next render — after N's smooth jump, once its row is rendered. */ @HostListener('document:keydown', ['$event']) onKeydown(event: KeyboardEvent): void { - const focusBefore = this.focus(); if (!this.keyboard.handle(event)) { return; } event.preventDefault(); - // Only a key that moved the focus scrolls to it: N scrolls to now on - // its own, and a reveal of a focus left off-screen would cancel it. - if (this.focus() !== focusBefore) { - this.viewportController.revealFocus(this.focus()); - } - afterNextRender(() => this.viewportController.focusRovingTarget(), { - injector: this.injector, - }); + this.viewportController.focusRovingTargetOnRow(this.tabbableRow()); } trackRow(_index: number, channel: EpgGuideChannel): string {