mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-14 13:16:16 -08:00
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>
This commit is contained in:
1 parent
f60d797011
commit
67cf84f490
5 files changed
+212
-5
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);
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
// <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,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
|
||||
|
||||
Reference in new issue
Block a user