Fix home-page flicker AND close #725: interactive snapshot removed only after the routed DOM settles#1321
Merged
Conversation
…es (real home-page flicker) Follow-up to #1318. On a real instance (VSB / dev-6.pc), an incognito Ctrl+Shift+R still flickered: the deployed code dropped the SSR snapshot ~3.2s in, while the home page was only half-rendered (search box, community list and several navbar items not yet present, content showing "Recent Submissions") and then everything popped into place ~600ms later. Captured frame-by-frame against the live instance. Root cause: the home page renders piecewise (each section fetches its own data), so the previous "<ds-app> has #main-content and height>=200" heuristic was satisfied while the page was still building -> snapshot removed too early -> visible flicker. Fix: after the auth/theme gate opens, keep the snapshot until the real <ds-app> subtree has SETTLED -- no element added/removed for SETTLE_QUIET_MS (600ms) -- via a MutationObserver, requiring minimum content first and capped at SETTLE_MAX_MS (10s). This stays decoupled from ApplicationRef.isStable (DOM-settle ignores non-rendering background async), so it does not bring back the post-login non-interactive page (#725): admin reveals in ~5s, not ~15s. Validated against the live dev-6.pc instance by intercepting the overlay-removal and driving it with this condition: - anon reload : drops @ ~4.8s, page COMPLETE (search + community list + full navbar) - admin reload: drops @ ~5.0s (reason "settled", not the cap), page complete vs the deployed code dropping @ ~3.2-3.6s on a half-built page. Refs: dspace-customers#725, PR #1288, PR #1317, PR #1318 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…er WebDriver (close #725) Addresses an expert review of the DOM-settle change: DOM-settle alone shortened but did not eliminate issue #725, because the masking window still left the page non-interactive (the opaque overlay sat over a visibility:hidden <ds-app>, so clicks landed on nothing) and still produced duplicate DOM that breaks strict-mode E2E locators. Changes: - index.html: drop the `ds-app[data-dspace-ssr-hidden]{visibility:hidden}` rule and give the overlay min-height:100vh. The overlay is already opaque + pointer-events:none, so it still masks the CSR rebuild visually, but clicks now pass THROUGH it to the real, still-visible <ds-app> underneath. The page is therefore interactive the entire time it is masked -> #725 ("looks rendered but nothing is clickable") cannot recur, even if removal rides the 10s cap. The snapshot is marked aria-hidden so AT (and the duplicate-node concern) target the live app, not the snapshot. - index.html: also bypass the overlay for WebDriver runners (navigator.webdriver), mirroring the existing Cypress guard, so Playwright/Selenium see no overlay and no duplicate DOM (fixes the #725 strict-mode locator failure). - app.component.ts: exclude the admin-sidebar subtree from the DOM-settle MutationObserver (its long :enter/:leave animations would keep re-arming the quiet window and push admin logins toward the cap); add OnDestroy + takeUntil + observer/timer teardown. - Fix stale `ApplicationRef.isStable` / `removeSsrOverlayWhenStable` comments (index.html, typings.d.ts). Verified against the live build (Playwright, CPU/network throttled): - WebDriver run: overlay absent (no duplicate DOM). - webdriver spoofed false (real-user path): a navbar click WHILE the snapshot is still shown reaches the live <ds-app> (interactive under mask). - anon reveal settled, CLS after reveal = 0; admin reveal settled (reason "settled", not cap), CLS after reveal = 0. Refs: dspace-customers#725, PR #1288, PR #1317, PR #1318, PR #1321 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ebar hack (no behaviour change)
The overlay-removal logic was a hard-to-read imperative blob (mutable done/quietTimer/capTimer flags,
manual arm/re-arm, manual MutationObserver/teardown bookkeeping) plus a brittle string-selector hack
(`inAdminSidebar` -> `closest('ds-themed-admin-sidebar, ds-admin-sidebar')`).
Rewritten as small, named, single-responsibility pieces:
- removeSsrOverlayWhenContentVisible(): guard + runOutsideAngular + subscribe(takeUntil(destroyed$)).
- routedPageReadyToReveal$(): loader gate (take 1) -> switchMap(dsAppDomSettled$).
- dsAppDomSettled$(): MutationObserver wrapped as an Observable; settle = startWith + debounceTime
(the quiet window) + filter(real content); race() with timer() for the cap; take(1).
- dsAppHasRenderedContent(), isElementChildListChange(), runAfterNextFrame(): tiny pure helpers.
RxJS now owns debounce, the cap, and teardown (the Observable disconnects the observer on unsubscribe,
takeUntil(destroyed$) ends everything on destroy), so the mutable flags, manual timers and the separate
cancelOverlaySettle field are gone (net -45 lines).
Dropped the admin-sidebar exclusion entirely: it was a fragile, theme-coupled selector guarding a
problem that interactive-under-mask already makes harmless (riding the cap is fine when the page is
clickable throughout) and that does not occur in practice (admin still settles via "settled", not the
cap). Tuning constants moved to named readonly fields.
Behaviour re-verified on the live build (Playwright, throttled): interactive-under-mask still works;
anon + admin both reveal with reason "settled" (not cap) and CLS after reveal = 0.
Refs: dspace-customers#725, PR #1321
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
milanmajchrak
added a commit
that referenced
this pull request
Jun 25, 2026
…-in reload doesn't shift
Follow-up to the anti-flicker overlay: with the overlay in place, an authenticated user's hard reload
still showed the content visibly jump right by the admin-sidebar width at the moment the snapshot was
removed. Root cause is independent of the overlay (the overlay just makes it a visible "reveal"):
RootComponent computes the @slideSidebarPadding (outer-wrapper padding-left) from
`cssService.getVariable('--ds-admin-sidebar-fixed-element-width')`, but that store is only populated in
the browser (AppComponent.storeCSSVariables -> getComputedStyle). On the server the variable never
resolves, `skipWhile(!val)` blocks forever, and the SSR HTML renders `outer-wrapper { padding-left: 0 }`.
The browser then resolves the real width and applies `padding-left: 55px`, so the whole authenticated
page (and the SSR snapshot, which is just that server HTML) jumps right by the sidebar width on reveal.
Fix: on the server, fall back to the compiled default sidebar width (see $ds-admin-sidebar-* in
src/styles/_bootstrap_variables.scss) so the SSR layout already reserves the gutter; in the browser keep
waiting for the real, theme-overridable value. Non-admins are unaffected (state stays 'hidden' -> 0).
Verified locally (authenticated admin, real UA, throttled hard reload):
- SSR HTML outer-wrapper inline style is now `padding-left:55px` (was `padding-left:0`).
- snapshot vs settled `#main-content` x = 55px in both -> horizontal shift at reveal = 0px (was 55px).
Refs: dspace-customers#725, PR #1321
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
milanmajchrak
added a commit
that referenced
this pull request
Jun 25, 2026
…n reload doesn't shift Follow-up to #1321 (the anti-flicker SSR overlay, merged into `customer/vsb-tuo`). ## Problem With the overlay in place, an authenticated user's hard reload still showed the whole page jump right by the admin-sidebar width when the SSR snapshot was removed. (Anonymous users were already smooth — they have no sidebar.) ## Root cause (independent of the overlay; the overlay just makes it a visible "reveal") The `.outer-wrapper` left gutter came from the `@slideSidebarPadding` Angular animation, whose width is read via `cssService.getVariable('--ds-admin-sidebar-*')`. That store is populated only in the browser (`AppComponent.storeCSSVariables` -> `getComputedStyle`); on the server it stays empty, `skipWhile(!val)` blocks forever, and the animation renders `outer-wrapper { padding-left: 0 }`. The browser then resolves the real width (e.g. 55px), so the authenticated page (and the SSR snapshot, which is just that server HTML) jumps right on reveal. ## Fix Drive the gutter from a CSS class (`ds-admin-sidebar-{hidden,unpinned,pinned}`, set from a small `sidebarPaddingState$`) whose `padding-left` is `var(--ds-admin-sidebar-fixed-element-width)` / `--ds-admin-sidebar-total-width`. CSS resolves those custom properties identically on the server (the snapshot) and the browser (the live app), so they always match — **no hardcoded pixel width, fully theme-overridable and viewport/media-query aware** — and a `transition: padding-left` keeps the pin/unpin slide. The `@slideSidebarPadding` animation (which can't take a `var()` value and is stripped to empty on SSR) is no longer used here. Non-admins are unaffected ('hidden' -> padding 0). ## Verification (authenticated admin, real UA, throttled hard reload, local SSR build) - SSR HTML renders `<div class="outer-wrapper … ds-admin-sidebar-unpinned">` (class, no inline px). - snapshot vs settled `#main-content`: **desktop 55/55 -> shift 0px; mobile (375px) 55/55 -> shift 0px**. Refs: dspace-customers#725, #1321 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
milanmajchrak
added a commit
that referenced
this pull request
Jun 25, 2026
…n reload doesn't shift Follow-up to #1321 (the anti-flicker SSR overlay, merged into `customer/vsb-tuo`). ## Problem With the overlay in place, an authenticated user's hard reload still showed the whole page jump right by the admin-sidebar width when the SSR snapshot was removed. (Anonymous users were already smooth — they have no sidebar.) ## Root cause (independent of the overlay; the overlay just makes it a visible "reveal") The `.outer-wrapper` left gutter came from the `@slideSidebarPadding` Angular animation, whose width is read via `cssService.getVariable('--ds-admin-sidebar-*')`. That store is populated only in the browser (`AppComponent.storeCSSVariables` -> `getComputedStyle`); on the server it stays empty, `skipWhile(!val)` blocks forever, and the animation renders `outer-wrapper { padding-left: 0 }`. The browser then resolves the real width (e.g. 55px), so the authenticated page (and the SSR snapshot, which is just that server HTML) jumps right on reveal. ## Fix Drive the gutter from a CSS class (`ds-admin-sidebar-{hidden,unpinned,pinned}`, set from a small `sidebarPaddingState$`) whose `padding-left` is `var(--ds-admin-sidebar-fixed-element-width)` / `--ds-admin-sidebar-total-width`. CSS resolves those custom properties identically on the server (the snapshot) and the browser (the live app), so they always match — no hardcoded pixel width, fully theme-overridable and viewport/media-query aware. The `@slideSidebarPadding` animation (which can't take a `var()` value and is stripped to empty on SSR) is no longer used here; the unused import/registration is dropped from the base and custom-theme root components. Non-admins are unaffected ('hidden' -> 0). The pin/unpin slide is preserved via `transition: padding-left`, but GATED behind a `ds-admin-sidebar-animate` class enabled only after the first browser paint (`gutterTransitionEnabled`). This stops the initial SSR->CSR gutter resolution — which happens behind the overlay — from animating and leaking a 300ms slide right as the overlay is removed (the overlay's settle detector watches DOM mutations, not style changes). Addresses review feedback (3 independent SSR/DSpace reviewers). ## Verification (authenticated admin, real UA, throttled hard reload, local SSR build) - SSR HTML renders `<div class="outer-wrapper ds-admin-sidebar-unpinned">` — a class, no inline px, and no `ds-admin-sidebar-animate` (transition correctly off on the server / initial paint). - snapshot vs settled `#main-content`: desktop 55/55 -> shift 0px; mobile (375px) 55/55 -> shift 0px; the live app gains `ds-admin-sidebar-animate` only after first paint. Refs: dspace-customers#725, #1321 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
milanmajchrak
added a commit
that referenced
this pull request
Jun 29, 2026
…logged-in reload doesn't shift (#1333) Follow-up to #1321 (the anti-flicker SSR overlay, merged into `customer/vsb-tuo`). ## Problem With the overlay in place, an authenticated user's hard reload still showed the whole page jump right by the admin-sidebar width when the SSR snapshot was removed. (Anonymous users were already smooth — they have no sidebar.) ## Root cause (independent of the overlay; the overlay just makes it a visible "reveal") The `.outer-wrapper` left gutter came from the `@slideSidebarPadding` Angular animation, whose width is read via `cssService.getVariable('--ds-admin-sidebar-*')`. That store is populated only in the browser (`AppComponent.storeCSSVariables` -> `getComputedStyle`); on the server it stays empty, `skipWhile(!val)` blocks forever, and the animation renders `outer-wrapper { padding-left: 0 }`. The browser then resolves the real width (e.g. 55px), so the authenticated page (and the SSR snapshot, which is just that server HTML) jumps right on reveal. ## Fix Drive the gutter from a CSS class (`ds-admin-sidebar-{hidden,unpinned,pinned}`, set from a small `sidebarPaddingState$`) whose `padding-left` is `var(--ds-admin-sidebar-fixed-element-width)` / `--ds-admin-sidebar-total-width`. CSS resolves those custom properties identically on the server (the snapshot) and the browser (the live app), so they always match — no hardcoded pixel width, fully theme-overridable and viewport/media-query aware. The `@slideSidebarPadding` animation (which can't take a `var()` value and is stripped to empty on SSR) is no longer used here; the unused import/registration is dropped from the base and custom-theme root components. Non-admins are unaffected ('hidden' -> 0). The pin/unpin slide is preserved via `transition: padding-left`, but GATED behind a `ds-admin-sidebar-animate` class enabled only after the first browser paint (`gutterTransitionEnabled`). This stops the initial SSR->CSR gutter resolution — which happens behind the overlay — from animating and leaking a 300ms slide right as the overlay is removed (the overlay's settle detector watches DOM mutations, not style changes). Addresses review feedback (3 independent SSR/DSpace reviewers). ## Verification (authenticated admin, real UA, throttled hard reload, local SSR build) - SSR HTML renders `<div class="outer-wrapper ds-admin-sidebar-unpinned">` — a class, no inline px, and no `ds-admin-sidebar-animate` (transition correctly off on the server / initial paint). - snapshot vs settled `#main-content`: desktop 55/55 -> shift 0px; mobile (375px) 55/55 -> shift 0px; the live app gains `ds-admin-sidebar-animate` only after first paint. Refs: dspace-customers#725, #1321 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
jr-rk
added a commit
that referenced
this pull request
Jun 30, 2026
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>
jr-rk
added a commit
that referenced
this pull request
Jun 30, 2026
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>
jr-rk
added a commit
that referenced
this pull request
Jun 30, 2026
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>
jr-rk
added a commit
that referenced
this pull request
Jun 30, 2026
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>
jr-rk
added a commit
that referenced
this pull request
Jun 30, 2026
Rebuilds this follow-up on the current customer/vsb-tuo base (which now carries the final overlay mechanism #1318/#1321 and #1333). Drops the duplicate dspace eager-theme import (imported once as DSpaceEagerThemeModule and again, unaliased, as EagerThemeModule) so this file matches the canonical form on TUL/SAV/ZCU-DATA/ ZCU-PUB: dspace eager theme once + custom eager theme once. The earlier blank-page guard from this branch is intentionally dropped: #1321 removed the ds-app visibility:hidden rule (the overlay is now a purely visual mask), so the early-return can no longer hide the app, and the other customers took VSB-TUO's #1321 index.html verbatim. Keeping a guard only here would re-diverge index.html. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 30, 2026
jr-rk
added a commit
that referenced
this pull request
Jul 1, 2026
…1318, #1321) The original overlay removal waited for ApplicationRef.isStable, which is held hostage by post-login/admin zone activity (page looks rendered but is not interactive - dspace-customers#725). Replace it with the final mechanism used across the customer backports: keep the SSR snapshot until the routed <ds-app> DOM has settled (MutationObserver + quiet window, content-height / #main-content check, 10s cap), and make the overlay a purely visual mask so the live app stays interactive underneath. index.html / app.component.ts / spec / typings are synced to the final version; the existing ngAfterViewInit delay(0) is preserved. Keeping the trunk on the final mechanism means new customer branches cut from dtq-dev start correct instead of re-inheriting the old isStable approach. Ref: #1318, #1321 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jr-rk
added a commit
that referenced
this pull request
Jul 1, 2026
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>
milanmajchrak
pushed a commit
that referenced
this pull request
Jul 2, 2026
* 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: align eager-themes.module.ts with the other backport instances Bind the custom eager theme to the CustomEagerThemeModule alias used by the other customer backports so this file is byte-identical across instances. The same ./custom/eager-theme.module is still imported eagerly - no runtime, build, or bundle-size change; the custom theme stays eager (which also keeps the untyped-item theming working, ref DSpace#1897). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Build: raise initial bundle budget to 5.5mb/6mb to match the other backports Aligns ZCU-PUB's initial budget with the value the root fix (#1287) and the other customer backports use, so the budget block is identical across instances. The custom theme is already eager here, so this only widens the headroom; the build already passes under the previous 5mb error ceiling. 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>
milanmajchrak
pushed a commit
that referenced
this pull request
Jul 2, 2026
* 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>
milanmajchrak
pushed a commit
that referenced
this pull request
Jul 2, 2026
* 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/install/spec logs _build.log, _install.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>
milanmajchrak
pushed a commit
that referenced
this pull request
Jul 2, 2026
* AI 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> * 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>
milanmajchrak
pushed a commit
that referenced
this pull request
Jul 2, 2026
* 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>
milanmajchrak
pushed a commit
that referenced
this pull request
Jul 2, 2026
…r backports (#1320) Rebuilds this follow-up on the current customer/vsb-tuo base (which now carries the final overlay mechanism #1318/#1321 and #1333). Drops the duplicate dspace eager-theme import (imported once as DSpaceEagerThemeModule and again, unaliased, as EagerThemeModule) so this file matches the canonical form on TUL/SAV/ZCU-DATA/ ZCU-PUB: dspace eager theme once + custom eager theme once. The earlier blank-page guard from this branch is intentionally dropped: #1321 removed the ds-app visibility:hidden rule (the overlay is now a purely visual mask), so the early-return can no longer hide the app, and the other customers took VSB-TUO's #1321 index.html verbatim. Keeping a guard only here would re-diverge index.html. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
milanmajchrak
pushed a commit
that referenced
this pull request
Jul 2, 2026
* Fix home-page SSR->CSR flicker
Angular 15 has no provideClientHydration; on every browser load Angular
tears down the entire SSR DOM and rebuilds the component tree from scratch.
Measured CLS = 0.89 at t=1.76s on /home (PerformanceObserver on dev-5).
The visible flicker is that ~600ms rebuild window between SSR view and
populated CSR view.
Two compounding causes, addressed in this PR:
1. CustomEagerThemeModule was commented out in src/themes/eager-themes.module.ts,
so every custom-themed wrapper (footer, header, root, ...) was lazy-loaded via
webpack code-splitting on the browser, stretching the gap. Re-enable it
(the existing custom/eager-theme.module.ts already declares the right set).
Bumps initial bundle by ~256KB; angular.json budget raised from 5MB to 8MB
to accommodate.
2. The bigger cause - no hydration - is masked by an inline pre-bootstrap script
in src/index.html that:
- Captures all <style ng-transition="dspace-angular"> blocks into
<style data-dspace-ssr-keep> tags Angular won't strip (Angular removes
the originals on bootstrap, which is why a naive overlay renders
unstyled).
- Moves (not clones) the SSR-rendered <ds-app> children into an
absolute-positioned overlay so they keep every live DOM/style detail.
- Hides the now-empty <ds-app> via a data-attribute and CSS rule.
- Exposes window.__dspaceRemoveSsrOverlay() for AppComponent to call
once ApplicationRef.isStable fires (with one rAF + 50ms pad).
- 15s safety fallback in case isStable never fires.
Bots and no-JS users still get the original SSR <ds-app> (the overlay is
JS-added). Real users see continuous SSR-rendered content while CSR rebuilds
invisibly underneath, then a 150ms fade reveals the CSR DOM in its final
data-loaded state.
Verified locally via Service Worker that suppresses the removal: overlay's
header height is 80px (proper styling preserved) versus 698px (the unstyled
fallback before this fix's style-preservation step).
Includes a small Windows cmd deploy helper at scripts/dspace-deploy.bat and
matching skill doc at .claude/skills/dspace-deploy/SKILL.md - multi-instance
safe local dev stack via the existing docker compose files.
* Lint: curly braces around early-return if
* Address Copilot review
- angular.json: tighten budget back to 5.5MB warn / 6MB error (was 8MB)
- index.html: re-entrancy guard on __dspaceRemoveSsrOverlay (null the
pointer up-front so the isStable + 15s safety fallback can't double-fade)
- index.html: drop aria-hidden from overlay so screen-reader users get the
SSR snapshot during boot (ds-app underneath has visibility:hidden which
already excludes it from a11y tree)
- index.html: console.warn on the overlay-script catch so a silently broken
flicker fix is at least diagnosable in DevTools
- typings.d.ts: typed Window.__dspaceRemoveSsrOverlay augmentation; drop
the `as any` cast in AppComponent.removeSsrOverlayWhenStable
- app.component.spec.ts: cover removeSsrOverlayWhenStable (calls the
global once on isStable=true; no-op when global absent)
- Drop scripts/dspace-deploy.bat + .claude/skills/dspace-deploy/SKILL.md
from this PR per request (local dev tooling, will live elsewhere)
* Disable SSR overlay when Cypress is driving
The overlay holds the SSR-rendered children alongside <ds-app>'s CSR-rendered
children during the masking window. Cypress's cy.get(selector) sees both
copies, so unique-id selectors return 2 elements and cy.click() fails. The
overlay is purely a UX smoothing layer (no behaviour to E2E-validate), so
short-circuit when window.Cypress is present. Browser users are unaffected.
* Advance root flicker fix to the final DOM-settle overlay mechanism (#1318, #1321)
The original overlay removal waited for ApplicationRef.isStable, which is held
hostage by post-login/admin zone activity (page looks rendered but is not
interactive - dspace-customers#725). Replace it with the final mechanism used
across the customer backports: keep the SSR snapshot until the routed <ds-app>
DOM has settled (MutationObserver + quiet window, content-height / #main-content
check, 10s cap), and make the overlay a purely visual mask so the live app stays
interactive underneath. index.html / app.component.ts / spec / typings are synced
to the final version; the existing ngAfterViewInit delay(0) is preserved.
Keeping the trunk on the final mechanism means new customer branches cut from
dtq-dev start correct instead of re-inheriting the old isStable approach.
Ref: #1318, #1321
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: jm <jm@maz>
Co-authored-by: Juraj Roka <95219754+jr-rk@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Follow-up to #1318. On the real VSB instance (dev-6.pc), an incognito Ctrl+Shift+R still flickers — the home page visibly "preblikne". My earlier verification passed only because my local backend was too fast to reproduce the anonymous case; this time I reproduced and validated frame-by-frame against the live instance.
Root cause (different from #1318's assumption)
The home page renders piecewise — navbar, search box, community list and recent submissions each fetch their own data and appear at different times. #1318 dropped the SSR snapshot as soon as
<ds-app>had#main-content+ height ≥ 200px, which is already true while the page is still half-built. So on a hard reload the snapshot is removed mid-render, exposing an incomplete page that then pops into place.Deployed code drops the snapshot at ~3771ms — half-built (no search box, no community list, navbar collapsed, content jumped to "Recent Submissions"):
~600ms later the same page is complete (what the SSR snapshot already showed):
That jump between the two is the flicker.
Fix — wait for the routed DOM to settle
After the auth/theme gate opens, keep the snapshot until the real
<ds-app>subtree has had no element added/removed for 600ms (SETTLE_QUIET_MS) via aMutationObserver— i.e. the page finished rendering its sections — requiring a minimum amount of content first and capped at 10s (SETTLE_MAX_MS; the 15s fallback inindex.htmlstays as the ultimate net).Crucially this stays decoupled from
ApplicationRef.isStable(DOM-settle ignores non-rendering background async), so it does not bring back the post-login non-interactive page (#725).Review-driven update: issue #725 fully closed (interactive while masked)
Three Angular reviewers checked this for #725 regression risk. Key finding: DOM-settle alone only
shortened #725 — during the (now ~2-5s) masking window the page was still non-interactive,
because the opaque overlay sat over a
visibility:hidden<ds-app>(clicks landed on nothing), andthe duplicate snapshot DOM still tripped strict-mode E2E locators. So two more changes were made:
pointer-events:none; the only thingthat killed interaction was a redundant
visibility:hiddenrule on<ds-app>. Dropping it (andgiving the overlay
min-height:100vh) means clicks pass through the opaque snapshot to thereal, still-visible app underneath. The page is interactive the entire time it is masked, so
import-TUL-dev5 #725 ("looks rendered but nothing is clickable") cannot recur — even if removal rides the 10s cap.
The snapshot is
aria-hiddenso AT/locators target the live app.navigator.webdriver), mirroring the existing Cypressguard, so Playwright/Selenium see no overlay and no duplicate DOM (the import-TUL-dev5 #725 strict-mode failure).
quiet window and push admin toward the cap); add
OnDestroy/takeUntilteardown.Verified on the live build (Playwright, throttled):
<ds-app>-> interactive under maskSo the flicker (CLS -> 0) and #725 (interactive throughout, no duplicate DOM for E2E) are both
resolved together, rather than traded against each other.
This is the original #1287 flicker, measured with #1287's own metric (CLS)
PR #1287 defined the problem as a layout shift — it reported CLS 0.89 — and its goal was to "reveal the CSR DOM in its final data-loaded state." Revealing early (deployed) is a visible layout shift; revealing once the DOM has settled is the final data-loaded state. Measured on dev-6.pc, the layout shift that occurs after the snapshot is removed (what the user actually sees):
DOM-settle is exactly #1287's "final data-loaded state" detector — what #1288 approximated with
isStable(too slow) and #1317 dropped (revealing early -> CLS back). The fix restores #1287's intent and drives the post-reveal CLS to 0.Validation (against the live dev-6.pc instance)
I intercepted the overlay-removal call and drove it with the new DOM-settle condition, screencasting the result:
DOM-settle drops the snapshot only once the page is complete:
Screencast videos: deployed (flicker) · DOM-settle (no flicker).
Robustness across instances (not tuned to one)
DOM-settle reacts to each load's actual DOM, so the reveal time is adaptive, not a fixed clock. Stress-tested on dev-6.pc from Fast-3G to a pathological 700ms-latency / 350kbps profile (anon):
The reveal time scales with how slow the instance is; CLS stays 0 throughout, so the fix is not tuned to one instance's speed. The only constants are the 600ms "DOM has stopped changing" debounce and the 10s cap (below index.html's 15s net). The 600ms held even at 700ms latency because DSpace issues the home-page section requests in parallel, so the gaps between section renders stay small. Theoretical edge: a single section rendering >600ms after everything else already settled could reveal a touch early - bounded by the cap.
"No flicker", measurably
At the instant the snapshot is removed, the real
<ds-app>DOM has been quiet for ≥ 600ms (the page stopped changing) — so the revealed page matches what the snapshot showed; no element appears/disappears afterward.Verify after deploy
The change is built and unit-tested locally, but the flicker only manifests on a real (slower) instance. Please redeploy this to dev-6.pc; I'll then re-run the live frame capture against the deployed build to confirm the snapshot now drops only on the complete page.
Refs: dspace-customers#725, PR #1288, PR #1317, PR #1318
🤖 Generated with Claude Code