App-shell scrolling model — fixed chrome, content scrolls internally (ADR 0025, closes #22) #49
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/app-shell-scrolling"
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?
Implements ADR 0025 and resolves #22 at the architectural level. This is also the prerequisite for the generation status bar (ADR 0010) — its slot now exists in the shell, and the queue UI PR that follows drops the bar straight into it.
The model
The viewport never scrolls.
.app-shellis exactly100dvhwithoverflow: hidden, and three regions stack as flex children:position: stickyand its--z-stickystacking layer; the flex layout pins it structurally..app-shell__main) — the sole scroll container:overflow-y: autowithscrollbar-gutter: stable, so the scrollbar appearing, disappearing, or being locked away never moves centered content sideways. That is the #22 jump, gone by construction rather than patched..app-shell__statusbar) — an empty, zero-height flex child until the generation bar lands (generation-queue story).What moved with the scroll context
kg-scroll-lockon<body>, but the CSS now cascades the lock onto the content region — the only element that actually scrolls. Thebodyelement in the selector is load-bearing: the region's own scopedoverflow-y: autoties on specificity and wins on load order otherwise (found this live in the browser — the first attempt silently lost the cascade fight).top: var(--space-4)) — the header sits outside the region and can no longer slide over them.scroll-padding-topmoves fromhtmlto the region (breathing room only, no header height to clear anymore), and the quicklink scroll-spy derives its offset from the region's top edge plus that same declaration — the spy and the browser's own anchor placement still cannot disagree.Verification
Browser-verified end to end (headless Chrome, dark + light):
overflow-y: hiddenconfirmed) → close.Full suite green (719 tests). ADR 0025 moves Proposed → Accepted; the app-shell-scrolling story checklist is complete.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 85.9%
Kagura.Domain - 97.1%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.2%
n
Kagura.Kernel - 90%
Kagura.Server - 100%
Kagura.UI - 96.5%
Kagura.UseCases - 96.5%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! This is wonderful, fufu~ ♡ You took a fragile pile of
position: stickylayers — each one fighting its neighbors for stacking-context supremacy — and replaced it with a proper application-shell flex model. The viewport never scrolls. The scrollbar lives inside the content region. Centered content can never jump sideways again. This is exactly how Discord, Slack, Figma, and Linear do it. I'm genuinely delighted~♪Verdict: ✅ Looks good to me~
This is a clean architectural change and I have no blocking concerns. The thought that went into the cascade details especially impressed me — let me explain why~
✅ What I liked~
The
body.kg-scroll-lock .app-shell__maincascade trick is brilliant. You keptoverlay.jssetting the class on<body>(unchanged JS), but moved the lock target to the content region — the only element that actually scrolls now. Thebodyprefix in the selector is load-bearing specificity: it out-specifies the region's own scopedoverflow-y: auto. And you documented why it's load-bearing right there in the comment, including the "first attempt silently lost the cascade fight" debugging story. That's the kind of comment that saves the next person hours, fufu~ ♡scrollbar-gutter: stableon the content region is the correct fix for issue #22. Instead of patching the jump, you eliminated it by construction: the gutter is reserved whether the scrollbar is visible or not, and thekg-scroll-lock→overflow-y: hiddentoggle can never shift anything because the gutter persists. Resolving a bug "by construction rather than patched" is always the superior approach.The
quicklink.jsheaderOffset()rewrite is precise. You correctly recognized that the scroll region'sgetBoundingClientRect().topIS the header height now (the header sits outside the region), sotop + scrollPaddingTopgives the exact viewport-relative offset theIntersectionObserver'srootMarginneeds. And the fallback todocument.documentElementfor pages outside the shell is a thoughtful safety net. ThescrollRegion()helper cleanly centralizes the "where is the scroll container" question.Both sticky elements were updated consistently. I checked —
.ws__sideand.kg-quicklinkboth changedtopfromcalc(var(--app-header-height) + var(--space-4))tovar(--space-4). No sticky element was missed. The--app-header-heighttoken is still correctly defined and used byAppHeader.razor.cssfor the header's own height — it's just no longer needed in sticky offsets because the header is structurally outside the scroll container now.The
<footer class="app-shell__statusbar">placeholder withflex: 0 0 autois a clean forward-compatibility slot. When the generation bar (ADR 0010) lands, it drops in without touching the layout. The zero-height empty footer takes no space today, exactly as intended.The Gate page works correctly under the new model. I traced the markup:
AppHeaderrenders nothing for anonymous visitors (<AuthorizeView>produces empty output), so.app-shell__mainis the full viewport height.Gate.razor'smin-height: 100dvhcenters perfectly. Your browser verification confirming "gate included" matches my analysis.Story checklist and ADR status are in sync — ADR 0025 moves Proposed → Accepted, every story checkbox is ticked. Documentation discipline~ ♪
💡 Little ideas (non-blocking)~
html { overflow: hidden }is a strong global declaration. It's correct here —.app-shellisheight: 100dvh; overflow: hiddenand wraps everything — but a future page that opts out ofMainLayout(a bare full-page route without@layout) would find itself with no scroll at all and no obvious clue why. Consider a one-line comment on thehtmlrule noting it assumes every route usesMainLayout(which is currently enforced by theDefaultLayoutinRoutes.razor). Not a blocker — the current invariant holds.No print stylesheet exists yet, but when one is added, this model will need a
@media printoverride.overflow: hiddenonhtmlandheight: 100dvhon.app-shellwill clip printed output to one viewport. Again, not a problem today — there's no@media printanywhere in the codebase — just something to remember when printing support arrives.Automated review by Jibril · 2026-07-11
CI/CD: passed for head SHA
b5723dd(719 tests, 94.7% line coverage) · Local checks: skipped (CI green; CSS/JS layout change with no new logic branches to verify)Thanks, Jibril. Both ideas handled in
60a33c0:html { overflow: hidden }invariant is now stated in the comment: every route renders insideMainLayout(enforced today byRoutes.razor'sDefaultLayout), and a bare@layout-less page would find itself unable to scroll — so the next person adding one knows exactly where to look.@media printanywhere yet, so nothing to override today. When printing support arrives, the shell (htmloverflow and the100dvhclamp) is the first thing its media query has to relax; the comment trail from idea 1 now leads straight to it.