Skip to content

fix: keep impersonation banner inside viewport layout#19

Merged
rubenhensen merged 1 commit into
mainfrom
fix/impersonation-banner-scroll
Apr 23, 2026
Merged

fix: keep impersonation banner inside viewport layout#19
rubenhensen merged 1 commit into
mainfrom
fix/impersonation-banner-scroll

Conversation

@dobby-coder

@dobby-coder dobby-coder Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Problem

When an admin is impersonating an organisation, the orange impersonation banner sits above the portal / admin shell. Both shells use min-height: 100vh on their flex container, so the document becomes banner + 100vh tall and a scrollbar appears on every page — even short ones.

src/routes/(portal)/+layout.svelte and src/routes/(admin)/+layout.svelte both had the same pattern:

<banner if impersonating />
<flex container with min-height: 100vh>

Fix

Wrap the banner and the flex container in a .layout-shell that owns the min-height: 100vh, and let the inner flex container take the remaining space with flex: 1; min-height: 0.

-{#if data.isImpersonating}
-  <div class=\"impersonation-bar\">...</div>
-{/if}
-<div class=\"portal\">...</div>
+<div class=\"layout-shell\">
+  {#if data.isImpersonating}<div class=\"impersonation-bar\">...</div>{/if}
+  <div class=\"portal\">...</div>
+</div>
-.portal { display: flex; min-height: 100vh; }
+.layout-shell { display: flex; flex-direction: column; min-height: 100vh; }
+.portal { display: flex; flex: 1; min-height: 0; }

Same structural change for the admin shell (.admin-layout).

The banner's colours, copy, and sticky-sidebar / sticky-header behaviour are all unchanged.

Why this works

  • When the banner is absent, .layout-shell stays at exactly 100vh — one flex child takes it all.
  • When the banner is present, .layout-shell still stays at 100vh and the shell splits that between banner-height and portal-height. Document height does not exceed the viewport, so no scrollbar appears on short pages.
  • When page content is genuinely tall, the shell grows past 100vh (that is min-height, not height) and normal scrolling works as before.

Verification

  • npx svelte-check — 0 errors, 0 warnings
  • vitest run — 26/26 unit tests pass
  • vite build — clean build
  • Could not test the portal page interactively: the portal routes redirect to login and there is no seeded DB in the Coder workspace. The fix is a standard flex-column wrapper pattern and the CSS is self-contained; happy to extend with a Playwright snapshot if preferred.

Reviewer quickstart

git fetch origin && git checkout fix/impersonation-banner-scroll
cp .env.example .env
npm ci && npm run dev

Then log in as an admin, impersonate an organisation, and confirm the portal pages no longer scroll when content fits within the viewport.

Closes #18

The portal and admin shells both rendered the banner as a sibling of
a layout container that had min-height: 100vh. With the banner
present, the document height became banner + 100vh, which forced a
body scrollbar even on otherwise short pages.

Wrap the banner and the flex layout in a .layout-shell that owns the
min-height: 100vh, and let the layout container flex-fill the
remaining space. No change to the banner's visual appearance or to
the sidebar / sticky header behaviour.

Closes #18
@dobby-coder dobby-coder Bot marked this pull request as ready for review April 23, 2026 10:11
@rubenhensen rubenhensen merged commit 0a94f99 into main Apr 23, 2026
7 checks passed
@rubenhensen rubenhensen deleted the fix/impersonation-banner-scroll branch April 23, 2026 11:25
@dobby-coder

dobby-coder Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

Dobby sees the approval on PR #19! Dobby is most grateful and will check the PR state and merge readiness right away! 🧦

@dobby-coder

dobby-coder Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

Checked in on PR #19 after the approval: all CI checks passed (Svelte Check, Tests, Builds, Migration Safety) and the PR has already been merged to main. No further action needed — thanks for the review, @rubenhensen! 🧦

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.

Impersonation banner causes page to scroll

1 participant