diff --git a/.gitignore b/.gitignore index bdab34cb367..92a4bb3dd42 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,9 @@ /tsd_typings/ npm-debug.log +# build/install/spec logs emitted by local deploy tooling +/_*.log + /build/ /coverage diff --git a/angular.json b/angular.json index bf3dd88c524..2f49e105344 100644 --- a/angular.json +++ b/angular.json @@ -100,8 +100,8 @@ "budgets": [ { "type": "initial", - "maximumWarning": "3mb", - "maximumError": "5mb" + "maximumWarning": "5.5mb", + "maximumError": "6mb" }, { "type": "anyComponentStyle", diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index e921c67acea..5d18f76b806 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -1,9 +1,10 @@ import { Store, StoreModule } from '@ngrx/store'; -import { ComponentFixture, 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'; import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; +import { BehaviorSubject } from 'rxjs'; // Load the implementations that should be tested import { AppComponent } from './app.component'; @@ -30,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'; @@ -41,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 { @@ -127,4 +128,78 @@ describe('App component', () => { }); }); + + describe('removeSsrOverlayWhenContentVisible', () => { + // The inline bootstrap script in src/index.html injects window.__dspaceRemoveSsrOverlay. + // 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); + themeService = TestBed.inject(ThemeService); + themeLoading$ = new BehaviorSubject(true); + (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) => { + cb(0); + return 0 as any; + }; + }); + + 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/theme are ready AND the DOM has settled', fakeAsync(() => { + const spy = jasmine.createSpy('__dspaceRemoveSsrOverlay'); + window.__dspaceRemoveSsrOverlay = spy; + + // Re-construct so constructor-time subscription picks up our patched streams + global. + const f = TestBed.createComponent(AppComponent); + f.detectChanges(); + + expect(spy).not.toHaveBeenCalled(); + + mockStore.setState({ core: { auth: { loading: false, blocking: false } } }); + themeLoading$.next(false); + + // 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(); + })); + + it('is a no-op when the global is not injected (e.g. CSR-only route, SSR skipped)', fakeAsync(() => { + // Global intentionally absent; constructor should not throw and should not break later. + delete (window as any).__dspaceRemoveSsrOverlay; + + const f = TestBed.createComponent(AppComponent); + expect(() => f.detectChanges()).not.toThrow(); + + mockStore.setState({ core: { auth: { loading: false, blocking: false } } }); + themeLoading$.next(false); + tick(700); + + expect(window.__dspaceRemoveSsrOverlay).toBeUndefined(); + discardPeriodicTasks(); + })); + }); }); diff --git a/src/app/app.component.ts b/src/app/app.component.ts index ba7b7382278..87c020aff05 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -1,4 +1,4 @@ -import { distinctUntilChanged, take, withLatestFrom } from 'rxjs/operators'; +import { debounceTime, distinctUntilChanged, filter, startWith, switchMap, take, takeUntil, withLatestFrom } from 'rxjs/operators'; import { DOCUMENT, isPlatformBrowser } from '@angular/common'; import { AfterViewInit, @@ -6,6 +6,8 @@ import { Component, HostListener, Inject, + NgZone, + OnDestroy, OnInit, PLATFORM_ID, } from '@angular/core'; @@ -16,7 +18,7 @@ import { Router, } from '@angular/router'; -import { BehaviorSubject, 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'; @@ -38,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 */ @@ -74,6 +88,7 @@ export class AppComponent implements OnInit, AfterViewInit { private cssService: CSSVariableService, private modalService: NgbModal, private modalConfig: NgbModalConfig, + private ngZone: NgZone, ) { this.notificationOptions = environment.notifications; @@ -82,6 +97,7 @@ export class AppComponent implements OnInit, AfterViewInit { if (isPlatformBrowser(this.platformId)) { this.trackIdleModal(); + this.removeSsrOverlayWhenContentVisible(); } this.isThemeLoading$ = this.themeService.isThemeLoading$; @@ -89,6 +105,121 @@ export class AppComponent implements OnInit, AfterViewInit { this.storeCSSVariables(); } + /** + * Drops the SSR mask overlay installed by the inline bootstrap script in src/index.html once the + * 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`. 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. + * + * 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 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: a MutationObserver watching the whole app must not trigger change + // detection (it would also keep ApplicationRef.isStable permanently false). + this.ngZone.runOutsideAngular(() => { + this.routedPageReadyToReveal$().pipe( + takeUntil(this.destroyed$), + ).subscribe(() => { + // one frame so the freshly rendered content is painted before the snapshot fades out + this.runAfterNextFrame(win, () => win.__dspaceRemoveSsrOverlay?.()); + }); + }); + } + + /** + * 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 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(); + } + }); + 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 { + 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/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 74fc0c9861b..5eb827c8d7f 100644 --- a/src/index.html +++ b/src/index.html @@ -7,12 +7,146 @@ DSpace + + diff --git a/src/themes/eager-themes.module.ts b/src/themes/eager-themes.module.ts index 4a46595f358..29d46032de8 100644 --- a/src/themes/eager-themes.module.ts +++ b/src/themes/eager-themes.module.ts @@ -1,6 +1,6 @@ import { NgModule } from '@angular/core'; import { EagerThemeModule as DSpaceEagerThemeModule } from './dspace/eager-theme.module'; -// import { EagerThemeModule as CustomEagerThemeModule } from './custom/eager-theme.module'; +import { EagerThemeModule as CustomEagerThemeModule } from './custom/eager-theme.module'; /** * This module bundles the eager theme modules for all available themes. @@ -8,11 +8,16 @@ import { EagerThemeModule as DSpaceEagerThemeModule } from './dspace/eager-theme * and entry components (to ensure their decorators get picked up). * * Themes that aren't in use should not be imported here so they don't take up unnecessary space in the main bundle. + * + * NOTE: CustomEagerThemeModule is included to prevent the home-page flicker that occurs when + * the active theme is `custom`. Without it, every themed wrapper (footer, header, root, ...) is + * lazy-loaded via webpack code-splitting on the browser, leaving visible gaps after the SSR DOM + * is torn down and before the CSR DOM is materialised. */ @NgModule({ imports: [ DSpaceEagerThemeModule, - // CustomEagerThemeModule, + CustomEagerThemeModule, ], }) export class EagerThemesModule { diff --git a/src/typings.d.ts b/src/typings.d.ts index c1c86511f88..445b5ecd2c7 100644 --- a/src/typings.d.ts +++ b/src/typings.d.ts @@ -86,3 +86,13 @@ declare module '*.scss' { const content: any; export default content; } + +/** + * Window global injected by the inline anti-flicker bootstrap script in `src/index.html`. + * 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; +}