Skip to content

Fix home-page flicker AND close #725: interactive snapshot removed only after the routed DOM settles #1321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
milanmajchrak merged 3 commits into from
Jun 23, 2026
Merged

Fix home-page flicker AND close #725: interactive snapshot removed only after the routed DOM settles #1321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a0258b
fix(ssr-overlay): remove overlay only after the routed page DOM settl…
milanmajchrak Jun 22, 2026
12a2550
fix(ssr-overlay): make the masked page interactive + skip overlay und…
milanmajchrak Jun 23, 2026
3079e57
refactor(ssr-overlay): declarative DOM-settle, drop fragile admin-sid…
milanmajchrak Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions src/app/app.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 <ds-app> DOM
// to settle (no element added/removed for the quiet window) and only then removes the overlay.
let mockStore: MockStore;
let themeLoading$: BehaviorSubject<boolean>;
let themeService: ThemeService;
let originalRaF: typeof window.requestAnimationFrame;
let dsAppEl: HTMLElement;

beforeEach(() => {
mockStore = TestBed.inject(MockStore);
Expand All @@ -144,6 +146,12 @@ describe('App component', () => {
(themeService as any).isThemeLoading$ = themeLoading$.asObservable();
mockStore.setState({ core: { auth: { loading: false, blocking: true } } });

// A settled <ds-app> 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 = '<main id="main-content" style="display:block;height:800px">home content</main>';
document.body.appendChild(dsAppEl);

// Force rAF to a synchronous shim so assertions are deterministic.
originalRaF = window.requestAnimationFrame;
(window as any).requestAnimationFrame = (cb: FrameRequestCallback) => {
Expand All @@ -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;

Expand All @@ -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();
}));

Expand All @@ -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();
Expand Down
189 changes: 111 additions & 78 deletions src/app/app.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -7,6 +7,7 @@ import {
HostListener,
Inject,
NgZone,
OnDestroy,
OnInit,
PLATFORM_ID,
} from '@angular/core';
Expand All @@ -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';
Expand All @@ -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<void>();

/** 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 <ds-app> is no longer the empty shell

/**
* Whether or not the authentication is currently blocking the UI
*/
Expand Down Expand Up @@ -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
* `<router-outlet>`; 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 <ds-app> 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 `<router-outlet>`; 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 <ds-app> 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 <ds-app> 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 <ds-app> 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 <ds-app> 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<unknown> {
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 <ds-app> 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<unknown> {
const dsApp: Element | null = this.document.querySelector('ds-app');
if (!dsApp) {
return timer(this.ssrOverlaySettleMaxMs);
}
const elementMutations$ = new Observable<void>((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 <ds-app> 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 () {
Expand Down
Loading
Loading