Pin the sticky header and size panes from its measured height - #1916
Open
atabisz wants to merge 1 commit into
Open
Pin the sticky header and size panes from its measured height#1916atabisz wants to merge 1 commit into
atabisz wants to merge 1 commit into
Conversation
`AppHeader` is `sticky top-0`, but `globals.css` set `overflow-x: hidden` on `body`. Any non-`visible` overflow makes `body` a scroll container, and `position: sticky` resolves against its nearest scrolling ancestor — so the header pinned itself to a box that never scrolls (the viewport scrolls, `body` does not) and travelled off-screen with the content. Scrolled 2136px down, its viewport-relative `top` reads -2136; with `overflow-x: clip` on `body`, which clips identically without creating a scroll container, it reads 0. `overflow-x: hidden` stays on `html`, where it does stop the mobile horizontal scroll it was added for. Five panes sized themselves with `h-[calc(100vh-3.5rem)]`, but the header's height is not a constant: tier 1 is `min-h-14` around a `flex-wrap` nav that grows a row when items wrap, tier 2 renders only inside /system, and there is a mobile menu. Measured in a browser at a root font-size of 15px, where `3.5rem` is 52.5px, the header draws 53.5px at 676px wide, 238px at 836px, 167.5px at 1076px and 91px at 1713px — 1.02x to 4.53x the assumed height. The nav is already on two rows on a wide desktop, so those panes run 38.5px too tall there and up to 185.5px too tall at the worst width. `AppHeader` now measures itself with a `ResizeObserver` and publishes `--app-header-h`; the panes consume it and repeat the old constant as a `var()` fallback, because an unresolved custom property is invalid at computed-value time and would drop the whole declaration rather than fall back to anything. The open mobile menu is subtracted back out of the published height. It measures 1297px against an 824px viewport, so including it would publish 1351.5px and make `calc(100vh - 1351.5px)` resolve to -527.5px, which floors at 0 — the pane collapses and loses its scroll offset, so closing the menu returns you to the top of the document. The menu is `md:hidden`, so the subtraction is a no-op at md and above. `main` used `min-h-screen`. The header is sticky rather than overlaid, so that made every document a full header taller than the container it sits in, and put a scrollbar on pages that fit.
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.
Three layout defects in the Observability app that compound with each other. All measured in a
browser on a clean checkout, at a root font-size of 15px — worth stating, because that makes the
assumed
3.5rem52.5px rather than 56px, and every number below is against that.1. The sticky header is not pinned
AppHeaderissticky top-0, butglobals.csssetsoverflow-x: hiddenonbody. Anynon-
visibleoverflow makesbodya scroll container, andposition: stickyresolves againstits nearest scrolling ancestor — so the header pins itself to a box that never scrolls (the
viewport scrolls,
bodydoes not) and travels off-screen with the content.Scrolled 2136px down, the header's viewport-relative
topreads-2136. Switchingbodytooverflow-x: clipreturns it to0, and switching back reproduces it.clipclips identicallybut does not create a scroll container, which is the whole difference here.
overflow-x: hiddenstays on
html, where it does stop the mobile horizontal scroll it was added for.2. Five panes assume the header is
3.5remtalldocs,system,memory/knowledge,WikiSidebarandWikiMetaall size themselves withh-[calc(100vh-3.5rem)]. But the header's height is not a constant: tier 1 ismin-h-14arounda
flex-wrapnav that grows a row when its items wrap, tier 2 renders only inside/system, andthere is a mobile menu.
3.5rem(52.5px)The nav is already on two rows at 1713px, so the assumption is off by 38.5px on a wide desktop and
by 185.5px at the worst width — each pane is that much too tall, so its bottom edge and the end of
its scroll track fall below the fold.
AppHeadernow measures itself with aResizeObserverand publishes the height as--app-header-h; the five panes consume it.globals.cssdefines a first-paint fallback ofcalc(3.5rem + 1px)— tier 1 at its minimum plus its bottom border, so a pane is briefly slightlytoo tall rather than clipped before hydration — and every consumer repeats that same value as a
var()fallback. That repetition is not redundancy: an unresolved custom property is invalid atcomputed-value time, so the whole declaration is dropped and the pane silently reverts to
autoheight with nothing logged anywhere.
The open mobile menu is subtracted back out of the published height. It is a long list: measured at
1297px against an 824px viewport, so including it would publish 1351.5px and make
calc(100vh - 1351.5px)resolve to -527.5px, which CSS floors at 0. The pane collapses, andcollapsing it resets its scroll offset — so closing the menu would return you to the top of the
document. The menu is
md:hidden, and at md and above it is absent from the DOM entirely, so thesubtraction is a no-op there.
3.
mainusedmin-h-screenThe header is sticky rather than overlaid, so the space left for content is the viewport minus the
header.
min-h-screenmade every page a full viewport tall inside a container that starts belowthe header, pushing a scrollbar onto pages that fit.
Verification
8 files, +66/-7, additive on every file. In a browser, across the four widths above: the published
value equals the header's rendered height at each one, and
main's computedmin-heightequalsthe viewport minus that height at each one. With the mobile menu open on
/docsthe publishedvalue stays at the collapsed header's height and the pane holds 769.5px of an 824px viewport. The
measured heights span 4.4x across those widths, so no fixed value can satisfy both the 676px and
the 836px case — which is what the current constant is trying to do.
One thing I deliberately left out: wrapping the five panes in
max(0px, ...)would make thecollapse in section 2 unreachable regardless of what is published. It is a real hardening, but it
changes all five expressions to defend against a case this patch already prevents, so it seemed
better as a separate change than mixed into this one.