diff --git a/docs/architecture/iptvnator-ui-guidelines.md b/docs/architecture/iptvnator-ui-guidelines.md index 9f07e1045..c45387b47 100644 --- a/docs/architecture/iptvnator-ui-guidelines.md +++ b/docs/architecture/iptvnator-ui-guidelines.md @@ -127,7 +127,16 @@ Everything else is lazy by these rules: Material 22 animates with CSS, deciding from `prefers-reduced-motion` itself, so `app.config.ts` does not call `provideAnimations()`; the Electron `material-motion.e2e.ts` checks that dialogs, selects, menus, snack-bars and - tooltips still animate in and out and stop under reduced motion. + tooltips still animate in and out and stop under reduced motion. One side + effect of the module is gone with it: its renderer deferred every DOM + removal to the end of the change-detection tick, so a destroy hook still + saw its view attached. Without it Angular detaches the view first, where + `scrollTop` reads 0 and Chromium has already fired a `focusout` with no + related target for the focused control. A destroy hook therefore never + reads geometry or focus from its own view: the catalog list tracks the + grid offset from scroll events, and the player settings panel decides + "focus was inside" in a microtask after such a `focusout`, which the close + destroys the panel before. Both have specs that detach the view first. The app theme is the `.dark-theme` class that `SettingsService` puts on ``; a "System" choice resolves the OS preference there and nowhere diff --git a/libs/portal/catalog/feature/src/lib/category-content-view/category-content-view.component.spec.ts b/libs/portal/catalog/feature/src/lib/category-content-view/category-content-view.component.spec.ts index ea16cd80c..0f41f4b56 100644 --- a/libs/portal/catalog/feature/src/lib/category-content-view/category-content-view.component.spec.ts +++ b/libs/portal/catalog/feature/src/lib/category-content-view/category-content-view.component.spec.ts @@ -792,6 +792,26 @@ describe('CategoryContentViewComponent', () => { expect(catalog.saveScrollPosition).toHaveBeenCalledWith(333); }); + it('saves the offset the grid last reported when it is destroyed detached', () => { + const infiniteFixture = createInfiniteFixture(); + infiniteFixture.detectChanges(); + const grid = infiniteFixture.nativeElement.querySelector( + 'app-grid-list' + ) as HTMLElement; + // A detached element reports scrollTop 0, as in a browser. + Object.defineProperty(grid, 'scrollTop', { + configurable: true, + get: () => (grid.isConnected ? 420 : 0), + }); + grid.dispatchEvent(new Event('scroll')); + + // The view's DOM leaves the document before ngOnDestroy runs. + infiniteFixture.nativeElement.remove(); + infiniteFixture.destroy(); + + expect(catalog.saveScrollPosition).toHaveBeenCalledWith(420); + }); + it('consumes a saved scroll position once the list is rendered', () => { catalog.consumeSavedScrollPosition.mockReturnValue(500); const infiniteFixture = createInfiniteFixture(); diff --git a/libs/portal/catalog/feature/src/lib/category-content-view/category-content-view.component.ts b/libs/portal/catalog/feature/src/lib/category-content-view/category-content-view.component.ts index da751b838..fcb6a793b 100644 --- a/libs/portal/catalog/feature/src/lib/category-content-view/category-content-view.component.ts +++ b/libs/portal/catalog/feature/src/lib/category-content-view/category-content-view.component.ts @@ -198,6 +198,18 @@ export class CategoryContentViewComponent implements OnInit, OnDestroy { }); private previousGridResetKey: string | null = null; private hasAttemptedScrollRestore = false; + /** + * The grid's last reported offset. On destroy the view is already + * detached from the document, where `scrollTop` reads 0, so the hook + * saves this instead. Scroll events do not bubble but can be captured. + */ + private lastGridScrollTop = 0; + private readonly trackGridScroll = (event: Event) => { + const target = event.target; + if (target instanceof HTMLElement && target.matches('app-grid-list')) { + this.lastGridScrollTop = target.scrollTop; + } + }; private readonly routeCategory = signal<{ id: string | null } | null>(null); private initializedRoute: { category: { id: string | null }; @@ -205,6 +217,16 @@ export class CategoryContentViewComponent implements OnInit, OnDestroy { } | null = null; constructor() { + const host = this.hostElement.nativeElement; + host.addEventListener('scroll', this.trackGridScroll, { + capture: true, + passive: true, + }); + this.destroyRef.onDestroy(() => + host.removeEventListener('scroll', this.trackGridScroll, { + capture: true, + }) + ); effect(() => { const category = this.routeCategory(); if (!category || this.catalog.routeReady?.() === false) { @@ -305,11 +327,15 @@ export class CategoryContentViewComponent implements OnInit, OnDestroy { ngOnDestroy(): void { // Leaving the list without opening a detail (e.g. switching portal // tabs) still snapshots the spot; the facade validates the selection - // before ever restoring it. + // before ever restoring it. Angular detaches the view's DOM before + // this hook runs, and a detached grid reports a scrollTop of 0, so + // the tracked offset stands in for it. if (!this.selectedItem()) { const grid = this.gridElement(); if (grid) { - this.catalog.saveScrollPosition?.(grid.scrollTop); + this.catalog.saveScrollPosition?.( + grid.isConnected ? grid.scrollTop : this.lastGridScrollTop + ); } } } @@ -346,6 +372,8 @@ export class CategoryContentViewComponent implements OnInit, OnDestroy { } private scrollGridToTop(): void { + // A grid already at the top fires no scroll event for this call. + this.lastGridScrollTop = 0; this.gridElement()?.scrollTo?.({ top: 0 }); } diff --git a/libs/ui/playback/src/lib/player-controls/player-controls.component.settings-panel.spec.ts b/libs/ui/playback/src/lib/player-controls/player-controls.component.settings-panel.spec.ts index 7aebb3175..b0ad560e0 100644 --- a/libs/ui/playback/src/lib/player-controls/player-controls.component.settings-panel.spec.ts +++ b/libs/ui/playback/src/lib/player-controls/player-controls.component.settings-panel.spec.ts @@ -444,6 +444,52 @@ describe('PlayerControlsComponent settings panel', () => { fixture.nativeElement.remove(); }); + it('returns focus to tune although removal fires focusout with no related target', async () => { + const tune = query( + '[data-test-id="player-controls-settings-button"]' + ) as HTMLButtonElement; + document.body.appendChild(fixture.nativeElement); + tune.focus(); + const matches = tune.matches.bind(tune); + Object.defineProperty(tune, 'matches', { + configurable: true, + value: (selector: string) => + selector === ':focus-visible' || matches(selector), + }); + tune.click(); + fixture.detectChanges(); + await fixture.whenStable(); + const sheet = query( + '[data-test-id="player-controls-settings-panel"]' + ) as HTMLElement; + expect(document.activeElement).toBe(sheet); + + // Chromium fires focusout with relatedTarget null on the focused + // element while its subtree is being removed, in the same task + // as the removal; jsdom does not, so the event is dispatched by + // hand right before the close destroys the panel. + sheet.dispatchEvent( + new FocusEvent('focusout', { + bubbles: true, + relatedTarget: null, + }) + ); + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }) + ); + fixture.detectChanges(); + await fixture.whenStable(); + await Promise.resolve(); + + expect( + query('[data-test-id="player-controls-settings-panel"]') + ).toBeNull(); + expect(document.activeElement).toBe( + query('[data-test-id="player-controls-settings-button"]') + ); + fixture.nativeElement.remove(); + }); + it('leaves focus alone when a pointer opens the sheet', async () => { query('[data-test-id="player-controls-settings-button"]')?.click(); fixture.detectChanges(); diff --git a/libs/ui/playback/src/lib/player-controls/player-settings-panel.component.ts b/libs/ui/playback/src/lib/player-controls/player-settings-panel.component.ts index db712da25..cbfae5b87 100644 --- a/libs/ui/playback/src/lib/player-controls/player-settings-panel.component.ts +++ b/libs/ui/playback/src/lib/player-controls/player-settings-panel.component.ts @@ -122,7 +122,20 @@ export class PlayerSettingsPanelComponent implements OnDestroy { onFocusOut(event: FocusEvent): void { const next = event.relatedTarget; - this.focusInside = next instanceof Node && this.host.contains(next); + if (next instanceof Node) { + this.focusInside = this.host.contains(next); + return; + } + // No related target: focus went to the page, or the panel is being + // removed with focus inside, which Chromium reports with this event + // while the panel is still attached. The close destroys the panel in + // the same task, before this microtask runs, so a removal leaves the + // flag for ngOnDestroy to act on. + queueMicrotask(() => { + this.focusInside = + this.host.isConnected && + this.host.contains(document.activeElement); + }); } /**