diff --git a/.changes/workspace-dashboard-return-swap.md b/.changes/workspace-dashboard-return-swap.md new file mode 100644 index 000000000..b8b06a04d --- /dev/null +++ b/.changes/workspace-dashboard-return-swap.md @@ -0,0 +1,8 @@ +--- +type: fix +area: workspace +--- + +Going back to the dashboard from a portal section no longer makes the page +jump: the dashboard replaces the portal page in one step instead of appearing +below it for a moment. diff --git a/apps/electron-backend-e2e/src/workspace-route-swap.e2e.ts b/apps/electron-backend-e2e/src/workspace-route-swap.e2e.ts new file mode 100644 index 000000000..cc9ea6378 --- /dev/null +++ b/apps/electron-backend-e2e/src/workspace-route-swap.e2e.ts @@ -0,0 +1,132 @@ +import { Page } from '@playwright/test'; +import { + addXtreamPortal, + closeElectronApp, + expect, + goToDashboard, + launchElectronApp, + resetMockServers, + test, + waitForXtreamWorkspaceReady, +} from './electron-test-fixtures'; + +/** + * Leaving a portal section for the dashboard must replace the routed page + * in one task. With the @angular/animations renderer the router inserted the + * dashboard at once but the old page (and the context panel) left only at + * the end of the next, rAF-coalesced change detection: in between Chromium + * laid out the dashboard below the portal page, at y=720 in a 720 px window, + * and scored its jump into view as a layout shift of 0.337 at 1280 px. + */ + +type RouteSwapProbe = { + overlaps: string[][]; + shiftSources: string[]; +}; + +declare global { + interface Window { + __routeSwapProbe?: RouteSwapProbe; + } +} + +/** Sources whose shift the old page's late removal caused. */ +const SWAP_SHIFT_SOURCES = [ + 'lib-workspace-dashboard-rails', + 'main.workspace-content', +]; + +async function armRouteSwapProbe(page: Page): Promise { + await page.evaluate(() => { + const content = document.querySelector('main.workspace-content'); + if (!content) { + throw new Error('workspace content pane is missing'); + } + const probe: RouteSwapProbe = { overlaps: [], shiftSources: [] }; + window.__routeSwapProbe = probe; + // Mutation callbacks run at the microtask checkpoint after the + // router's activation, before any frame or deferred change detection. + new MutationObserver(() => { + const pages = [...content.children] + .filter((child) => child.tagName !== 'ROUTER-OUTLET') + .map((child) => child.tagName.toLowerCase()); + if (pages.length > 1) { + probe.overlaps.push(pages); + } + }).observe(content, { childList: true }); + new PerformanceObserver((list) => { + for (const entry of list.getEntries()) { + const sources = + (entry as PerformanceEntry & { + sources?: { node?: Node | null }[]; + }).sources ?? []; + for (const { node } of sources) { + if (node instanceof Element) { + const className = node.classList.contains( + 'workspace-content' + ) + ? '.workspace-content' + : ''; + probe.shiftSources.push( + node.tagName.toLowerCase() + className + ); + } + } + } + }).observe({ type: 'layout-shift' }); + }); +} + +async function waitForTwoFrames(page: Page): Promise { + await page.evaluate( + () => + new Promise((resolve) => + requestAnimationFrame(() => + requestAnimationFrame(() => setTimeout(resolve, 0)) + ) + ) + ); +} + +test.describe('Workspace route swap', () => { + test('replaces a portal page with the dashboard without a frame showing both', async ({ + dataDir, + request, + }) => { + await resetMockServers(request, ['xtream']); + const app = await launchElectronApp(dataDir); + const page = app.mainWindow; + + try { + await app.electronApp.evaluate(({ BrowserWindow }) => { + BrowserWindow.getAllWindows()[0].setContentSize(1280, 720); + }); + await addXtreamPortal(page); + await waitForXtreamWorkspaceReady(page); + await expect(page).toHaveURL(/\/workspace\/xtreams\/[^/]+\/vod/); + await expect( + page.locator('app-workspace-shell-context-sidebar') + ).toBeVisible(); + + await armRouteSwapProbe(page); + await goToDashboard(page); + await expect( + page.locator('lib-workspace-dashboard-rails') + ).toBeVisible(); + await expect( + page.locator('app-workspace-shell-context-sidebar') + ).toHaveCount(0); + await waitForTwoFrames(page); + + const probe = await page.evaluate(() => window.__routeSwapProbe); + expect(probe?.overlaps).toEqual([]); + expect( + probe?.shiftSources.filter((source) => + SWAP_SHIFT_SOURCES.includes(source) + ) + ).toEqual([]); + } finally { + await closeElectronApp(app); + } + }); +}); diff --git a/apps/web/src/app/app.config.spec.ts b/apps/web/src/app/app.config.spec.ts new file mode 100644 index 000000000..aaed3b80c --- /dev/null +++ b/apps/web/src/app/app.config.spec.ts @@ -0,0 +1,33 @@ +import { ANIMATION_MODULE_TYPE, RendererFactory2 } from '@angular/core'; + +type ProviderEntry = { provide?: unknown }; + +function flattenProviders(providers: readonly unknown[]): ProviderEntry[] { + return providers.flatMap((provider) => { + if (Array.isArray(provider)) { + return flattenProviders(provider); + } + // makeEnvironmentProviders() wraps its list in ɵproviders. + const wrapped = (provider as { ɵproviders?: unknown[] } | null) + ?.ɵproviders; + return wrapped + ? flattenProviders(wrapped) + : [provider as ProviderEntry]; + }); +} + +describe('appConfig', () => { + // The @angular/animations renderer queues every DOM removal until the + // end of the next change detection, while the router inserts a routed + // page at once: leaving a portal for the dashboard laid out both pages + // until the coalesced tick and scored the swap as a layout shift. + it('keeps the default DOM renderer, so routed pages swap in one task', async () => { + const { appConfig } = await import('./app.config'); + const tokens = flattenProviders(appConfig.providers).map( + (provider) => provider?.provide + ); + + expect(tokens).not.toContain(RendererFactory2); + expect(tokens).not.toContain(ANIMATION_MODULE_TYPE); + }); +}); diff --git a/apps/web/src/app/app.config.ts b/apps/web/src/app/app.config.ts index 6a5b072bb..77a486595 100644 --- a/apps/web/src/app/app.config.ts +++ b/apps/web/src/app/app.config.ts @@ -10,7 +10,6 @@ import { } from '@angular/cdk/overlay'; import { ApplicationConfig, inject, importProvidersFrom } from '@angular/core'; import { MAT_FORM_FIELD_DEFAULT_OPTIONS } from '@angular/material/form-field'; -import { provideAnimations } from '@angular/platform-browser/animations'; import { provideRouter, withComponentInputBinding } from '@angular/router'; import { provideServiceWorker } from '@angular/service-worker'; import { provideEffects } from '@ngrx/effects'; @@ -106,8 +105,12 @@ export function DataFactory() { export const appConfig: ApplicationConfig = { providers: [ ...changeDetectionProviders, + // No provideAnimations(): nothing uses @angular/animations triggers + // (Material animates with CSS), and its renderer defers every DOM + // removal to the end of the next change detection. The router + // inserts the new page at once, so the old page stayed beside it + // until that tick (see workspace-shell.md, "Route swaps"). provideRouter(routes, withComponentInputBinding()), - provideAnimations(), // CDK overlays (menus, tooltips, dialogs) live in a container under // , which the browser hides behind an element in DOM fullscreen. // This container follows the fullscreen element instead, so the diff --git a/docs/architecture/workspace-shell.md b/docs/architecture/workspace-shell.md index 58b09ed0c..95f3da51b 100644 --- a/docs/architecture/workspace-shell.md +++ b/docs/architecture/workspace-shell.md @@ -174,9 +174,10 @@ navigation (Back, drawer toggle) changes. ends: `perf:compositing` reads the same layer and mask counts on every route as before the slides (#1895). 6. **Not covered.** A slide does not change what the route renders. Going - back from a portal to the dashboard still shows one frame with the new - dashboard below the old page before the old page leaves; that jump is a - routing issue, not a layout slide. + back from a portal to the dashboard used to lay the new dashboard out + below the old page before the old page left, a shift a slide cannot + hide. That was the route swap, fixed at its cause (see + [Route swaps](#route-swaps)). `WorkspaceShellComponent` binds only to `WorkspaceShellFacade`. The facade is kept as a thin template-facing API and delegates ownership to component-scoped @@ -207,6 +208,36 @@ When adding shell behavior, prefer placing it in the service that owns the nearest existing state. Keep `WorkspaceShellFacade` as a stable re-export layer for the template unless the template contract itself intentionally changes. +### Route swaps + +The router's activation runs in one task, outside change detection: +deactivation destroys the old routed view, then activation creates the new +one and inserts its host element into `main.workspace-content` at once. +`WorkspaceShellRouteStateService.currentUrl` is set on `NavigationEnd`, which +the router emits synchronously after activation, so the shell's state does +not lag the outlet. The shell re-renders (the context panel leaves, the +content frame slides) in the next change detection. With event coalescing +that runs in the next `requestAnimationFrame`, before that frame paints. + +The app does not provide `provideAnimations()`. Nothing uses +`@angular/animations` triggers (Material animates with CSS), and that +package's renderer queues every DOM removal until the end of the next change +detection, while insertions apply at once. Leaving a portal section for the +dashboard therefore inserted the dashboard below the still-present portal +page, and the old page left only in the coalesced tick. In between, Chromium +laid out the document with both pages, the dashboard at y=720 in a 720 px +window, and scored the swap as one layout shift: 0.337 at 1280 px, 0.394 at +900 px, 0.723 at 420 px (measured with a `layout-shift` observer in the +`electron-performance` build). With the default DOM renderer the old page is +removed in the same microtask, before the new one is inserted, and the swap +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. + +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. + ## Header Back The header's leading slot is the workspace's one page-level Back. Pages do not