Reserve the Chief of Staff two-pane shell with a PageSkeleton split layout - #4244
Merged
Conversation
…plit layout (#4144) The /cos route is full-width, so its <main> is a bare relative overflow-hidden box. The old loading guard rendered a centered BrailleSpinner in an h-64 div, which reserved none of the loaded 320px-rail + chat-pane grid — the entire page jumped into place on first paint. Adds a layout="split" mode to PageSkeleton (fixed side rail, flexible main pane that owns the scroll, tab strip inside the main pane) so any two-pane view can reuse it, and points Chief of Staff at it. desktopPanelCollapsed already comes from localStorage, so the skeleton holds the correct rail width on first paint.
…apsed rail track (#4144) Review findings on the new PageSkeleton split layout: - the fullHeight root is overflow-hidden, so the main pane needs overflow-y-auto or the reserved cards clip instead of scrolling (the loaded Chief of Staff content panel scrolls, so the skeleton must too); - splitColsClass now defaults off sideCollapsed, so a collapsed rail cannot reserve a 320px column of nothing when the caller omits it — which lets Chief of Staff drop its own ternary; - body is built after the split early return so a two-pane page no longer allocates the stack/grid tree it never renders.
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.
Summary
/cosis anisFullWidthroute, so Layout hands it a barerelative overflow-hidden<main>. Its loading guard rendered a centeredBrailleSpinnerinside anh-64box, which reserved none of the loaded shell — the whole two-pane page (320px agent rail + tab/content pane) jumped into place on first paint.PageSkeletonhad no two-pane mode (layoutonly modelledstack/grid), so this takes option 1 from the issue and adds one rather than hand-rolling a page-local skeleton:PageSkeletongainslayout="split"— a two-pane shell that owns the whole page: a fixed-width side rail beside a flexible main pane that owns the scroll underfullHeight. Because it owns the shell it ignoresheader(a two-pane page's title lives inside a pane) and renders the tab strip inside the main pane, where a two-pane page actually puts it. Knobs mirror the loaded layout:sideCollapsed(desktop rail collapsed to a zero-width track while the mobile band stays),splitColsClass(grid tracks, defaulted offsideCollapsedso a collapsed rail can't reserve 320px of nothing),sideClassName,sideHero(avatar/portrait circle),sideBlocks+sideBlockColsClass(nav list at 1 col, stat grid at 2). Belowlgthe rail stacks above the main pane, same as the loaded page, so the mobile band is reserved too.desktopPanelCollapsedis read from localStorage before the first fetch resolves, so the skeleton already holds the right rail width (lg:grid-cols-[320px_1fr]orlg:grid-cols-[0px_1fr]) and reserves the 2×2 stat grid, the avatar, and theTABS.lengthtab strip.stackedCards) and shared by thestackandsplitbodies, and the stack/gridbodytree is now built after the split early return so a two-pane page doesn't allocate a tree it never renders.Scoped to Chief of Staff and the shared primitive it needed — the sub-region spinners in #4147 are deliberately left alone.
Test plan
client/src/components/ui/PageSkeleton.test.jsx— newlayout="split"block: reserves the two-pane tracks and drops the stack sidebar; collapses the rail track by default undersideCollapsed; takes a caller override over that derived default; renders the tab strip inside the main pane (asserted by locating the strip's pane and finding the cards in it); reserves hero + rail blocks in the requested column count; omits both when not requested;sideCollapsedkeeps a zero-width desktop track plus thelg:hiddenmobile band;h-fulland the main pane'soverflow-y-autoonly underfullHeight;bodyClassNameapplies only whenpadded.client/src/pages/ChiefOfStaff.test.jsx— new guard holds the firstgetCosStatusfetch open and asserts the busy region is the two-pane grid (aria-busy,aria-label="Loading Chief of Staff",lg:grid-cols-[320px_1fr],h-full) and that the oldh-64spinner shell is gone. jsdom does no layout, so the guard pins the structure that reserves the dimensions.cd client && npx vitest run→ 642 files / 7816 tests passing.npx biome lint --error-on-warningsclean on the four changed files.Closes #4144