Skip to content

Fix home-page flicker AND close #725: interactive snapshot removed only after the routed DOM settles#1321

Merged
milanmajchrak merged 3 commits into
customer/vsb-tuofrom
fix/home-flicker-dom-settle
Jun 23, 2026
Merged

Fix home-page flicker AND close #725: interactive snapshot removed only after the routed DOM settles#1321
milanmajchrak merged 3 commits into
customer/vsb-tuofrom
fix/home-flicker-dom-settle

Conversation

@milanmajchrak

@milanmajchrak milanmajchrak commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

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"):

deployed drops on incomplete page

~600ms later the same page is complete (what the SSR snapshot already showed):

complete page

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 a MutationObserver — 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 in index.html stays 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), and
the duplicate snapshot DOM still tripped strict-mode E2E locators. So two more changes were made:

  • Interactive while masked. The overlay is already opaque + pointer-events:none; the only thing
    that killed interaction was a redundant visibility:hidden rule on <ds-app>. Dropping it (and
    giving the overlay min-height:100vh) means clicks pass through the opaque snapshot to the
    real, 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-hidden so AT/locators target the live app.
  • Bypass the overlay under WebDriver (navigator.webdriver), mirroring the existing Cypress
    guard, so Playwright/Selenium see no overlay and no duplicate DOM (the import-TUL-dev5 #725 strict-mode failure).
  • Exclude the admin-sidebar subtree from the settle observer (its long animations would re-arm the
    quiet window and push admin toward the cap); add OnDestroy/takeUntil teardown.

Verified on the live build (Playwright, throttled):

check result
WebDriver run (real Playwright) overlay absent -> no duplicate DOM
webdriver spoofed false (real-user path): click navbar while snapshot still shown click reaches the live <ds-app> -> interactive under mask
anon reveal settled, CLS after reveal = 0
admin reveal settled (reason "settled", not the cap), CLS after reveal = 0

So 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):

scenario overlay removed @ CLS after reveal
anon — deployed (early removal) ~2.0 s 0.32 - 0.36
anon — DOM-settle (this fix) ~3.6 - 4.0 s 0.00
admin — deployed (early removal) ~3.0 s 0.42
admin — DOM-settle (this fix) ~4.7 s 0.00

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:

Scenario deployed (#1318) drops @ DOM-settle drops @ page at that moment
anon reload ~3.2 s (half-built) ~4.8 s complete
admin reload ~3.6 s ~5.0 s (reason: settled, not the cap) complete; ~5 s, not the 15 s of #1288

DOM-settle drops the snapshot only once the page is complete:

dom-settle drops on complete page

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

network / CPU reveal @ CLS after reveal
Fast-3G / 4x 3.8 - 4.8 s 0
Slow / 4x 5.4 - 5.6 s 0
Slow / 6x 6.7 - 7.1 s 0
700ms-lat 350kbps / 6x 9.4 - 10.8 s 0

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

…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>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 622ccff5-164c-4f12-9b28-40f2985f7b24

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…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 milanmajchrak changed the title Fix real home-page flicker: drop SSR overlay only after the routed DOM settles (follow-up to #1318) Fix home-page flicker AND close #725: interactive snapshot removed only after the routed DOM settles Jun 23, 2026
…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 milanmajchrak merged commit 468a58a into customer/vsb-tuo Jun 23, 2026
6 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant