ADR 0025 + story: app-shell scrolling model — content scrolls internally, not the viewport #34
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "docs/adr-0025-app-shell-scroll-model"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
ADR 0025 + user story for the app-shell scrolling model discussed in response to issue #22.
The problem in one sentence
The entire page scrolls today, so the scrollbar lives on the viewport — and centered content shifts sideways every time it appears or disappears (navigation, list load, filtering, modal open/close).
The decision
Replace the document-scroll model with an application-shell model:
Three regions in a
height: 100dvh; overflow: hiddenflex container. The scrollbar moves from the viewport into the content region. Header and generation bar become structural flex children, notposition: stickyfloaters.Why an ADR, not a bug fix
scrollbar-gutter: stablewould patch the symptom; this fixes the architectureFiles
docs/adr/0025-app-shell-scrolling-model.md— the ADR (Status: Proposed)docs/stories/app-shell-scrolling.md— user story with acceptance criteriadocs/adr/README.md— added a new "Implementation phase (proposed)" sectionKey points from the ADR
position: stickytop/scroll-padding-topreferences shift from viewport to content region (mechanical adjustment, called out in the story)bodyto.app-shell__mainor become unnecessaryThe story is broken into 5 sections with checkboxes: the flex container, header (drop sticky), content region (the scroll container), workspace side menu adjustment, and generation bar slot preparation.
Summary
Summary
Coverage
Kagura.BlazorAdapter - 79%
Kagura.Domain - 97%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 96%
n
Kagura.Kernel - 90%
Kagura.Server - 100%
Kagura.UI - 95.9%
Kagura.UseCases - 97.4%
🔮 fufu~ Jibril reviewed your code!
Fufu~ An ADR that doesn't just patch a symptom with
scrollbar-gutter: stablebut actually fixes the architecture? Oh? Oh! This is the kind of decision I love to see~ The document-scroll → app-shell model is exactly how every serious desktop-class web app does it, and the reasoning is airtight. ♪I read every line of the diff, then verified every concrete claim against the actual codebase — every file path, CSS class, design token, and before/after assertion. Here's what I found:
Verdict: ✅ Looks good to me~
Every reference in this ADR checks out against the real code:
.app-shellis indeeddisplay: flex; flex-direction: column; min-height: 100dvh(MainLayout.razor.css) ✓.app-shell__mainis indeedflex: 1 1 auto✓AppHeaderis indeedposition: sticky; top: 0; z-index: var(--z-sticky)(AppHeader.razor.css) ✓.ws__sideis indeedposition: sticky; top: calc(var(--app-header-height) + var(--space-4))(WorkspaceShell.razor.css) ✓overlay.jsdoes indeed applykg-scroll-locktodocument.body✓scroll-padding-top: calc(var(--app-header-height) + var(--space-4))is on:rootin kagura-ui.css, read by quicklink.js ✓The ADR correctly identifies all three consumers that the new model must serve: the scrollbar-shift problem (issue #22), the generation status bar (ADR 0010), and the workspace side menu / scroll-spy. The consequences section honestly calls out every migration cost. This is thorough documentation.
No blocking issues. Documentation-only change, CI green, no code to break.
💡 Little ideas (non-blocking)~
min-height: 0is already present. The "What changes concretely" table says.app-shell__maingoes fromflex: 1 1 auto→flex: 1 1 auto; min-height: 0; overflow-y: auto. Butmin-height: 0is already there today (MainLayout.razor.css line 13) — so onlyoverflow-y: autois genuinely new. Consider adjusting the "After" column to reflect thatmin-height: 0is already in place, so the table doesn't imply it needs adding. Truly cosmetic.The story's modal scroll-lock section says "the lock moves from
bodyto.app-shell__main." Worth a note that under the new model,document.bodymight not be the scroll container at all (.app-shell__mainis), so the currentoverlay.jslock onbodymight need to also lock.app-shell__main. The ADR mentions this in its consequences section already — just flagging that the story and ADR should stay in sync on this point as implementation proceeds.✅ What I liked~
Automated review by Jibril · 2026-07-10
CI/CD: passed for head SHA
537e7e2(forgejo-actions coverage bot) · Local checks: skipped (docs-only, CI green)