Skip to content

Commit 157129c

Browse files
jr-rkclaude
andauthored
ZCU-DATA/Backport: Fix home-page SSR->CSR flicker (#1312)
* Backport of Fix home-page SSR->CSR flicker * Fix: always unhide app when removing SSR anti-flicker overlay The overlay remover bailed out via `if (!el) return;` before unhiding <ds-app>, so if the overlay node went missing (browser extension, race, external script) the app stayed visibility:hidden forever -> blank page, plus the kept SSR styles leaked. Unhide the app and clean up the kept styles unconditionally, before checking for the overlay node. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Chore: drop accidentally committed build/spec logs _build.log and _spec.log are local deploy-tooling output that should never have been tracked. Remove them and gitignore /_*.log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * 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> * Refactor: remove SSR overlay on content-visible instead of isStable Propagates VSB-TUO's fix #1317 to this instance. The overlay was removed when ApplicationRef.isStable settled, but isStable can be delayed for seconds by post-login admin zone activity (auth work, background polling, third-party scripts) - during which the live app stays hidden under the SSR mask and the page renders but is non-interactive (dataquest-dev/dspace-customers#725). Switch removal to the same condition root.component.html uses to show real content: !isAuthenticationBlocking && !isThemeLoading. Drop the now-unused ApplicationRef injection and the 50ms pad; keep the 15s hard fallback as a catastrophic safety net. Tests and the theme-service mock updated to match. Ref: #1317 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Backport final SSR-overlay mechanism from VSB-TUO (#1318, #1321) Supersedes the #1317 content-visible trigger backported earlier. That gate (!isAuthenticationBlocking && !isThemeLoading) still revealed a half-built page on hard reload, so VSB-TUO's #1318/#1321 keep the snapshot until the routed <ds-app> DOM has SETTLED (MutationObserver + quiet window, with a content height / #main-content check and a 10s cap). The overlay is now a purely visual mask, so the live app stays interactive underneath while it rebuilds (closes dspace-customers#725 - "looks rendered but not clickable"). index.html, app.component.ts, spec and typings are synced to VSB-TUO's final version; the VSB-only ngAfterViewInit delay(0) is omitted (these instances don't carry it). Ref: #1318, #1321 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 61683b7 commit 157129c

8 files changed

Lines changed: 375 additions & 10 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
/tsd_typings/
77
npm-debug.log
88

9+
# build/install/spec logs emitted by local deploy tooling
10+
/_*.log
11+
912
/build/
1013

1114
/coverage

angular.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@
100100
"budgets": [
101101
{
102102
"type": "initial",
103-
"maximumWarning": "3mb",
104-
"maximumError": "5mb"
103+
"maximumWarning": "5.5mb",
104+
"maximumError": "6mb"
105105
},
106106
{
107107
"type": "anyComponentStyle",

src/app/app.component.spec.ts

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { Store, StoreModule } from '@ngrx/store';
2-
import { ComponentFixture, inject, TestBed, waitForAsync } from '@angular/core/testing';
2+
import { ComponentFixture, discardPeriodicTasks, fakeAsync, inject, TestBed, tick, waitForAsync } from '@angular/core/testing';
33
import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
44
import { CommonModule } from '@angular/common';
55
import { ActivatedRoute, Router } from '@angular/router';
66
import { TranslateLoader, TranslateModule } from '@ngx-translate/core';
7+
import { BehaviorSubject } from 'rxjs';
78

89
// Load the implementations that should be tested
910
import { AppComponent } from './app.component';
@@ -30,7 +31,7 @@ import { Angulartics2DSpace } from './statistics/angulartics/dspace-provider';
3031
import { storeModuleConfig } from './app.reducer';
3132
import { LocaleService } from './core/locale/locale.service';
3233
import { authReducer } from './core/auth/auth.reducer';
33-
import { provideMockStore } from '@ngrx/store/testing';
34+
import { MockStore, provideMockStore } from '@ngrx/store/testing';
3435
import { ThemeService } from './shared/theme-support/theme.service';
3536
import { getMockThemeService } from './shared/mocks/theme-service.mock';
3637
import { BreadcrumbsService } from './breadcrumbs/breadcrumbs.service';
@@ -41,7 +42,7 @@ let comp: AppComponent;
4142
let fixture: ComponentFixture<AppComponent>;
4243
const menuService = new MenuServiceStub();
4344
const initialState = {
44-
core: { auth: { loading: false } }
45+
core: { auth: { loading: false, blocking: false } }
4546
};
4647

4748
export function getMockLocaleService(): LocaleService {
@@ -127,4 +128,78 @@ describe('App component', () => {
127128
});
128129

129130
});
131+
132+
describe('removeSsrOverlayWhenContentVisible', () => {
133+
// The inline bootstrap script in src/index.html injects window.__dspaceRemoveSsrOverlay.
134+
// Once auth blocking and theme loading are both false, AppComponent waits for the <ds-app> DOM
135+
// to settle (no element added/removed for the quiet window) and only then removes the overlay.
136+
let mockStore: MockStore;
137+
let themeLoading$: BehaviorSubject<boolean>;
138+
let themeService: ThemeService;
139+
let originalRaF: typeof window.requestAnimationFrame;
140+
let dsAppEl: HTMLElement;
141+
142+
beforeEach(() => {
143+
mockStore = TestBed.inject(MockStore);
144+
themeService = TestBed.inject(ThemeService);
145+
themeLoading$ = new BehaviorSubject<boolean>(true);
146+
(themeService as any).isThemeLoading$ = themeLoading$.asObservable();
147+
mockStore.setState({ core: { auth: { loading: false, blocking: true } } });
148+
149+
// A settled <ds-app> with real content present, so the DOM-settle watcher can resolve.
150+
dsAppEl = document.createElement('ds-app');
151+
dsAppEl.setAttribute('style', 'display:block;height:800px');
152+
dsAppEl.innerHTML = '<main id="main-content" style="display:block;height:800px">home content</main>';
153+
document.body.appendChild(dsAppEl);
154+
155+
// Force rAF to a synchronous shim so assertions are deterministic.
156+
originalRaF = window.requestAnimationFrame;
157+
(window as any).requestAnimationFrame = (cb: FrameRequestCallback) => {
158+
cb(0);
159+
return 0 as any;
160+
};
161+
});
162+
163+
afterEach(() => {
164+
(window as any).requestAnimationFrame = originalRaF;
165+
delete (window as any).__dspaceRemoveSsrOverlay;
166+
if (dsAppEl && dsAppEl.parentNode) { dsAppEl.parentNode.removeChild(dsAppEl); }
167+
});
168+
169+
it('removes the overlay once auth/theme are ready AND the DOM has settled', fakeAsync(() => {
170+
const spy = jasmine.createSpy('__dspaceRemoveSsrOverlay');
171+
window.__dspaceRemoveSsrOverlay = spy;
172+
173+
// Re-construct so constructor-time subscription picks up our patched streams + global.
174+
const f = TestBed.createComponent(AppComponent);
175+
f.detectChanges();
176+
177+
expect(spy).not.toHaveBeenCalled();
178+
179+
mockStore.setState({ core: { auth: { loading: false, blocking: false } } });
180+
themeLoading$.next(false);
181+
182+
// Not removed at the gate: the DOM-settle quiet window must elapse first.
183+
expect(spy).not.toHaveBeenCalled();
184+
tick(700); // > SETTLE_QUIET_MS
185+
expect(spy).toHaveBeenCalledTimes(1);
186+
187+
discardPeriodicTasks();
188+
}));
189+
190+
it('is a no-op when the global is not injected (e.g. CSR-only route, SSR skipped)', fakeAsync(() => {
191+
// Global intentionally absent; constructor should not throw and should not break later.
192+
delete (window as any).__dspaceRemoveSsrOverlay;
193+
194+
const f = TestBed.createComponent(AppComponent);
195+
expect(() => f.detectChanges()).not.toThrow();
196+
197+
mockStore.setState({ core: { auth: { loading: false, blocking: false } } });
198+
themeLoading$.next(false);
199+
tick(700);
200+
201+
expect(window.__dspaceRemoveSsrOverlay).toBeUndefined();
202+
discardPeriodicTasks();
203+
}));
204+
});
130205
});

src/app/app.component.ts

Lines changed: 134 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
import { distinctUntilChanged, take, withLatestFrom } from 'rxjs/operators';
1+
import { debounceTime, distinctUntilChanged, filter, startWith, switchMap, take, takeUntil, withLatestFrom } from 'rxjs/operators';
22
import { DOCUMENT, isPlatformBrowser } from '@angular/common';
33
import {
44
AfterViewInit,
55
ChangeDetectionStrategy,
66
Component,
77
HostListener,
88
Inject,
9+
NgZone,
10+
OnDestroy,
911
OnInit,
1012
PLATFORM_ID,
1113
} from '@angular/core';
@@ -16,7 +18,7 @@ import {
1618
Router,
1719
} from '@angular/router';
1820

19-
import { BehaviorSubject, Observable } from 'rxjs';
21+
import { BehaviorSubject, combineLatest, Observable, race, Subject, timer } from 'rxjs';
2022
import { select, Store } from '@ngrx/store';
2123
import { NgbModal, NgbModalConfig } from '@ng-bootstrap/ng-bootstrap';
2224
import { TranslateService } from '@ngx-translate/core';
@@ -38,10 +40,22 @@ import { distinctNext } from './core/shared/distinct-next';
3840
styleUrls: ['./app.component.scss'],
3941
changeDetection: ChangeDetectionStrategy.OnPush,
4042
})
41-
export class AppComponent implements OnInit, AfterViewInit {
43+
export class AppComponent implements OnInit, AfterViewInit, OnDestroy {
4244
notificationOptions;
4345
models;
4446

47+
/**
48+
* Emits on destroy to tear down the SSR-overlay-removal pipeline (gate subscription, the
49+
* MutationObserver and its debounce/cap timers all unsubscribe via `takeUntil(destroyed$)`).
50+
* AppComponent is the root component so this is mostly defensive + test hygiene.
51+
*/
52+
private destroyed$ = new Subject<void>();
53+
54+
/** SSR anti-flicker overlay (see src/index.html) removal tuning. */
55+
private readonly ssrOverlaySettleQuietMs = 600; // routed page is "done" after this long with no DOM change
56+
private readonly ssrOverlaySettleMaxMs = 10000; // backstop reveal (below index.html's 15s catastrophic net)
57+
private readonly ssrOverlayMinContentHeightPx = 200; // proves <ds-app> is no longer the empty shell
58+
4559
/**
4660
* Whether or not the authentication is currently blocking the UI
4761
*/
@@ -74,6 +88,7 @@ export class AppComponent implements OnInit, AfterViewInit {
7488
private cssService: CSSVariableService,
7589
private modalService: NgbModal,
7690
private modalConfig: NgbModalConfig,
91+
private ngZone: NgZone,
7792
) {
7893
this.notificationOptions = environment.notifications;
7994

@@ -82,13 +97,129 @@ export class AppComponent implements OnInit, AfterViewInit {
8297

8398
if (isPlatformBrowser(this.platformId)) {
8499
this.trackIdleModal();
100+
this.removeSsrOverlayWhenContentVisible();
85101
}
86102

87103
this.isThemeLoading$ = this.themeService.isThemeLoading$;
88104

89105
this.storeCSSVariables();
90106
}
91107

108+
/**
109+
* Drops the SSR mask overlay installed by the inline bootstrap script in src/index.html once the
110+
* routed CSR page has actually finished rendering. Picking the right moment is the whole problem,
111+
* and the two earlier attempts each fixed one symptom and reintroduced the other:
112+
*
113+
* - PR #1288 waited for `ApplicationRef.isStable`. No flicker, but isStable is held hostage by ANY
114+
* ongoing zone async — after an admin login the app keeps the zone busy (authz/widgets, periodic
115+
* polling, AAI/discojuice scripts) so isStable fires many seconds late (or hits the 15s
116+
* fallback). The inert snapshot then masks the live page -> "looks rendered but not interactive"
117+
* (issue #725).
118+
* - PR #1317 switched to the loader-swap gate `!isAuthenticationBlocking && !isThemeLoading` plus a
119+
* single requestAnimationFrame. Prompt, but that gate only un-hides `<router-outlet>`; the home
120+
* page then renders piecewise (navbar, search box, community list, recent submissions) as each
121+
* section's data arrives. Dropping the snapshot at the gate — or, as an earlier revision of this
122+
* method did, as soon as *some* content exists — exposes a half-built page that visibly pops
123+
* into place on a hard reload -> the flicker.
124+
*
125+
* The signal we actually need is "the routed page has stopped changing". So, after the gate opens,
126+
* we keep the snapshot until the live <ds-app> DOM has SETTLED (no element added/removed for a
127+
* short quiet window, with real content present). This stays decoupled from isStable (DOM-settle
128+
* ignores non-rendering background async, so admin reveals in a few seconds rather than ~15s).
129+
* See {@link routedPageReadyToReveal$}.
130+
*/
131+
private removeSsrOverlayWhenContentVisible(): void {
132+
const win: Window | undefined = this._window?.nativeWindow;
133+
if (!win || typeof win.__dspaceRemoveSsrOverlay !== 'function') {
134+
return; // SSR was skipped for this route, so no overlay was installed — nothing to remove
135+
}
136+
// Run outside Angular: a MutationObserver watching the whole app must not trigger change
137+
// detection (it would also keep ApplicationRef.isStable permanently false).
138+
this.ngZone.runOutsideAngular(() => {
139+
this.routedPageReadyToReveal$().pipe(
140+
takeUntil(this.destroyed$),
141+
).subscribe(() => {
142+
// one frame so the freshly rendered content is painted before the snapshot fades out
143+
this.runAfterNextFrame(win, () => win.__dspaceRemoveSsrOverlay?.());
144+
});
145+
});
146+
}
147+
148+
/**
149+
* Emits once when it is safe to drop the SSR snapshot: the auth/theme loader gate has opened
150+
* (same condition root.component.html uses to swap its fullscreen loader for the routed content)
151+
* AND the routed page's DOM has settled. See {@link dsAppDomSettled$}.
152+
*/
153+
private routedPageReadyToReveal$(): Observable<unknown> {
154+
const loaderGateOpen$ = combineLatest([
155+
this.store.pipe(select(isAuthenticationBlocking), distinctUntilChanged()),
156+
this.themeService.isThemeLoading$,
157+
]).pipe(
158+
filter(([authBlocking, themeLoading]: [boolean, boolean]) => !authBlocking && !themeLoading),
159+
take(1),
160+
);
161+
return loaderGateOpen$.pipe(
162+
switchMap(() => this.dsAppDomSettled$()),
163+
);
164+
}
165+
166+
/**
167+
* Emits once when the live <ds-app> subtree stops being mutated (elements added/removed) for
168+
* `ssrOverlaySettleQuietMs` AND it holds real content — or after `ssrOverlaySettleMaxMs`, whichever
169+
* comes first. The cap guarantees a page that never goes quiet (constant background DOM updates)
170+
* still reveals; the 15s fallback in index.html remains the ultimate net.
171+
*/
172+
private dsAppDomSettled$(): Observable<unknown> {
173+
const dsApp: Element | null = this.document.querySelector('ds-app');
174+
if (!dsApp) {
175+
return timer(this.ssrOverlaySettleMaxMs);
176+
}
177+
const elementMutations$ = new Observable<void>((subscriber) => {
178+
const observer = new MutationObserver((records) => {
179+
if (records.some((record) => this.isElementChildListChange(record))) {
180+
subscriber.next();
181+
}
182+
});
183+
observer.observe(dsApp, { childList: true, subtree: true });
184+
return () => observer.disconnect();
185+
});
186+
const settled$ = elementMutations$.pipe(
187+
startWith(undefined), // start the quiet window immediately
188+
debounceTime(this.ssrOverlaySettleQuietMs), // ... reset by each render, fires once quiet
189+
filter(() => this.dsAppHasRenderedContent(dsApp)), // ... but never on the empty shell
190+
);
191+
return race(settled$, timer(this.ssrOverlaySettleMaxMs)).pipe(take(1));
192+
}
193+
194+
/** True once the live <ds-app> is no longer the empty shell the overlay script left behind. */
195+
private dsAppHasRenderedContent(dsApp: Element): boolean {
196+
const height = dsApp.getBoundingClientRect?.().height ?? 0;
197+
return height >= this.ssrOverlayMinContentHeightPx && dsApp.querySelector('#main-content') !== null;
198+
}
199+
200+
/** A childList mutation that adds or removes at least one element node (ignores text/attr noise). */
201+
private isElementChildListChange(record: MutationRecord): boolean {
202+
if (record.type !== 'childList') {
203+
return false;
204+
}
205+
const changedNodes = [...Array.from(record.addedNodes), ...Array.from(record.removedNodes)];
206+
return changedNodes.some((node) => node.nodeType === Node.ELEMENT_NODE);
207+
}
208+
209+
/** Runs `callback` after the next paint (or synchronously if requestAnimationFrame is unavailable). */
210+
private runAfterNextFrame(win: Window, callback: () => void): void {
211+
if (typeof win.requestAnimationFrame === 'function') {
212+
win.requestAnimationFrame(() => callback());
213+
} else {
214+
callback();
215+
}
216+
}
217+
218+
ngOnDestroy(): void {
219+
this.destroyed$.next();
220+
this.destroyed$.complete();
221+
}
222+
92223
ngOnInit() {
93224
/** Implement behavior for interface {@link ModalBeforeDismiss} */
94225
this.modalConfig.beforeDismiss = async function () {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,18 @@ import { ThemeConfig } from '../../../config/theme.config';
44
import { isNotEmpty } from '../empty.util';
55

66
export function getMockThemeService(themeName = 'base', themes?: ThemeConfig[]): ThemeService {
7+
// getThemeName$ is a real method on ThemeService (called as getThemeName$()),
8+
// so it must stay a spy method that returns an Observable.
9+
// isThemeLoading$ is a real property getter on ThemeService, so it must be a
10+
// property on the mock - not a spy method - or AsyncPipe / combineLatest will
11+
// receive a function instead of a stream.
712
const spy = jasmine.createSpyObj('themeService', {
813
getThemeName: themeName,
914
getThemeName$: observableOf(themeName),
1015
getThemeConfigFor: undefined,
1116
listenForRouteChanges: undefined,
17+
}, {
18+
isThemeLoading$: observableOf(false),
1219
});
1320

1421
if (isNotEmpty(themes)) {

0 commit comments

Comments
 (0)