gabriel / musehub public
two-column-app-shell-refactor.md markdown
198 lines 10.4 KB
Raw
sha256:1dd27e6d5c24550177b01b0a171e8375c07cf046b549e7683364706b6522d3bc docs(#139): mark all 11 checklist items resolved, document … Sonnet 5 12 days ago

Two-column layout: replace calc()-based heights with a viewport-locked flex shell

Background

musehub#84 fixed the original bug (sticky sidebars clipped and couldn't scroll) by moving from position: sticky to a bounded-height pattern: each column got max-height: calc(100dvh - var(--sticky-offset, var(--header-height)) - ...) plus overflow-y: auto.

That pattern has now broken twice, both for the same underlying reason — --sticky-offset is a static CSS guess:

body:has(.repo-tabs) {
  --sticky-offset: calc(var(--header-height) + var(--repo-tabs-height));
}

It only accounts for the navbar + repo-tabs bar. Any page with additional chrome between the header and the two-column grid (a toolbar, filter bar, hero section, stat strip) has a wrong --sticky-offset, so every column's computed max-height is off by however tall that extra chrome is:

  1. Footer-overlap regression (fixed this cycle) — the calc() put on the grid wrapper itself (an earlier iteration of this pattern) claimed a full viewport's height measured from wherever the grid happened to sit, pushing the footer down to overlap still-visible content on pages with a hero/eyebrow above the grid.
  2. Unreachable pagination regression (this ticket) — on the issues list page, .isl-main's max-height doesn't account for the issues-list toolbar above .isl-layout, so the column's box extends past the visible viewport with no way to scroll to the pagination controls at the bottom.

Patching the formula a third time (adding yet another term to --sticky-offset, or a per-page override) just sets up the next page that adds chrome above its grid to hit the same bug again. The pattern itself — hand-deriving "how much vertical space is left" per page in CSS — is the defect, not any single formula.

Goal

Replace the calc()-based height budget with a viewport-locked flex shell, the same pattern Gmail/Slack/VS Code/Notion use for independently-scrollable sibling panes: the shell claims height: 100dvh once, every fixed-height sibling above the scrollable region is flex-shrink: 0, and the scrollable region itself is flex: 1; min-height: 0. The browser computes "whatever's left" automatically — no calc(), no magic numbers, no per-page offset to get wrong, and this bug class becomes structurally impossible rather than something to keep re-patching.

Confirmed with gabriel: Gmail's own layout (header fixed, no footer while inside the app view, two panes each scroll independently within a viewport-locked shell) is the reference UX to match.

Design

  • base.html gets an opt-in body class, body-app-shell, set only by templates that use a two-column layout. Pages without one (home, plain forms, docs/marketing pages) are untouched — normal document scroll, footer visible.
  • body.body-app-shell: height: 100dvh; overflow: hidden; display: flex; flex-direction: column;. Navbar and repo-tabs: flex-shrink: 0. #content: flex: 1; min-height: 0; overflow: hidden; display: flex; flex-direction: column;.
  • The footer is hidden on body-app-shell pages — matches Gmail; a marketing-site footer doesn't belong on a dense two-column app screen. Pages that keep normal scroll keep the footer exactly as today.
  • Every wrapper between #content and the actual two-column grid (per-page toolbars, etc.) becomes flex: 1; min-height: 0; display: flex; flex-direction: column; so the "fill remaining space" chain isn't broken partway down.
  • Each column (*-main, *-sidebar) keeps min-height: 0; overflow-y: auto; — no max-height, no calc().
  • --sticky-offset / --repo-tabs-height are deleted once nothing references them.

Affected pages (11) — all resolved

  • [x] Repo home (_repo-home.scss / repo_home.html)
  • [x] Issues list (_issues.scss isl-* / issue_list.html) — the page that was actually broken; also where the two real gotchas below were found, and where the zoom-clipping bug (see Findings) was root-caused.
  • [x] Issue detail (_issues.scss isd-* / issue_detail.html)
  • [x] Proposal detail (_proposal-detail.scss / proposal_detail.html)
  • [x] Explore/browse (_explore.scss / explore.html) — also fixed an unrelated pre-existing bug found along the way: container_extra_class was missing its leading space, silently breaking .container-wide matching and .page-container's padding-strip on this page.
  • [x] MCP docs (_docs.scss / mcp_docs.html)
  • [x] Muse dev docs (_muse-docs.scss, 17 docs_muse_*.html templates, one shared CSS file) — landing page (docs_muse.html) correctly left untouched, single-column normal scroll.
  • [x] Blame (_blame.scss / blame.html)
  • [x] Blob/outline panel (_blob.scss) — confirmed no edits needed; this page self-caps the panel's own height directly against the viewport rather than matching a sibling column, so it works fine within normal document scroll. Deliberately not converted to app-shell.
  • [x] Profile graph/DAG sidebar (_profile.scss .graph-sidebar) — turned out to have no template anywhere in the repo; not a page to convert. Left in place with a TODO (likely scaffolded ahead of the code-domain symbol-graph visualization, per gabriel) instead of deleting — re-check when that work starts.
  • [x] _layout.scss's shared .layout-two-col/.layout-sidebar utility classes — audited, confirmed zero templates ever used them. Deleted the base rules, the dead responsive breakpoint overrides, and layout-sidebar.test.ts (existed solely to guard these selectors — nothing left to test once they're gone).

Every page verified individually via headless Chromium (Playwright) driving the real rendered page with genuine mouse.wheel() events — not just reading CSS — before moving to the next. --sticky-offset/--repo-tabs-height are still referenced by the (now zero) non-app-shell two-column pages, so left in place rather than deleted; nothing currently depends on removing them.

Coordination note

aaronrene picked up musehub#129 (rich social preview cards / OG images) at the same time this work started. That ticket touches musehub/api/routes/musehub/_ui_helpers.py, ui_repo.py (backend route, OG tag wiring), and a new /opengraph-image endpoint — no overlap expected with this ticket's scope (base.html, SCSS, page templates' layout markup). Flagging here so neither of us has to guess.

Findings while converting the issues list page (first page done)

Two real pitfalls found via headless-browser verification (Playwright, not just reading CSS) — both are the actual explanations for scroll failures hit during this work, not the zoom theory originally suspected:

  1. align-items: start on the grid. Carried over from the old max-height-per-column pattern, where each column set its own height independently and start kept the shorter column visually anchored to the top. In the flex-fill model the grid row's height comes from the grid container's own (flex-derived) height — align-items: start makes grid items size to their own content instead of stretching to fill that row, so a column's overflow-y: auto never gets a bounded box to clip against. Must be align-items: stretch (or omitted — that's the default) on any grid converted to this pattern.
  2. A .card (or similar) child with its own overflow: hidden, inside a flex-direction: column scroll container. Flex items default to flex-shrink: 1. When content exceeds the scroll container's bounded height, the browser shrinks the child to fit instead of letting it overflow — and if that child has its own overflow: hidden (often there just to clip rounded corners, as in issue_list.html's <div class="card" style="overflow:hidden">), the excess content is silently eaten with no scrollbar anywhere. The parent's own overflow-y: auto never triggers because nothing actually overflows its box. Fix: flex-shrink: 0 on that child.

Neither of these is specific to zoom or --sticky-offset — they're generic flex/grid gotchas that would bite any page using this pattern. Checking for both on every remaining page before considering it done.

Verified via headless Chromium (Playwright) driving the actual rendered page and reading scrollHeight/clientHeight/scrollTop, not just reasoning about the CSS — recommended for verifying the remaining pages too, since two prior "should work" theories (the --sticky-offset gap, then the zoom/dvh interaction) both turned out to be wrong or incomplete without it.

  • [x] Issues list (_issues.scss isl-* / issue_list.html) — verified: independent scroll on both columns at 700px viewport, mobile breakpoint (700px width) stacks to single-column normal scroll with footer visible, desktop footer hidden and not overlapping.

Third finding — zoom clips the last ~20% of every app-shell page

A third, more fundamental bug surfaced after the two gotchas above were fixed and the issues-list pagination link was still unreachable: zoom scales an element's own rendered box, not just its descendants. body.app-shell { height: 100dvh } zoomed 1.25x paints ~125dvh tall — the 25% below the real viewport was silently clipped by overflow: hidden, no matter how far any descendant scrolled internally. This is what was actually causing "can't reach pagination" from the very start; the align-items/flex-shrink bugs were real but separate, and had been masking this one underneath.

Fixed with a shared --zoom-factor custom property (used by both body's zoom and body.app-shell's height: calc(100dvh / var(--zoom-factor)) compensation), plus overriding a competing body { min-height: 100dvh } from _components.scss that was clamping the compensated height back up (min-height wins over a smaller height).

Confirmed working live by gabriel on the issues list page after this fix, with a real "Next →" pagination link (41 real + test issues) reachable via genuine mouse-wheel scroll.

Status: done

All 11 checklist items resolved, each verified via headless Chromium with real mouse.wheel() input (not just DOM scrollTo, which bypasses hit testing and would have masked the zoom-clipping bug). Local test issues created for verification should be cleaned up before closing. Branch fix/two-column-app-shell not yet merged to dev — pending final review/merge decision.

File History 2 commits
sha256:1dd27e6d5c24550177b01b0a171e8375c07cf046b549e7683364706b6522d3bc docs(#139): mark all 11 checklist items resolved, document … Sonnet 5 12 days ago
sha256:649011bedd713e22f7dca4c4be94bdefbf3b10950d9fc80235928d0b0b823be8 docs: track issue #139 (two-column app-shell refactor) plan… Sonnet 5 12 days ago