MENDELU/Fix header/nav widget shift on SSR->CSR hydration - #1377
Open
MatusBeke wants to merge 1 commit into
Open
MENDELU/Fix header/nav widget shift on SSR->CSR hydration#1377MatusBeke wants to merge 1 commit into
MatusBeke wants to merge 1 commit into
Conversation
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).
|
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 |
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).
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
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-shiftPerformanceObserver, 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 byThemedComponenton hydration — SSR content is discarded and the component re-instantiates from scratch (seesrc/app/shared/theme-support/themed.component.ts). Each briefly renders narrower/empty before settling back to its SSR-resolved width:ds-auth-nav-menu'sshowAuth$defaults toof(false)untilngOnInitre-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-rightis aflex-nowraprow pinned viajustify-content:endinside aflex-grow#header-left/#header-rightcontainer, 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.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.showAuth$ = of(false)pattern is shared/common code (confirmed identical oncustomer/TULandcustomer/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 stablemin-widthfor 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:
#header-rightappeared 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).#header-rightdid 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
/homehard reload, repeated — header/search-box no longer visibly shiftsSync verification
No i18n changes in this PR.