ZCU-DATA/Backport: Fix home-page SSR->CSR flicker#1312
Conversation
|
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 |
There was a problem hiding this comment.
Pull request overview
Backports the SSR→CSR “home-page flicker” mitigation by keeping SSR-rendered DOM visible during Angular 15 bootstrap and ensuring custom-themed wrappers are eagerly loaded to reduce the teardown/rebuild gap.
Changes:
- Re-enable
CustomEagerThemeModulein the eager themes bundle to avoid custom theme wrappers being lazy-loaded during initial render. - Add an inline SSR-overlay bootstrap script + CSS in
src/index.html, and hook its removal toApplicationRef.isStableinAppComponent. - Add typings/tests for the new
window.__dspaceRemoveSsrOverlayglobal and raise Angular initial bundle budgets.
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/typings.d.ts |
Adds a typed Window.__dspaceRemoveSsrOverlay global declaration used by the overlay removal hook. |
src/themes/eager-themes.module.ts |
Ensures the custom eager theme module is included to reduce SSR→CSR visual gaps. |
src/index.html |
Introduces the inline overlay/CSS that preserves SSR DOM & styles until Angular becomes stable. |
src/app/app.component.ts |
Removes the overlay when ApplicationRef.isStable first becomes true (browser-only). |
src/app/app.component.spec.ts |
Adds unit tests verifying the overlay removal callback behavior and no-op behavior when absent. |
angular.json |
Increases initial bundle size budgets to account for the added eager theme/module weight. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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>
_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>
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>
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>
|
Switched the SSR overlay-removal trigger from
|
Problem description
Backport from #1287
Analysis
(Write here, if there is needed describe some specific problem. Erase it, when it is not needed.)
Problems
(Write here, if some unexpected problems occur during solving issues. Erase it, when it is not needed.)
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