diff --git a/docs/architecture/workspace-shell.md b/docs/architecture/workspace-shell.md index 95f3da51b..fad1fa96d 100644 --- a/docs/architecture/workspace-shell.md +++ b/docs/architecture/workspace-shell.md @@ -234,6 +234,15 @@ records no shift. `app.config.spec.ts` keeps the animations renderer out, and `workspace-route-swap.e2e.ts` asserts that no mutation checkpoint holds both pages and that neither the dashboard nor the content pane shifts. +Destroy hooks therefore run with the view's DOM already detached, where +`scrollTop` reads 0 and focus has dropped to `body`. Chromium blurs a focused +element synchronously while removing it, so its `focusout` (no +`relatedTarget`) arrives before the hook. The deferred removal used to hide +this. Record what a hook needs while the view is attached: the catalog keeps +the offset of the grid that scrolled last, and the player settings panel +judges a `focusout` a microtask later, when a removed panel is detached and a +pointer-blurred one is not. + Do not add `provideAnimations()` or `provideAnimationsAsync()` back; animate with CSS. A leave animation (`animate.leave` included) delays its element's removal in the same way, so a routed page must not carry one. 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..8c6b1ba2b 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,54 @@ describe('CategoryContentViewComponent', () => { expect(catalog.saveScrollPosition).toHaveBeenCalledWith(333); }); + it('saves the last scrolled offset on destroy, not the detached grid', () => { + const infiniteFixture = createInfiniteFixture(); + infiniteFixture.detectChanges(); + const grid = infiniteFixture.nativeElement.querySelector( + 'app-grid-list' + ) as HTMLElement; + Object.defineProperty(grid, 'scrollTop', { + configurable: true, + value: 640, + }); + grid.dispatchEvent(new Event('scroll')); + // Angular detaches a routed view before its destroy hooks run; + // a detached grid has no layout and reads 0. + Object.defineProperty(grid, 'scrollTop', { + configurable: true, + value: 0, + }); + + infiniteFixture.destroy(); + + expect(catalog.saveScrollPosition).toHaveBeenCalledWith(640); + }); + + it('saves the top on destroy for a grid recreated since the last scroll', () => { + const infiniteFixture = createInfiniteFixture(); + infiniteFixture.detectChanges(); + const scrolledGrid = infiniteFixture.nativeElement.querySelector( + 'app-grid-list' + ) as HTMLElement; + Object.defineProperty(scrolledGrid, 'scrollTop', { + configurable: true, + value: 640, + }); + scrolledGrid.dispatchEvent(new Event('scroll')); + // An inline detail replaces the grid; closing it renders a new one. + selectedItem.set({ xtream_id: 42 }); + infiniteFixture.detectChanges(); + selectedItem.set(null); + infiniteFixture.detectChanges(); + expect( + infiniteFixture.nativeElement.querySelector('app-grid-list') + ).not.toBe(scrolledGrid); + + infiniteFixture.destroy(); + + expect(catalog.saveScrollPosition).toHaveBeenLastCalledWith(0); + }); + 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..c29bae3e2 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 @@ -7,6 +7,7 @@ import { effect, ElementRef, inject, + NgZone, OnDestroy, OnInit, signal, @@ -198,6 +199,9 @@ export class CategoryContentViewComponent implements OnInit, OnDestroy { }); private previousGridResetKey: string | null = null; private hasAttemptedScrollRestore = false; + /** The offset of the grid that scrolled last (see ngOnDestroy). */ + private lastGridScroll: { grid: Element; scrollTop: number } | null = + null; private readonly routeCategory = signal<{ id: string | null } | null>(null); private initializedRoute: { category: { id: string | null }; @@ -205,6 +209,8 @@ export class CategoryContentViewComponent implements OnInit, OnDestroy { } | null = null; constructor() { + this.trackGridScrollTop(); + effect(() => { const category = this.routeCategory(); if (!category || this.catalog.routeReady?.() === false) { @@ -306,11 +312,15 @@ export class CategoryContentViewComponent implements OnInit, OnDestroy { // 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. - if (!this.selectedItem()) { - const grid = this.gridElement(); - if (grid) { - this.catalog.saveScrollPosition?.(grid.scrollTop); - } + // Angular removes a view's DOM before its destroy hooks run, so the + // grid is detached here and its scrollTop reads 0. A grid that never + // scrolled (a new category's) is still at the top. + const grid = this.gridElement(); + if (!this.selectedItem() && grid) { + const last = this.lastGridScroll; + this.catalog.saveScrollPosition?.( + last?.grid === grid ? last.scrollTop : 0 + ); } } @@ -339,6 +349,28 @@ export class CategoryContentViewComponent implements OnInit, OnDestroy { } } + /** Scroll events do not bubble; a passive capture listener sees them. */ + private trackGridScrollTop(): void { + const host: HTMLElement = this.hostElement.nativeElement; + const onScroll = (event: Event) => { + if (event.target instanceof HTMLElement) { + if (event.target.tagName === 'APP-GRID-LIST') { + this.lastGridScroll = { + grid: event.target, + scrollTop: event.target.scrollTop, + }; + } + } + }; + const options = { capture: true, passive: true }; + inject(NgZone).runOutsideAngular(() => + host.addEventListener('scroll', onScroll, options) + ); + this.destroyRef.onDestroy(() => + host.removeEventListener('scroll', onScroll, options) + ); + } + private gridElement(): HTMLElement | null { return this.hostElement.nativeElement.querySelector( 'app-grid-list' 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..be8dad471 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,89 @@ describe('PlayerControlsComponent settings panel', () => { fixture.nativeElement.remove(); }); + async function openSheetByKeyboard(): Promise<{ + sheet: HTMLElement; + tune: HTMLButtonElement; + }> { + 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); + return { sheet, tune }; + } + + async function closeSheetWithEscape(): Promise { + // Inside a task, as in the app: zone.js drains microtasks after + // a top-level listener, which would run them mid-removal. + await new Promise((resolve) => + setTimeout(() => { + document.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Escape', + bubbles: true, + }) + ); + fixture.detectChanges(); + resolve(); + }) + ); + await fixture.whenStable(); + await Promise.resolve(); + expect( + query('[data-test-id="player-controls-settings-panel"]') + ).toBeNull(); + } + + const blur = (target: HTMLElement) => + target.dispatchEvent( + new FocusEvent('focusout', { bubbles: true, relatedTarget: null }) + ); + + it('returns focus to tune when the removed sheet reports a focusout', async () => { + const { sheet, tune } = await openSheetByKeyboard(); + // Chromium blurs a focused element synchronously while removing + // it, before the view's destroy hook; jsdom does not, so + // emulate it. + const remove = sheet.remove.bind(sheet); + sheet.remove = () => { + blur(sheet); + remove(); + }; + + await closeSheetWithEscape(); + + expect(document.activeElement).toBe(tune); + fixture.nativeElement.remove(); + }); + + it('leaves focus on the page after a pointer release blurred the sheet', async () => { + const { sheet, tune } = await openSheetByKeyboard(); + // A pointer click releases focus in its own task, while the + // sheet stays attached. + blur(sheet); + sheet.blur(); + await Promise.resolve(); + + await closeSheetWithEscape(); + + expect(document.activeElement).not.toBe(tune); + 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..3ce8b109c 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,16 @@ export class PlayerSettingsPanelComponent implements OnDestroy { onFocusOut(event: FocusEvent): void { const next = event.relatedTarget; - this.focusInside = next instanceof Node && this.host.contains(next); + const inside = next instanceof Node && this.host.contains(next); + // Chromium blurs a focused element synchronously while removing it, + // before this view's destroy hook runs. Judge once the removal is + // done: a detached panel did not lose focus to anything, while a + // pointer release blurs a panel that stays attached. + queueMicrotask(() => { + if (this.host.isConnected) { + this.focusInside = inside; + } + }); } /**