VSB-TUO/Fix post login non-interactive UI (#1317)

* Change is stable to is visible in remove ssr overlay

* Refactor tests based on changes

* Fix specs

---------

Co-authored-by: Matus Kasak <matus.kasak@dataquest.sk>

Author: Kasinhou

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<AppComponent>;
 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<boolean>;
+  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<boolean>;
+    let themeService: ThemeService;
     let originalRaF: typeof window.requestAnimationFrame;
 
     beforeEach(() => {
-      appRef = TestBed.inject(ApplicationRef);
-      isStable$ = new BehaviorSubject<boolean>(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<boolean>(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();
     }));
   });
 });

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

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)) {

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();