diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index 79db5691437..5d18f76b806 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -1,5 +1,5 @@ import { Store, StoreModule } from '@ngrx/store'; -import { ComponentFixture, discardPeriodicTasks, fakeAsync, flush, inject, TestBed, waitForAsync } from '@angular/core/testing'; +import { ComponentFixture, discardPeriodicTasks, fakeAsync, inject, TestBed, tick, waitForAsync } from '@angular/core/testing'; import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; import { CommonModule } from '@angular/common'; import { ActivatedRoute, Router } from '@angular/router'; @@ -131,11 +131,13 @@ describe('App component', () => { describe('removeSsrOverlayWhenContentVisible', () => { // The inline bootstrap script in src/index.html injects window.__dspaceRemoveSsrOverlay. - // AppComponent should remove it once both auth blocking and theme loading are false. + // Once auth blocking and theme loading are both false, AppComponent waits for the DOM + // to settle (no element added/removed for the quiet window) and only then removes the overlay. let mockStore: MockStore; let themeLoading$: BehaviorSubject; let themeService: ThemeService; let originalRaF: typeof window.requestAnimationFrame; + let dsAppEl: HTMLElement; beforeEach(() => { mockStore = TestBed.inject(MockStore); @@ -144,6 +146,12 @@ describe('App component', () => { (themeService as any).isThemeLoading$ = themeLoading$.asObservable(); mockStore.setState({ core: { auth: { loading: false, blocking: true } } }); + // A settled with real content present, so the DOM-settle watcher can resolve. + dsAppEl = document.createElement('ds-app'); + dsAppEl.setAttribute('style', 'display:block;height:800px'); + dsAppEl.innerHTML = '
home content
'; + document.body.appendChild(dsAppEl); + // Force rAF to a synchronous shim so assertions are deterministic. originalRaF = window.requestAnimationFrame; (window as any).requestAnimationFrame = (cb: FrameRequestCallback) => { @@ -155,9 +163,10 @@ describe('App component', () => { afterEach(() => { (window as any).requestAnimationFrame = originalRaF; delete (window as any).__dspaceRemoveSsrOverlay; + if (dsAppEl && dsAppEl.parentNode) { dsAppEl.parentNode.removeChild(dsAppEl); } }); - it('removes the overlay once auth is unblocked and theme loading is finished', fakeAsync(() => { + it('removes the overlay once auth/theme are ready AND the DOM has settled', fakeAsync(() => { const spy = jasmine.createSpy('__dspaceRemoveSsrOverlay'); window.__dspaceRemoveSsrOverlay = spy; @@ -169,9 +178,12 @@ describe('App component', () => { mockStore.setState({ core: { auth: { loading: false, blocking: false } } }); themeLoading$.next(false); - flush(); + // Not removed at the gate: the DOM-settle quiet window must elapse first. + expect(spy).not.toHaveBeenCalled(); + tick(700); // > SETTLE_QUIET_MS expect(spy).toHaveBeenCalledTimes(1); + discardPeriodicTasks(); })); @@ -184,7 +196,7 @@ describe('App component', () => { mockStore.setState({ core: { auth: { loading: false, blocking: false } } }); themeLoading$.next(false); - flush(); + tick(700); expect(window.__dspaceRemoveSsrOverlay).toBeUndefined(); discardPeriodicTasks(); diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 9a64199ac32..bef650a9326 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -1,4 +1,4 @@ -import { distinctUntilChanged, filter, first, take, withLatestFrom, delay } from 'rxjs/operators'; +import { debounceTime, distinctUntilChanged, filter, startWith, switchMap, take, takeUntil, withLatestFrom, delay } from 'rxjs/operators'; import { DOCUMENT, isPlatformBrowser } from '@angular/common'; import { AfterViewInit, @@ -7,6 +7,7 @@ import { HostListener, Inject, NgZone, + OnDestroy, OnInit, PLATFORM_ID, } from '@angular/core'; @@ -17,7 +18,7 @@ import { Router, } from '@angular/router'; -import { BehaviorSubject, combineLatest, Observable } from 'rxjs'; +import { BehaviorSubject, combineLatest, Observable, race, Subject, timer } from 'rxjs'; import { select, Store } from '@ngrx/store'; import { NgbModal, NgbModalConfig } from '@ng-bootstrap/ng-bootstrap'; import { TranslateService } from '@ngx-translate/core'; @@ -39,10 +40,22 @@ import { distinctNext } from './core/shared/distinct-next'; styleUrls: ['./app.component.scss'], changeDetection: ChangeDetectionStrategy.OnPush, }) -export class AppComponent implements OnInit, AfterViewInit { +export class AppComponent implements OnInit, AfterViewInit, OnDestroy { notificationOptions; models; + /** + * Emits on destroy to tear down the SSR-overlay-removal pipeline (gate subscription, the + * MutationObserver and its debounce/cap timers all unsubscribe via `takeUntil(destroyed$)`). + * AppComponent is the root component so this is mostly defensive + test hygiene. + */ + private destroyed$ = new Subject(); + + /** SSR anti-flicker overlay (see src/index.html) removal tuning. */ + private readonly ssrOverlaySettleQuietMs = 600; // routed page is "done" after this long with no DOM change + private readonly ssrOverlaySettleMaxMs = 10000; // backstop reveal (below index.html's 15s catastrophic net) + private readonly ssrOverlayMinContentHeightPx = 200; // proves is no longer the empty shell + /** * Whether or not the authentication is currently blocking the UI */ @@ -94,99 +107,119 @@ export class AppComponent implements OnInit, AfterViewInit { /** * Drops the SSR mask overlay installed by the inline bootstrap script in src/index.html once the - * real CSR content has actually been painted. This trigger has to thread a needle between the two - * earlier approaches, each of which fixed one symptom and reintroduced the other: + * routed CSR page has actually finished rendering. Picking the right moment is the whole problem, + * and the two earlier attempts each fixed one symptom and reintroduced the other: * - * - PR #1288 waited for `ApplicationRef.isStable`. That guaranteed the content was painted (no - * flicker) but isStable is held hostage by ANY ongoing zone async — after an admin login the - * app keeps the zone busy (authz/widgets, periodic polling, AAI/discojuice scripts) so isStable - * fires many seconds late (or never, hitting the 15s fallback). The inert snapshot then masks - * the live, already-rendered page -> "looks rendered but not interactive" (issue #725). - * - PR #1317 switched to the loader-swap gate `!isAuthenticationBlocking && !isThemeLoading` plus - * a single requestAnimationFrame. That fires promptly (fixing #725) but the gate only un-hides - * ``; Angular has NOT yet rendered the routed page at that instant and one rAF - * runs before the browser paints, so the snapshot is dropped over an empty for a frame - * or two -> the flicker came back. + * - PR #1288 waited for `ApplicationRef.isStable`. No flicker, but isStable is held hostage by ANY + * ongoing zone async — after an admin login the app keeps the zone busy (authz/widgets, periodic + * polling, AAI/discojuice scripts) so isStable fires many seconds late (or hits the 15s + * fallback). The inert snapshot then masks the live page -> "looks rendered but not interactive" + * (issue #725). + * - PR #1317 switched to the loader-swap gate `!isAuthenticationBlocking && !isThemeLoading` plus a + * single requestAnimationFrame. Prompt, but that gate only un-hides ``; the home + * page then renders piecewise (navbar, search box, community list, recent submissions) as each + * section's data arrives. Dropping the snapshot at the gate — or, as an earlier revision of this + * method did, as soon as *some* content exists — exposes a half-built page that visibly pops + * into place on a hard reload -> the flicker. * - * We keep #1317's decoupling from isStable (so background async can never delay us) but, after the - * gate opens, we wait across animation frames until the real is actually laid out before - * removing the snapshot. See {@link removeSsrOverlayAfterContentPainted}. + * The signal we actually need is "the routed page has stopped changing". So, after the gate opens, + * we keep the snapshot until the live DOM has SETTLED (no element added/removed for a + * short quiet window, with real content present). This stays decoupled from isStable (DOM-settle + * ignores non-rendering background async, so admin reveals in a few seconds rather than ~15s). + * See {@link routedPageReadyToReveal$}. */ private removeSsrOverlayWhenContentVisible(): void { - const w: Window | undefined = this._window?.nativeWindow; - if (!w || typeof w.__dspaceRemoveSsrOverlay !== 'function') { - return; + const win: Window | undefined = this._window?.nativeWindow; + if (!win || typeof win.__dspaceRemoveSsrOverlay !== 'function') { + return; // SSR was skipped for this route, so no overlay was installed — nothing to remove } - // run outside Angular so the subscription does not keep change detection alive + // Run outside Angular: a MutationObserver watching the whole app must not trigger change + // detection (it would also keep ApplicationRef.isStable permanently false). this.ngZone.runOutsideAngular(() => { - combineLatest([ - this.store.pipe(select(isAuthenticationBlocking), distinctUntilChanged()), - this.themeService.isThemeLoading$, - ]).pipe( - filter(([blocking, themeLoading]: [boolean, boolean]) => !blocking && !themeLoading), - first(), + this.routedPageReadyToReveal$().pipe( + takeUntil(this.destroyed$), ).subscribe(() => { - this.removeSsrOverlayAfterContentPainted(w); + // one frame so the freshly rendered content is painted before the snapshot fades out + this.runAfterNextFrame(win, () => win.__dspaceRemoveSsrOverlay?.()); }); }); } /** - * Waits until the routed CSR view has been committed to the DOM and painted, then removes the SSR - * snapshot overlay. "Painted" is approximated by the real reaching a non-trivial height - * AND containing its `#main-content` host (i.e. it is no longer the empty shell the overlay script - * left behind). We poll this cheap layout signal once per animation frame, capped at MAX_FRAMES so - * that — unlike isStable in #1288 — nothing can hold the overlay open indefinitely; the 15s hard - * fallback in index.html stays as the catastrophic-error safety net. + * Emits once when it is safe to drop the SSR snapshot: the auth/theme loader gate has opened + * (same condition root.component.html uses to swap its fullscreen loader for the routed content) + * AND the routed page's DOM has settled. See {@link dsAppDomSettled$}. */ - private removeSsrOverlayAfterContentPainted(w: Window): void { - const doc: Document = this.document; - const raf: ((cb: FrameRequestCallback) => number) | null = - typeof w.requestAnimationFrame === 'function' ? w.requestAnimationFrame.bind(w) : null; - const remove = () => { - if (typeof w.__dspaceRemoveSsrOverlay === 'function') { - w.__dspaceRemoveSsrOverlay(); - } - }; - const MAX_FRAMES = 180; // ~3s @60fps safety cap; the routed shell normally paints within a few frames - const MIN_CONTENT_HEIGHT = 200; // px: enough to prove the real is no longer the empty shell - let frames = 0; - const contentPainted = (): boolean => { - const app: Element | null = doc.querySelector('ds-app'); - if (!app) { - return false; - } - let height = 0; - try { - height = app.getBoundingClientRect().height; - } catch (e) { - height = 0; - } - return height >= MIN_CONTENT_HEIGHT && app.querySelector('#main-content') !== null; - }; - const tick = () => { - if (contentPainted() || ++frames >= MAX_FRAMES) { - // one more frame so the painted content is committed to screen before the snapshot fades - if (raf) { - raf(remove); - } else { - remove(); + private routedPageReadyToReveal$(): Observable { + const loaderGateOpen$ = combineLatest([ + this.store.pipe(select(isAuthenticationBlocking), distinctUntilChanged()), + this.themeService.isThemeLoading$, + ]).pipe( + filter(([authBlocking, themeLoading]: [boolean, boolean]) => !authBlocking && !themeLoading), + take(1), + ); + return loaderGateOpen$.pipe( + switchMap(() => this.dsAppDomSettled$()), + ); + } + + /** + * Emits once when the live subtree stops being mutated (elements added/removed) for + * `ssrOverlaySettleQuietMs` AND it holds real content — or after `ssrOverlaySettleMaxMs`, whichever + * comes first. The cap guarantees a page that never goes quiet (constant background DOM updates) + * still reveals; the 15s fallback in index.html remains the ultimate net. + */ + private dsAppDomSettled$(): Observable { + const dsApp: Element | null = this.document.querySelector('ds-app'); + if (!dsApp) { + return timer(this.ssrOverlaySettleMaxMs); + } + const elementMutations$ = new Observable((subscriber) => { + const observer = new MutationObserver((records) => { + if (records.some((record) => this.isElementChildListChange(record))) { + subscriber.next(); } - return; - } - if (raf) { - raf(tick); - } else { - setTimeout(tick, 16); - } - }; - if (raf) { - raf(tick); + }); + observer.observe(dsApp, { childList: true, subtree: true }); + return () => observer.disconnect(); + }); + const settled$ = elementMutations$.pipe( + startWith(undefined), // start the quiet window immediately + debounceTime(this.ssrOverlaySettleQuietMs), // ... reset by each render, fires once quiet + filter(() => this.dsAppHasRenderedContent(dsApp)), // ... but never on the empty shell + ); + return race(settled$, timer(this.ssrOverlaySettleMaxMs)).pipe(take(1)); + } + + /** True once the live is no longer the empty shell the overlay script left behind. */ + private dsAppHasRenderedContent(dsApp: Element): boolean { + const height = dsApp.getBoundingClientRect?.().height ?? 0; + return height >= this.ssrOverlayMinContentHeightPx && dsApp.querySelector('#main-content') !== null; + } + + /** A childList mutation that adds or removes at least one element node (ignores text/attr noise). */ + private isElementChildListChange(record: MutationRecord): boolean { + if (record.type !== 'childList') { + return false; + } + const changedNodes = [...Array.from(record.addedNodes), ...Array.from(record.removedNodes)]; + return changedNodes.some((node) => node.nodeType === Node.ELEMENT_NODE); + } + + /** Runs `callback` after the next paint (or synchronously if requestAnimationFrame is unavailable). */ + private runAfterNextFrame(win: Window, callback: () => void): void { + if (typeof win.requestAnimationFrame === 'function') { + win.requestAnimationFrame(() => callback()); } else { - setTimeout(tick, 16); + callback(); } } + ngOnDestroy(): void { + this.destroyed$.next(); + this.destroyed$.complete(); + } + ngOnInit() { /** Implement behavior for interface {@link ModalBeforeDismiss} */ this.modalConfig.beforeDismiss = async function () { diff --git a/src/index.html b/src/index.html index 3801f720208..5eb827c8d7f 100644 --- a/src/index.html +++ b/src/index.html @@ -9,23 +9,31 @@ @@ -40,18 +48,21 @@ available before Angular 16, so on every browser load Angular tears down the entire SSR DOM and re-renders the component tree from scratch. The rebuild takes ~600-1500 ms on slow connections, during which the user sees the SSR view -> blank/half-built CSR view -> final CSR view. - This script captures the SSR DOM as a non-interactive snapshot the moment it's parsed (before - any module/main script runs - those are type=module and therefore deferred). While Angular - rebuilds the real invisibly, the snapshot keeps the page looking stable. AppComponent - removes the overlay once ApplicationRef.isStable settles. + This script captures the SSR DOM as a snapshot the moment it's parsed (before any module/main + script runs - those are type=module and therefore deferred). While Angular rebuilds the real + underneath (visually covered by the opaque snapshot, but still interactive), the snapshot + keeps the page looking stable. AppComponent removes the overlay once the routed CSR page has + finished rendering (its DOM has settled) - see AppComponent.removeSsrOverlayWhenDomSettles. */ (function () { if (typeof window === 'undefined' || typeof document === 'undefined') return; - // Skip when Cypress is driving the page. The overlay duplicates SSR DOM (moved into the - // overlay) alongside the CSR DOM (rendered into ) during the masking window — so - // any cy.get('#some-id').click() picks up two elements and fails. The overlay is a pure - // UX nicety, and Cypress E2E doesn't measure visual smoothness anyway; bail early. + // Skip when an E2E runner is driving the page. The overlay duplicates SSR DOM (moved into the + // overlay) alongside the CSR DOM (rendered into ) during the masking window — so a + // strict-mode locator like cy.get('#x')/page.locator('#x') picks up two elements and fails. The + // overlay is a pure UX nicety and E2E doesn't measure visual smoothness, so bail early for both + // Cypress (window.Cypress) and any WebDriver-based runner (Playwright/Selenium: navigator.webdriver). if (typeof window.Cypress !== 'undefined') return; + if (typeof navigator !== 'undefined' && navigator.webdriver) return; try { var app = document.querySelector('ds-app'); // If SSR was skipped for this route (excludePathPatterns), there are no children; nothing to mask. @@ -78,25 +89,27 @@ // so the overlay is pixel-identical to what the user already saw before Angular booted. // Cloning via innerHTML loses parent-context-dependent rendering. // - // Accessibility note: we deliberately do NOT set aria-hidden on the overlay. The overlay - // *is* the visible page during the masking window, so assistive technologies should read - // it. The original underneath gets visibility:hidden (via attribute + CSS rule), - // which removes both itself and its children from the accessibility tree. + // Accessibility: the snapshot is now a purely VISUAL mask, so we mark it aria-hidden. The real + // underneath is no longer visibility:hidden — it stays in the accessibility tree and + // is the interactive surface — so assistive tech (and mouse clicks) target the live, functional + // app rather than a soon-to-be-removed duplicate snapshot. This also avoids the duplicate + // a11y nodes the old (overlay-as-a11y-surface) approach produced during masking. var overlay = document.createElement('div'); overlay.id = '__dspace_ssr_overlay'; + overlay.setAttribute('aria-hidden', 'true'); while (app.firstChild) { overlay.appendChild(app.firstChild); } - // Hide the now-empty so Angular can rebuild into it invisibly. We use an attribute - // (CSS in targets it) rather than setting .style.visibility directly so Angular's - // template doesn't blow it away on first ChangeDetection. + // Mark as being masked. NOTE: this attribute is now only a state hook — it no longer + // hides the element (the visibility:hidden CSS rule was removed) so the live app stays visible + // (covered by the opaque overlay) and, crucially, INTERACTIVE while Angular rebuilds into it. app.setAttribute('data-dspace-ssr-hidden', ''); document.body.appendChild(overlay); var removing = false; window.__dspaceRemoveSsrOverlay = function () { - // Re-entrancy guard: null the pointer up-front so a racing isStable + 15s safety + // Re-entrancy guard: null the pointer up-front so a racing DOM-settle removal + 15s safety // fallback cannot start two interleaving fade-out passes (which would re-remove // the kept styles from underneath the first pass). if (removing) return; diff --git a/src/typings.d.ts b/src/typings.d.ts index f7397dc290e..445b5ecd2c7 100644 --- a/src/typings.d.ts +++ b/src/typings.d.ts @@ -89,8 +89,9 @@ declare module '*.scss' { /** * Window global injected by the inline anti-flicker bootstrap script in `src/index.html`. - * Called once by `AppComponent.removeSsrOverlayWhenStable()` when `ApplicationRef.isStable` - * fires, to drop the SSR-mask overlay and let the freshly built CSR DOM become visible. + * Called once by `AppComponent.removeSsrOverlayWhenContentVisible()` (via + * `removeSsrOverlayWhenDomSettles()`) once the routed CSR page's DOM has settled, to drop the + * SSR-mask overlay and let the freshly built CSR DOM become visible. */ interface Window { __dspaceRemoveSsrOverlay?: (() => void) | null;