VSB-TUO/Cherry-pick: Fix home-page SSR->CSR flicker#1288
Merged
milanmajchrak merged 5 commits intoMay 20, 2026
Conversation
added 4 commits
May 19, 2026 14:06
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.
- 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)
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.
There was a problem hiding this comment.
Pull request overview
Cherry-picks a fix intended to reduce the visible SSR→CSR “flash/flicker” on /home in this Angular 15 + Universal app by keeping SSR-rendered content visible until the client app stabilizes.
Changes:
- Adds an inline “SSR overlay” bootstrap script + styles in
index.html, and removes the overlay onceApplicationRef.isStableemitstrue. - Re-enables eager-loading of the
customtheme wrappers to avoid lazy-load gaps during client bootstrap. - Updates typings/tests to support and validate the overlay-removal hook; adjusts Angular bundle budgets and ignores local tooling artifacts.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/typings.d.ts |
Adds a Window typing for the overlay removal global function. |
src/themes/eager-themes.module.ts |
Includes CustomEagerThemeModule to avoid theme-wrapper lazy-load gaps during bootstrap. |
src/index.html |
Adds SSR-overlay CSS + inline pre-bootstrap script to mask SSR→CSR rebuild flicker. |
src/app/app.component.ts |
Calls the overlay removal global once Angular reaches first stable state. |
src/app/app.component.spec.ts |
Adds unit tests around overlay removal timing/guard behavior. |
angular.json |
Raises initial bundle size budgets to accommodate eager theme loading. |
.gitignore |
Ignores local deploy script artifacts and Playwright investigation outputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 tasks
milanmajchrak
added a commit
that referenced
this pull request
Jun 22, 2026
… on isStable or bare auth gate) The SSR anti-flicker overlay (PR #1288) is removed by AppComponent. Two prior approaches each fixed one symptom and reintroduced the other: - #1288 waited for ApplicationRef.isStable: no flicker, but admin sessions keep the zone busy (authz/widgets, polling, AAI scripts) so isStable fires many seconds late (or hits the 15s fallback), leaving the live, already-rendered page masked & non-interactive (dspace-customers#725). - #1317 switched to the loader-swap gate (!isAuthenticationBlocking && !isThemeLoading) + a single requestAnimationFrame: fast/interactive, but the gate only un-hides <router-outlet> and one rAF runs before paint, so the snapshot is dropped over an empty <ds-app> -> the flicker returned. Neither signal means "the routed content is painted AND the app is interactive": isStable over-waits (couples removal to unrelated background async); the auth/theme gate under-waits (decoupled from actual content paint). This keeps #1317's decoupling from isStable but, after the gate opens, waits across animation frames until the real <ds-app> is actually laid out (height >= 200px AND its #main-content host present) before removing the overlay, capped at ~3s (MAX_FRAMES) so background async can never hold it open. The 15s fallback in index.html stays as the catastrophic-error net. Verified (DSpace 7.6.5 backend, CPU 4x, hard reload, admin session): - #1288: TTI 15963ms (page masked ~13s) #1317: ds-app height 0 at removal (217ms flash) - fix: TTI ~3.1-3.4s, ds-app height 5281px at removal, gap <= 0 (no flash), 3x deterministic; anon reload also no-flicker; 0 CORS and 0 SSR/hydration/NG0 console errors. Verification videos are linked in the PR description. Refs: dspace-customers#725, PR #1288, PR #1317 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
milanmajchrak
added a commit
that referenced
this pull request
Jun 22, 2026
… on isStable or bare auth gate) (#1318) The SSR anti-flicker overlay (PR #1288) is removed by AppComponent. Two prior approaches each fixed one symptom and reintroduced the other: - #1288 waited for ApplicationRef.isStable: no flicker, but admin sessions keep the zone busy (authz/widgets, polling, AAI scripts) so isStable fires many seconds late (or hits the 15s fallback), leaving the live, already-rendered page masked & non-interactive (dspace-customers#725). - #1317 switched to the loader-swap gate (!isAuthenticationBlocking && !isThemeLoading) + a single requestAnimationFrame: fast/interactive, but the gate only un-hides <router-outlet> and one rAF runs before paint, so the snapshot is dropped over an empty <ds-app> -> the flicker returned. Neither signal means "the routed content is painted AND the app is interactive": isStable over-waits (couples removal to unrelated background async); the auth/theme gate under-waits (decoupled from actual content paint). This keeps #1317's decoupling from isStable but, after the gate opens, waits across animation frames until the real <ds-app> is actually laid out (height >= 200px AND its #main-content host present) before removing the overlay, capped at ~3s (MAX_FRAMES) so background async can never hold it open. The 15s fallback in index.html stays as the catastrophic-error net. Verified (DSpace 7.6.5 backend, CPU 4x, hard reload, admin session): - #1288: TTI 15963ms (page masked ~13s) #1317: ds-app height 0 at removal (217ms flash) - fix: TTI ~3.1-3.4s, ds-app height 5281px at removal, gap <= 0 (no flash), 3x deterministic; anon reload also no-flicker; 0 CORS and 0 SSR/hydration/NG0 console errors. Verification videos are linked in the PR description. Refs: dspace-customers#725, PR #1288, PR #1317 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
milanmajchrak
added a commit
that referenced
this pull request
Jun 23, 2026
…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>
milanmajchrak
added a commit
that referenced
this pull request
Jun 23, 2026
…moved only after the routed DOM settles (#1321) * fix(ssr-overlay): remove overlay only after the routed page DOM settles (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> * fix(ssr-overlay): make the masked page interactive + skip overlay under 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> * refactor(ssr-overlay): declarative DOM-settle, drop fragile admin-sidebar 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> --------- 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 description
Cherry-pick from #1287
Sync verification
If en.json5 or cs.json5 translation files were updated:
yarn run sync-i18n -t src/assets/i18n/cs.json5 -ito synchronize messages, and changes are included in this PR.Manual Testing (if applicable)
Copilot review