Skip to content

Commit 85c8004

Browse files
jr-rkclaude
andauthored
MENDELU/Backport: Fix home-page SSR->CSR flicker (#1315)
* Backport of Fix home-page SSR->CSR flicker * Test: isolate isStable override and cover the no-rAF overlay path Make the ApplicationRef.isStable override in the removeSsrOverlayWhenStable suite configurable and restore the original descriptor in afterEach, so the patched observable can't leak onto the shared TestBed instance. Add a test for the requestAnimationFrame-absent fallback branch of the remover. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix: mark the SSR clone overlay inert so it can't trap keyboard focus The overlay holds a deep clone of the SSR DOM purely as a freeze-frame. It was aria-hidden and pointer-events:none, but Tab focus could still land on the dead cloned controls. Add the inert attribute to take it out of the tab order while it exists. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Backport final DOM-settle overlay mechanism to Mendelu (#1318, #1321) Mendelu was the last instance still on the original ApplicationRef.isStable trigger. isStable is held hostage by post-login/admin zone activity, so the hydration-safe clone overlay lingered over an app that had long finished (the same class of problem as dspace-customers#725 - visually here, since the clone lets clicks through, but the frozen frame still sat there for up to 15s). Port the final trigger used across the other customers: drop the clone once the auth/theme loader gate opens AND the live <ds-app> DOM has settled (MutationObserver + quiet window, content-height / #main-content check, 10s cap), decoupled from isStable. The Angular-18 hydration-safe CLONE index.html is unchanged (comments only); the DOM-settle trigger lives in AppComponent. Spec, theme-service mock and an OnDestroy teardown updated to match. NOTE: hand-port to the Angular-18 stack - CI validates compile + unit tests but NOT the real hydration + clone + DOM-settle timing; needs a visual check on a running Mendelu instance (authenticated hard reload) before merge. Ref: #1318, #1321 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: Reformatted the import into multiline style in app.component.spec.ts:20 --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3f3df1a commit 85c8004

5 files changed

Lines changed: 313 additions & 3 deletions

File tree

src/app/app.component.spec.ts

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ import { CommonModule } from '@angular/common';
22
import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
33
import {
44
ComponentFixture,
5+
discardPeriodicTasks,
6+
fakeAsync,
57
inject,
68
TestBed,
9+
tick,
710
waitForAsync,
811
} from '@angular/core/testing';
912
import {
@@ -14,11 +17,15 @@ import {
1417
Store,
1518
StoreModule,
1619
} from '@ngrx/store';
17-
import { provideMockStore } from '@ngrx/store/testing';
20+
import {
21+
MockStore,
22+
provideMockStore,
23+
} from '@ngrx/store/testing';
1824
import {
1925
TranslateLoader,
2026
TranslateModule,
2127
} from '@ngx-translate/core';
28+
import { BehaviorSubject } from 'rxjs';
2229

2330
import { APP_CONFIG } from '../config/app-config.interface';
2431
import { environment } from '../environments/environment';
@@ -58,7 +65,7 @@ let comp: AppComponent;
5865
let fixture: ComponentFixture<AppComponent>;
5966
const menuService = new MenuServiceStub();
6067
const initialState = {
61-
core: { auth: { loading: false } },
68+
core: { auth: { loading: false, blocking: false } },
6269
};
6370

6471
export function getMockLocaleService(): LocaleService {
@@ -149,4 +156,78 @@ describe('App component', () => {
149156
});
150157

151158
});
159+
160+
describe('removeSsrOverlayWhenContentVisible', () => {
161+
// The inline bootstrap script in src/index.html injects window.__dspaceRemoveSsrOverlay.
162+
// Once auth blocking and theme loading are both false, AppComponent waits for the <ds-app> DOM
163+
// to settle (no element added/removed for the quiet window) and only then removes the overlay.
164+
let mockStore: MockStore;
165+
let themeLoading$: BehaviorSubject<boolean>;
166+
let themeService: ThemeService;
167+
let originalRaF: typeof window.requestAnimationFrame;
168+
let dsAppEl: HTMLElement;
169+
170+
beforeEach(() => {
171+
mockStore = TestBed.inject(MockStore);
172+
themeService = TestBed.inject(ThemeService);
173+
themeLoading$ = new BehaviorSubject<boolean>(true);
174+
(themeService as any).isThemeLoading$ = themeLoading$.asObservable();
175+
mockStore.setState({ core: { auth: { loading: false, blocking: true } } });
176+
177+
// A settled <ds-app> with real content present, so the DOM-settle watcher can resolve.
178+
dsAppEl = document.createElement('ds-app');
179+
dsAppEl.setAttribute('style', 'display:block;height:800px');
180+
dsAppEl.innerHTML = '<main id="main-content" style="display:block;height:800px">home content</main>';
181+
document.body.appendChild(dsAppEl);
182+
183+
// Force rAF to a synchronous shim so assertions are deterministic.
184+
originalRaF = window.requestAnimationFrame;
185+
(window as any).requestAnimationFrame = (cb: FrameRequestCallback) => {
186+
cb(0);
187+
return 0 as any;
188+
};
189+
});
190+
191+
afterEach(() => {
192+
(window as any).requestAnimationFrame = originalRaF;
193+
delete (window as any).__dspaceRemoveSsrOverlay;
194+
if (dsAppEl && dsAppEl.parentNode) { dsAppEl.parentNode.removeChild(dsAppEl); }
195+
});
196+
197+
it('removes the overlay once auth/theme are ready AND the DOM has settled', fakeAsync(() => {
198+
const spy = jasmine.createSpy('__dspaceRemoveSsrOverlay');
199+
window.__dspaceRemoveSsrOverlay = spy;
200+
201+
// Re-construct so constructor-time subscription picks up our patched streams + global.
202+
const f = TestBed.createComponent(AppComponent);
203+
f.detectChanges();
204+
205+
expect(spy).not.toHaveBeenCalled();
206+
207+
mockStore.setState({ core: { auth: { loading: false, blocking: false } } });
208+
themeLoading$.next(false);
209+
210+
// Not removed at the gate: the DOM-settle quiet window must elapse first.
211+
expect(spy).not.toHaveBeenCalled();
212+
tick(700); // > SETTLE_QUIET_MS
213+
expect(spy).toHaveBeenCalledTimes(1);
214+
215+
discardPeriodicTasks();
216+
}));
217+
218+
it('is a no-op when the global is not injected (e.g. CSR-only route, SSR skipped)', fakeAsync(() => {
219+
// Global intentionally absent; constructor should not throw and should not break later.
220+
delete (window as any).__dspaceRemoveSsrOverlay;
221+
222+
const f = TestBed.createComponent(AppComponent);
223+
expect(() => f.detectChanges()).not.toThrow();
224+
225+
mockStore.setState({ core: { auth: { loading: false, blocking: false } } });
226+
themeLoading$.next(false);
227+
tick(700);
228+
229+
expect(window.__dspaceRemoveSsrOverlay).toBeUndefined();
230+
discardPeriodicTasks();
231+
}));
232+
});
152233
});

src/app/app.component.ts

Lines changed: 130 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import {
99
Component,
1010
HostListener,
1111
Inject,
12+
NgZone,
13+
OnDestroy,
1214
OnInit,
1315
PLATFORM_ID,
1416
} from '@angular/core';
@@ -29,12 +31,21 @@ import {
2931
import { TranslateService } from '@ngx-translate/core';
3032
import {
3133
BehaviorSubject,
34+
combineLatest,
3235
Observable,
36+
race,
37+
Subject,
38+
timer,
3339
} from 'rxjs';
3440
import {
41+
debounceTime,
3542
delay,
3643
distinctUntilChanged,
44+
filter,
45+
startWith,
46+
switchMap,
3747
take,
48+
takeUntil,
3849
withLatestFrom,
3950
} from 'rxjs/operators';
4051

@@ -64,10 +75,22 @@ import { ThemeService } from './shared/theme-support/theme.service';
6475
ThemedRootComponent,
6576
],
6677
})
67-
export class AppComponent implements OnInit, AfterViewInit {
78+
export class AppComponent implements OnInit, AfterViewInit, OnDestroy {
6879
notificationOptions;
6980
models;
7081

82+
/**
83+
* Emits on destroy to tear down the SSR-overlay-removal pipeline (gate subscription, the
84+
* MutationObserver and its debounce/cap timers all unsubscribe via `takeUntil(destroyed$)`).
85+
* AppComponent is the root component so this is mostly defensive + test hygiene.
86+
*/
87+
private destroyed$ = new Subject<void>();
88+
89+
/** SSR anti-flicker overlay (see src/index.html) removal tuning. */
90+
private readonly ssrOverlaySettleQuietMs = 600; // routed page is "done" after this long with no DOM change
91+
private readonly ssrOverlaySettleMaxMs = 10000; // backstop reveal (below index.html's 15s catastrophic net)
92+
private readonly ssrOverlayMinContentHeightPx = 200; // proves <ds-app> is no longer the empty shell
93+
7194
/**
7295
* Whether or not the authentication is currently blocking the UI
7396
*/
@@ -100,18 +123,124 @@ export class AppComponent implements OnInit, AfterViewInit {
100123
private cssService: CSSVariableService,
101124
private modalService: NgbModal,
102125
private modalConfig: NgbModalConfig,
126+
private ngZone: NgZone,
103127
) {
104128
this.notificationOptions = environment.notifications;
105129

106130
if (isPlatformBrowser(this.platformId)) {
107131
this.trackIdleModal();
132+
this.removeSsrOverlayWhenContentVisible();
108133
}
109134

110135
this.isThemeLoading$ = this.themeService.isThemeLoading$;
111136

112137
this.storeCSSVariables();
113138
}
114139

140+
/**
141+
* Drops the hydration-safe SSR freeze-frame (a detached clone painted on top; see src/index.html)
142+
* once the routed CSR page has actually finished rendering. Earlier revisions waited for
143+
* `ApplicationRef.isStable`, but on DSpace 9 (Angular 18) the theme system re-creates every
144+
* `ds-themed-*` wrapper imperatively on the client, and post-login zone activity keeps isStable
145+
* busy — so the frozen clone lingered (up to the 15s fallback) over an app that had long finished.
146+
*
147+
* Instead we drop the clone once the auth/theme loader gate has opened AND the live <ds-app> DOM
148+
* has SETTLED (no element added/removed for a short quiet window, with real content present) —
149+
* i.e. once the imperative themed re-render is done. This stays decoupled from isStable.
150+
* See {@link routedPageReadyToReveal$}.
151+
*/
152+
private removeSsrOverlayWhenContentVisible(): void {
153+
const win: Window | undefined = this._window?.nativeWindow;
154+
if (!win || typeof win.__dspaceRemoveSsrOverlay !== 'function') {
155+
return; // SSR was skipped for this route, so no overlay was installed — nothing to remove
156+
}
157+
// Run outside Angular: a MutationObserver watching the whole app must not trigger change
158+
// detection (it would also keep ApplicationRef.isStable permanently false).
159+
this.ngZone.runOutsideAngular(() => {
160+
this.routedPageReadyToReveal$().pipe(
161+
takeUntil(this.destroyed$),
162+
).subscribe(() => {
163+
// one frame so the freshly rendered content is painted before the snapshot fades out
164+
this.runAfterNextFrame(win, () => win.__dspaceRemoveSsrOverlay?.());
165+
});
166+
});
167+
}
168+
169+
/**
170+
* Emits once when it is safe to drop the SSR snapshot: the auth/theme loader gate has opened
171+
* (same condition root.component.html uses to swap its fullscreen loader for the routed content)
172+
* AND the routed page's DOM has settled. See {@link dsAppDomSettled$}.
173+
*/
174+
private routedPageReadyToReveal$(): Observable<unknown> {
175+
const loaderGateOpen$ = combineLatest([
176+
this.store.pipe(select(isAuthenticationBlocking), distinctUntilChanged()),
177+
this.themeService.isThemeLoading$,
178+
]).pipe(
179+
filter(([authBlocking, themeLoading]: [boolean, boolean]) => !authBlocking && !themeLoading),
180+
take(1),
181+
);
182+
return loaderGateOpen$.pipe(
183+
switchMap(() => this.dsAppDomSettled$()),
184+
);
185+
}
186+
187+
/**
188+
* Emits once when the live <ds-app> subtree stops being mutated (elements added/removed) for
189+
* `ssrOverlaySettleQuietMs` AND it holds real content — or after `ssrOverlaySettleMaxMs`, whichever
190+
* comes first. The cap guarantees a page that never goes quiet (constant background DOM updates)
191+
* still reveals; the 15s fallback in index.html remains the ultimate net.
192+
*/
193+
private dsAppDomSettled$(): Observable<unknown> {
194+
const dsApp: Element | null = this.document.querySelector('ds-app');
195+
if (!dsApp) {
196+
return timer(this.ssrOverlaySettleMaxMs);
197+
}
198+
const elementMutations$ = new Observable<void>((subscriber) => {
199+
const observer = new MutationObserver((records) => {
200+
if (records.some((record) => this.isElementChildListChange(record))) {
201+
subscriber.next();
202+
}
203+
});
204+
observer.observe(dsApp, { childList: true, subtree: true });
205+
return () => observer.disconnect();
206+
});
207+
const settled$ = elementMutations$.pipe(
208+
startWith(undefined), // start the quiet window immediately
209+
debounceTime(this.ssrOverlaySettleQuietMs), // ... reset by each render, fires once quiet
210+
filter(() => this.dsAppHasRenderedContent(dsApp)), // ... but never on the empty shell
211+
);
212+
return race(settled$, timer(this.ssrOverlaySettleMaxMs)).pipe(take(1));
213+
}
214+
215+
/** True once the live <ds-app> is no longer the empty shell the overlay script left behind. */
216+
private dsAppHasRenderedContent(dsApp: Element): boolean {
217+
const height = dsApp.getBoundingClientRect?.().height ?? 0;
218+
return height >= this.ssrOverlayMinContentHeightPx && dsApp.querySelector('#main-content') !== null;
219+
}
220+
221+
/** A childList mutation that adds or removes at least one element node (ignores text/attr noise). */
222+
private isElementChildListChange(record: MutationRecord): boolean {
223+
if (record.type !== 'childList') {
224+
return false;
225+
}
226+
const changedNodes = [...Array.from(record.addedNodes), ...Array.from(record.removedNodes)];
227+
return changedNodes.some((node) => node.nodeType === Node.ELEMENT_NODE);
228+
}
229+
230+
/** Runs `callback` after the next paint (or synchronously if requestAnimationFrame is unavailable). */
231+
private runAfterNextFrame(win: Window, callback: () => void): void {
232+
if (typeof win.requestAnimationFrame === 'function') {
233+
win.requestAnimationFrame(() => callback());
234+
} else {
235+
callback();
236+
}
237+
}
238+
239+
ngOnDestroy(): void {
240+
this.destroyed$.next();
241+
this.destroyed$.complete();
242+
}
243+
115244
ngOnInit() {
116245
/** Implement behavior for interface {@link ModalBeforeDismiss} */
117246
this.modalConfig.beforeDismiss = async function () {

src/app/shared/mocks/theme-service.mock.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,15 @@ import { isNotEmpty } from '../empty.util';
55
import { ThemeService } from '../theme-support/theme.service';
66

77
export function getMockThemeService(themeName = 'base', themes?: ThemeConfig[]): ThemeService {
8+
// isThemeLoading$ is a real property getter on ThemeService, so it must be a property on the mock
9+
// (not a spy method) - AppComponent's overlay-removal gate reads it as a stream.
810
const spy = jasmine.createSpyObj('themeService', {
911
getThemeName: themeName,
1012
getThemeName$: of(themeName),
1113
getThemeConfigFor: undefined,
1214
listenForRouteChanges: undefined,
15+
}, {
16+
isThemeLoading$: of(false),
1317
});
1418

1519
if (isNotEmpty(themes)) {

0 commit comments

Comments
 (0)