Skip to content

fix(ssr-overlay): render admin-sidebar gutter on the server so logged-in reload doesn't shift#1333

Merged
milanmajchrak merged 1 commit into
customer/vsb-tuofrom
fix/admin-sidebar-ssr-padding
Jun 29, 2026
Merged

fix(ssr-overlay): render admin-sidebar gutter on the server so logged-in reload doesn't shift#1333
milanmajchrak merged 1 commit into
customer/vsb-tuofrom
fix/admin-sidebar-ssr-padding

Conversation

@milanmajchrak

@milanmajchrak milanmajchrak commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #1321 (the anti-flicker SSR overlay, already 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.storeCSSVariablesgetComputedStyle); 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 — no hardcoded width

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 — theme-overridable and viewport/media-query aware, with zero magic pixels — and a transition: padding-left keeps the pin/unpin slide. The @slideSidebarPadding animation isn't used here anymore (it can't take a var() value — Angular strips it to empty on SSR — which is exactly why the pure-CSS route is needed). 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"> (a class, no inline px).
  • snapshot vs settled #main-content:
viewport snapshot live shift
desktop (1600) 55px / x=55 55px / x=55 0px
mobile (375) 55px / x=55 55px / x=55 0px

Refs: dspace-customers#725, #1321

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jun 25, 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: c65648a0-4170-4212-b695-557cfc6b2a7a

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.

@milanmajchrak milanmajchrak force-pushed the fix/admin-sidebar-ssr-padding branch from 5b91e72 to 75fca50 Compare June 25, 2026 14:07
…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 milanmajchrak force-pushed the fix/admin-sidebar-ssr-padding branch from 75fca50 to 13d73fa Compare June 25, 2026 14:35
@milanmajchrak

Copy link
Copy Markdown
Collaborator Author

Review summary

Reviewed for SSR correctness, DSpace regressions, and "does it actually solve the flicker / can it break anything". Verdict: correct, no layout regression, solves the reported symptom for the right reason.

Why the SSR snapshot matches the live app (the core guarantee):

  • --ds-admin-sidebar-fixed-element-width / --ds-admin-sidebar-total-width are global :root custom properties (src/styles/_custom_variables.scss), so var(...) in the component-scoped .outer-wrapper rule resolves identically whether the node is under <ds-app> or moved into the anti-flicker overlay snapshot. Moving a node doesn't change its _ngcontent attr or which :root it inherits from.
  • The scoped rule survives the overlay: BrowserModule.withServerTransition ships it as style[ng-transition], and the inline overlay script clones those into data-dspace-ssr-keep before Angular strips them.
  • The class is deterministic on the server: the ADMIN menu is always present in initialMenusState, store selects are synchronous, and slideSidebarOver$ has startWith(true)combineLatest emits unpinned synchronously (no transient ds-admin-sidebar-null in the serialized HTML). The browser rehydrates the same NgRx state (transferState), so it starts from the same class → 0px shift, not luck.
  • Non-admins → hiddenpadding-left: 0 on both sides (no new gutter).

Review feedback addressed in 13d73fa:

  • A plain transition: padding-left is a new potential flicker source (unlike the old animation it fires on the first hidden→unpinned change, and the overlay settle-detector watches DOM mutations, not style changes). It's now gated behind ds-admin-sidebar-animate, enabled only after the first browser paint — so the initial SSR→CSR resolution (behind the overlay) never animates, while genuine pin/unpin toggles still slide.
  • Removed the now-dead @slideSidebarPadding import/registration from the base and custom-theme root components.
  • Added an explicit .ds-admin-sidebar-hidden { padding-left: 0 } for self-documentation.

Verified (authenticated admin, real UA, throttled hard reload, local SSR build): SSR renders outer-wrapper ds-admin-sidebar-unpinned (a class, no inline px, no -animate); snapshot vs settled #main-content = 55/55 → shift 0px on desktop (1600) and mobile (375); live app gains -animate only after first paint.

Honest residuals (pre-existing, not introduced here, out of scope):

  • Pinned sidebar: the server always renders unpinned and pin state isn't persisted to a cookie, so a hard reload returns to collapsed (no jump) — but pin-across-reload simply isn't preserved.
  • Scrollbar gutter (~8–15px) when CSR adds a scrollbar the SSR snapshot lacked affects all users incl. anonymous; separate issue.
  • The "snapshot matches live" guarantee relies on universal.transferState: true (the default everywhere).

@milanmajchrak milanmajchrak merged commit adda0f2 into customer/vsb-tuo Jun 29, 2026
6 checks passed
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