diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index 9294f1ff1dd..79db5691437 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -1,6 +1,6 @@ import { Store, StoreModule } from '@ngrx/store'; -import { ComponentFixture, fakeAsync, flush, inject, TestBed, tick, waitForAsync } from '@angular/core/testing'; -import { ApplicationRef, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; +import { ComponentFixture, discardPeriodicTasks, fakeAsync, flush, inject, TestBed, waitForAsync } from '@angular/core/testing'; +import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; import { CommonModule } from '@angular/common'; import { ActivatedRoute, Router } from '@angular/router'; import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; @@ -31,7 +31,7 @@ import { Angulartics2DSpace } from './statistics/angulartics/dspace-provider'; import { storeModuleConfig } from './app.reducer'; import { LocaleService } from './core/locale/locale.service'; import { authReducer } from './core/auth/auth.reducer'; -import { provideMockStore } from '@ngrx/store/testing'; +import { MockStore, provideMockStore } from '@ngrx/store/testing'; import { ThemeService } from './shared/theme-support/theme.service'; import { getMockThemeService } from './shared/mocks/theme-service.mock'; import { BreadcrumbsService } from './breadcrumbs/breadcrumbs.service'; @@ -42,7 +42,7 @@ let comp: AppComponent; let fixture: ComponentFixture; const menuService = new MenuServiceStub(); const initialState = { - core: { auth: { loading: false } } + core: { auth: { loading: false, blocking: false } } }; export function getMockLocaleService(): LocaleService { @@ -129,20 +129,22 @@ describe('App component', () => { }); - describe('removeSsrOverlayWhenStable', () => { - // The inline bootstrap script in src/index.html injects window.__dspaceRemoveSsrOverlay - // and AppComponent must call it exactly once when ApplicationRef.isStable first emits true. - let appRef: ApplicationRef; - let isStable$: BehaviorSubject; + 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. + let mockStore: MockStore; + let themeLoading$: BehaviorSubject; + let themeService: ThemeService; let originalRaF: typeof window.requestAnimationFrame; beforeEach(() => { - appRef = TestBed.inject(ApplicationRef); - isStable$ = new BehaviorSubject(false); - // Patch isStable to our controllable subject for this test only - Object.defineProperty(appRef, 'isStable', { value: isStable$.asObservable() }); + mockStore = TestBed.inject(MockStore); + themeService = TestBed.inject(ThemeService); + themeLoading$ = new BehaviorSubject(true); + (themeService as any).isThemeLoading$ = themeLoading$.asObservable(); + mockStore.setState({ core: { auth: { loading: false, blocking: true } } }); - // Force rAF to a synchronous shim so we can flush() through the chain deterministically. + // Force rAF to a synchronous shim so assertions are deterministic. originalRaF = window.requestAnimationFrame; (window as any).requestAnimationFrame = (cb: FrameRequestCallback) => { cb(0); @@ -155,21 +157,22 @@ describe('App component', () => { delete (window as any).__dspaceRemoveSsrOverlay; }); - it('removes the overlay once isStable emits true', fakeAsync(() => { + it('removes the overlay once auth is unblocked and theme loading is finished', fakeAsync(() => { const spy = jasmine.createSpy('__dspaceRemoveSsrOverlay'); window.__dspaceRemoveSsrOverlay = spy; - // Re-construct so the constructor-time subscription picks up our patched isStable + global. + // Re-construct so constructor-time subscription picks up our patched streams + global. const f = TestBed.createComponent(AppComponent); f.detectChanges(); expect(spy).not.toHaveBeenCalled(); - isStable$.next(true); - tick(50); // matches the 50ms pad after rAF in removeSsrOverlayWhenStable + mockStore.setState({ core: { auth: { loading: false, blocking: false } } }); + themeLoading$.next(false); flush(); expect(spy).toHaveBeenCalledTimes(1); + discardPeriodicTasks(); })); it('is a no-op when the global is not injected (e.g. CSR-only route, SSR skipped)', fakeAsync(() => { @@ -179,11 +182,12 @@ describe('App component', () => { const f = TestBed.createComponent(AppComponent); expect(() => f.detectChanges()).not.toThrow(); - isStable$.next(true); - tick(50); + mockStore.setState({ core: { auth: { loading: false, blocking: false } } }); + themeLoading$.next(false); flush(); expect(window.__dspaceRemoveSsrOverlay).toBeUndefined(); + discardPeriodicTasks(); })); }); }); diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 8200033fc20..9e6e0f150b5 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -2,7 +2,6 @@ import { distinctUntilChanged, filter, first, take, withLatestFrom, delay } from import { DOCUMENT, isPlatformBrowser } from '@angular/common'; import { AfterViewInit, - ApplicationRef, ChangeDetectionStrategy, Component, HostListener, @@ -18,7 +17,7 @@ import { Router, } from '@angular/router'; -import { BehaviorSubject, Observable } from 'rxjs'; +import { BehaviorSubject, combineLatest, Observable } from 'rxjs'; import { select, Store } from '@ngrx/store'; import { NgbModal, NgbModalConfig } from '@ng-bootstrap/ng-bootstrap'; import { TranslateService } from '@ngx-translate/core'; @@ -76,7 +75,6 @@ export class AppComponent implements OnInit, AfterViewInit { private cssService: CSSVariableService, private modalService: NgbModal, private modalConfig: NgbModalConfig, - private appRef: ApplicationRef, private ngZone: NgZone, ) { this.notificationOptions = environment.notifications; @@ -86,7 +84,7 @@ export class AppComponent implements OnInit, AfterViewInit { if (isPlatformBrowser(this.platformId)) { this.trackIdleModal(); - this.removeSsrOverlayWhenStable(); + this.removeSsrOverlayWhenContentVisible(); } this.isThemeLoading$ = this.themeService.isThemeLoading$; @@ -95,33 +93,38 @@ export class AppComponent implements OnInit, AfterViewInit { } /** - * Drops the SSR mask overlay installed by the inline bootstrap script in src/index.html as soon - * as Angular reaches its first stable state. The overlay is the only thing the user sees while - * Angular 15 rebuilds the SSR DOM; removing it too early would expose the rebuild flicker, too - * late would feel sluggish. We add a short safety pad to let the first paint settle, and there - * is also a 15s hard fallback inside the script itself in case isStable never fires. + * Drops the SSR mask overlay installed by the inline bootstrap script in src/index.html the + * moment the real CSR content is actually visible. We do NOT wait for ApplicationRef.isStable + * (which can be delayed many seconds by ongoing zone tasks, e.g. admin-only background HTTP + * polling, periodic timers, third-party AAI/discojuice scripts). Instead we react to the same + * condition root.component.html uses to swap the fullscreen loader for the real content: + * `!isAuthenticationBlocking && !isThemeLoading`. At that exact point the routed page is + * rendered, so removing the SSR snapshot does not produce flicker. One rAF delay lets the + * change-detection result commit to the DOM before the overlay fades. */ - private removeSsrOverlayWhenStable(): void { + private removeSsrOverlayWhenContentVisible(): void { const w: Window | undefined = this._window?.nativeWindow; if (!w || typeof w.__dspaceRemoveSsrOverlay !== 'function') { return; } - // run outside Angular so we don't keep changeDetection ticking on the overlay timer + // run outside Angular so the subscription does not keep change detection alive this.ngZone.runOutsideAngular(() => { - this.appRef.isStable.pipe( - filter((stable: boolean) => stable), + combineLatest([ + this.store.pipe(select(isAuthenticationBlocking), distinctUntilChanged()), + this.themeService.isThemeLoading$, + ]).pipe( + filter(([blocking, themeLoading]: [boolean, boolean]) => !blocking && !themeLoading), first(), ).subscribe(() => { - // one rAF + small pad to let the first stable paint commit before fading the overlay const remove = () => { if (typeof w.__dspaceRemoveSsrOverlay === 'function') { w.__dspaceRemoveSsrOverlay(); } }; if (typeof w.requestAnimationFrame === 'function') { - w.requestAnimationFrame(() => setTimeout(remove, 50)); + w.requestAnimationFrame(remove); } else { - setTimeout(remove, 50); + remove(); } }); }); diff --git a/src/app/shared/mocks/theme-service.mock.ts b/src/app/shared/mocks/theme-service.mock.ts index 3997d175047..af3014605c0 100644 --- a/src/app/shared/mocks/theme-service.mock.ts +++ b/src/app/shared/mocks/theme-service.mock.ts @@ -4,11 +4,18 @@ import { ThemeConfig } from '../../../config/theme.config'; import { isNotEmpty } from '../empty.util'; export function getMockThemeService(themeName = 'base', themes?: ThemeConfig[]): ThemeService { + // getThemeName$ is a real method on ThemeService (called as getThemeName$()), + // so it must stay a spy method that returns an Observable. + // isThemeLoading$ is a real property getter on ThemeService, so it must be a + // property on the mock - not a spy method - or AsyncPipe / combineLatest will + // receive a function instead of a stream. const spy = jasmine.createSpyObj('themeService', { getThemeName: themeName, getThemeName$: observableOf(themeName), getThemeConfigFor: undefined, listenForRouteChanges: undefined, + }, { + isThemeLoading$: observableOf(false), }); if (isNotEmpty(themes)) { diff --git a/src/index.html b/src/index.html index 14a782b7f5e..3801f720208 100644 --- a/src/index.html +++ b/src/index.html @@ -117,7 +117,9 @@ }, 200); }; - // Safety net: if the app never reaches isStable (e.g. permanent HTTP poll), remove anyway. + // Hard safety net: only fires on catastrophic JS errors that prevent AppComponent from + // running its overlay-removal logic. Under normal operation the overlay is removed + // event-driven (no timeout) the moment the real CSR content becomes visible. setTimeout(function () { if (typeof window.__dspaceRemoveSsrOverlay === 'function') { window.__dspaceRemoveSsrOverlay();