mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-14 05:06:16 -08:00
test(workspace): cover the atomic route swap and document it (#1922)
* fix(workspace): swap routed pages in one task when returning to the dashboard Leaving an Xtream portal section for the dashboard inserted the dashboard below the still-present portal page: the @angular/animations renderer queues every DOM removal until the end of the next change detection, while the router inserts the new page at once. With event coalescing that tick runs in the next requestAnimationFrame, so Chromium laid the document out 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, sources lib-workspace-dashboard-rails. Nothing uses @angular/animations triggers (Material animates with CSS), so the app no longer provides provideAnimations(). The old page now leaves in the same microtask, before the new one is inserted, and the swap records no shift. The router itself was already atomic: NavigationEnd and the shell's currentUrl follow activation synchronously. app.config.spec.ts keeps the animations renderer out; workspace-route-swap.e2e.ts asserts that no mutation checkpoint holds both pages and that neither the dashboard nor the content pane shifts. Both fail with provideAnimations() restored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(workspace): read scroll and focus state before a view's DOM detaches Without the @angular/animations renderer a destroyed view's DOM leaves before its destroy hooks run, which the deferred removal used to hide. Two hooks read state from the detached DOM: - CategoryContentView.ngOnDestroy saved the grid's scrollTop, which reads 0 once detached, over the offset saved when a card was clicked: back from a VOD detail landed at the top of the list (catalog-sorting E2E on every OS). The view now keeps the offset of the grid that scrolled last; a grid rendered since (a new category's) saves the top. - Chromium blurs a focused element synchronously while removing it, so removing the focused player settings panel fired a focusout without a relatedTarget before the panel's destroy hook, which cleared focusInside: Escape left focus on the page instead of the tune button (player-controls-keyboard web E2E). The panel now judges a focusout a microtask later: a removed panel is detached then, while a pointer release blurs a panel that stays attached and still drops focus. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: 4gray <fourgray@proton.me> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
5 files changed
+214
-4
No files matched your search
@@ -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.
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
await page.evaluate(
|
||||
() =>
|
||||
new Promise<void>((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);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -108,7 +108,9 @@ export const appConfig: ApplicationConfig = {
|
||||
// No provideAnimations(): nothing declares an Angular animation
|
||||
// trigger, and Material 22 animates with CSS, reading
|
||||
// prefers-reduced-motion itself. The provider only put the 66 KB
|
||||
// animation engine on the initial path.
|
||||
// animation engine on the initial path, and its renderer deferred
|
||||
// every DOM removal, so a routed page stayed beside the next one
|
||||
// until the following tick (workspace-shell.md, "Route swaps").
|
||||
// CDK overlays (menus, tooltips, dialogs) live in a container under
|
||||
// <body>, which the browser hides behind an element in DOM fullscreen.
|
||||
// This container follows the fullscreen element instead, so the
|
||||
|
||||
@@ -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,40 @@ 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.
|
||||
|
||||
Destroy hooks therefore run with the view's DOM already detached; the
|
||||
[UI guidelines](./iptvnator-ui-guidelines.md) ("No animations module") say
|
||||
how hooks record geometry and focus instead.
|
||||
|
||||
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
|
||||
|
||||
Reference in new issue
Block a user