Skip to content

MENDELU/Fix header/nav widget shift on SSR->CSR hydration - #1377

Open
MatusBeke wants to merge 1 commit into
customer/mendelufrom
mendelu/fe-fix-home-flicker-shift
Open

MENDELU/Fix header/nav widget shift on SSR->CSR hydration#1377
MatusBeke wants to merge 1 commit into
customer/mendelufrom
mendelu/fe-fix-home-flicker-shift

Conversation

@MatusBeke

Copy link
Copy Markdown
Collaborator

Problem description

Home page (and other SSR-rendered pages) visibly flicker/shift on a hard reload. Reported as: page flickers, and content now also visibly shifts.

Root cause

Confirmed locally (Docker: DSpace 9.1 backend + production SSR build) with Playwright + a layout-shift PerformanceObserver, and cross-checked against a live measurement on the shared MENDELU dev instance:

  • ds-auth-nav-menu (the "Admin Access" login button) and #desktop-navbar (<ds-navbar>, the top-level menu) are both recreated client-side by ThemedComponent on hydration — SSR content is discarded and the component re-instantiates from scratch (see src/app/shared/theme-support/themed.component.ts). Each briefly renders narrower/empty before settling back to its SSR-resolved width:
    • ds-auth-nav-menu's showAuth$ defaults to of(false) until ngOnInit re-subscribes to the router-store selector (src/app/shared/auth-nav-menu/auth-nav-menu.component.ts), so the button goes 0 → ~126px.
    • #desktop-navbar's menu items go through the same recreation and settle from ~339px → ~366px.
  • #header-right is a flex-nowrap row pinned via justify-content:end inside a flex-grow #header-left/#header-right container, so either widget's width swing visibly shifts the search box next to it — measured as a Cumulative Layout Shift of ~0.21–0.36 on the affected loads.
  • The anti-flicker overlay (src/index.html + AppComponent.removeSsrOverlayWhenContentVisible) usually masks this, but it's a timing race, not a guarantee — this is why it's visible in production sometimes but not every load.
  • This showAuth$ = of(false) pattern is shared/common code (confirmed identical on customer/TUL and customer/vsb-tuo), so it's a latent bug everywhere — it's just that mendelu's header layout (this specific flex arrangement, absolutely-positioned search input) is what makes it visible, unlike the other customer themes.

Fix

CSS-only, scoped to mendelu's own theme (src/themes/custom/app/header/header.component.scss): reserve stable min-width for both widgets so the row's footprint never changes regardless of hydration timing —

  • ds-auth-nav-menu: ~150px (covers "Admin Access" / "Admin přístup", the widest login-button label)
  • #desktop-navbar: ~380px (covers mendelu's current 2-item top-level menu, ~366px measured, with headroom)

Both are documented in the SCSS as content-driven, non-upstream constants that should be remeasured if the menu changes.

Verification

Local Docker stack (DSpace 9.1 backend, production SSR build, seeded test data) — 16 total hard-reload samples across two independent measurement passes:

  • Before the fix: #header-right appeared as a layout-shift source in the majority of runs, with widths jumping up to ~126px (auth widget) and positions shifting up to ~26.5px (pushed by the navbar).
  • After the fix: #header-right did not appear as a layout-shift source in any of the 16 samples.

Known residual, out of scope for this PR: a separate, smaller layout shift (~24px vertical reflow of the homepage welcome paragraph) was observed in local testing. It is structurally unrelated to the ThemedComponent/hydration mechanism this PR fixes, was not part of the original reproduction on the live dev instance, and may be specific to the locally-seeded test content rather than production. Not fixed here; flagging for a possible follow-up.

Manual Testing

  • Anonymous /home hard reload, repeated — header/search-box no longer visibly shifts
  • Verified via automated CLS measurement (Playwright), not just visual inspection

Sync verification

No i18n changes in this PR.

ds-auth-nav-menu and #desktop-navbar (<ds-navbar>) are both recreated
client-side by ThemedComponent on a hard reload (SSR content discarded,
see themed.component.ts), so they briefly render narrower/empty before
settling to their final width. #header-right is a flex-nowrap row
pinned via justify-content:end, so that swing on either widget visibly
shifts the search box next to it. The anti-flicker overlay usually
masks this, but it is a timing race, not a guarantee.

Reserve stable width for both widgets so the row's footprint never
changes regardless of timing:
- ds-auth-nav-menu: min-width covering the widest login-button label
  (en "Admin Access" / cs "Admin přístup", ~127px)
- #desktop-navbar: min-width covering mendelu's current top-level menu
  (~366px measured)

Verified locally: header-right no longer appears as a layout-shift
source across repeated hard-reload samples (was ~26-126px before).
@coderabbitai

coderabbitai Bot commented Jul 15, 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: 62b34eba-93c9-4700-bbee-a955c3201a30

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.

@MatusBeke
MatusBeke requested a review from milanmajchrak July 16, 2026 09:15
MatusBeke added a commit that referenced this pull request Jul 20, 2026
Fixes dataquest-dev/dspace-customers#823 (bitstream portion). Ports
the real vanilla DSpace feature that already exists on this fork's
DSpace 9.x lines (e.g. customer/mendelu): a unified
AccessStatusBadgeComponent that renders an access-status badge for
either an Item or a Bitstream, instead of a separate embargo-only
component (an earlier draft of this PR introduced a standalone
EmbargoBadgeComponent copied from a since-abandoned upstream design
(PR dataquest-dev/dspace-angular#3882 / commit 5f83139); that
component has no live upstream reference on 9.3/main/mendelu and has
been removed in favor of this unified approach).

Change set:
- Bitstream model gains an `accessStatus` HAL link (mirrors the
  existing thumbnail/format link shape).
- AccessStatusObject (both the canonical copy and a pre-existing
  duplicate under object-list/) gains `embargoDate`.
- AccessStatusBadgeComponent's `object` input widens from `Item` to
  `Item | Bitstream`; it now resolves the accessStatus link lazily via
  LinkService.resolveLink(..., followLink('accessStatus', { isOptional:
  true })) instead of eagerly reading `_links.accessStatus.href`
  directly. isOptional: true is required (unlike upstream, which never
  needs it because its backend always has the link already) because
  this backend doesn't expose the bitstream-level link yet - without
  it, resolveLink() throws for every bitstream instead of failing
  closed.
- file-download-link.component.html renders
  <ds-themed-access-status-badge [object]="bitstream"> next to each
  file, gated by the existing item.bitstream.showAccessStatuses flag.
- That flag now defaults to false (matching upstream/mendelu) instead
  of true - flip it once the backend companion PR (DSpace#1377) is
  deployed, not before.
- i18n copy aligned with vanilla/mendelu: "Embargo until {{ date }}"
  (en) / "Embargo do {{ date }}" (cs).
- Removed the unused findBitstreamAccessStatusFor() and the deleted
  EmbargoBadgeComponent/ThemedEmbargoBadgeComponent + their shared
  .module.ts registration.

The accessibility fix (aria-label/role/sr-only on the lock icon) that
was previously bundled into this branch has been split out to its own
PR (#1390) for independent review - this branch was rebuilt from
scratch off customer/zcu-pub to drop those commits and keep this PR
scoped to the embargo-badge feature only.

Test evidence:
  npx ng lint --quiet                       -> All files pass linting.
  npx ng build --configuration production   -> build succeeded, no budget warnings
  npx ng test (access-status-badge + file-download-link + thumbnail specs)
                                             -> 39/39 SUCCESS, including a
     regression test that constructs a real Bitstream lacking
     _links.accessStatus and asserts ngOnInit doesn't throw and the
     file list renders without a badge.
  Manual: local Docker stack running the CURRENT (pre-#1377) zcu-pub
     backend image, showAccessStatuses temporarily forced true - an
     embargoed item's file list rendered normally, zero console
     errors, and no accessStatus network request was even attempted
     (LinkService detects the missing _links key up front).
milanmajchrak pushed a commit that referenced this pull request Jul 21, 2026
Fixes dataquest-dev/dspace-customers#823 (bitstream portion). Ports
the real vanilla DSpace feature that already exists on this fork's
DSpace 9.x lines (e.g. customer/mendelu): a unified
AccessStatusBadgeComponent that renders an access-status badge for
either an Item or a Bitstream, instead of a separate embargo-only
component (an earlier draft of this PR introduced a standalone
EmbargoBadgeComponent copied from a since-abandoned upstream design
(PR dataquest-dev/dspace-angular#3882 / commit 5f83139); that
component has no live upstream reference on 9.3/main/mendelu and has
been removed in favor of this unified approach).

Change set:
- Bitstream model gains an `accessStatus` HAL link (mirrors the
  existing thumbnail/format link shape).
- AccessStatusObject (both the canonical copy and a pre-existing
  duplicate under object-list/) gains `embargoDate`.
- AccessStatusBadgeComponent's `object` input widens from `Item` to
  `Item | Bitstream`; it now resolves the accessStatus link lazily via
  LinkService.resolveLink(..., followLink('accessStatus', { isOptional:
  true })) instead of eagerly reading `_links.accessStatus.href`
  directly. isOptional: true is required (unlike upstream, which never
  needs it because its backend always has the link already) because
  this backend doesn't expose the bitstream-level link yet - without
  it, resolveLink() throws for every bitstream instead of failing
  closed.
- file-download-link.component.html renders
  <ds-themed-access-status-badge [object]="bitstream"> next to each
  file, gated by the existing item.bitstream.showAccessStatuses flag.
- That flag now defaults to false (matching upstream/mendelu) instead
  of true - flip it once the backend companion PR (DSpace#1377) is
  deployed, not before.
- i18n copy aligned with vanilla/mendelu: "Embargo until {{ date }}"
  (en) / "Embargo do {{ date }}" (cs).
- Removed the unused findBitstreamAccessStatusFor() and the deleted
  EmbargoBadgeComponent/ThemedEmbargoBadgeComponent + their shared
  .module.ts registration.

The accessibility fix (aria-label/role/sr-only on the lock icon) that
was previously bundled into this branch has been split out to its own
PR (#1390) for independent review - this branch was rebuilt from
scratch off customer/zcu-pub to drop those commits and keep this PR
scoped to the embargo-badge feature only.

Test evidence:
  npx ng lint --quiet                       -> All files pass linting.
  npx ng build --configuration production   -> build succeeded, no budget warnings
  npx ng test (access-status-badge + file-download-link + thumbnail specs)
                                             -> 39/39 SUCCESS, including a
     regression test that constructs a real Bitstream lacking
     _links.accessStatus and asserts ngOnInit doesn't throw and the
     file list renders without a badge.
  Manual: local Docker stack running the CURRENT (pre-#1377) zcu-pub
     backend image, showAccessStatuses temporarily forced true - an
     embargoed item's file list rendered normally, zero console
     errors, and no accessStatus network request was even attempted
     (LinkService detects the missing _links key up front).
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