Reserve the Chief of Staff two-pane shell with a PageSkeleton split layout

.changelog/next/changed-issue-4144.md

@@ -0,0 +1 @@
+- Chief of Staff now paints a dimension-reserving two-pane loading skeleton instead of a centered spinner, via a new PageSkeleton `layout="split"` mode

client/src/components/ui/PageSkeleton.jsx

@@ -16,6 +16,13 @@
 //                      body region is loading (Goals).
 //   layout  'stack'  — vertically stacked cards, optional right sidebar.
 //           'grid'   — responsive card grid (dashboard widgets, tiles).
+//           'split'  — two-pane shell: a fixed-width side rail beside a
+//                      flexible main pane that owns the scroll (Chief of
+//                      Staff). Unlike `stack`/`grid` this owns the whole
+//                      shell, so it ignores `header` (a two-pane page's
+//                      title lives inside a pane) and renders the tab strip
+//                      INSIDE the main pane where a two-pane page puts it,
+//                      not above the split. Tuned by `split*`/`side*` below.
 //   tabs    n > 0    — reserves a `TabPills` (underline variant) strip under
 //                      the header. A default-size TabPills button is `text-sm`
 //                      (20px line box) + `py-3`, i.e. 44px — its
@@ -53,6 +60,25 @@ export default function PageSkeleton({
   fullHeight = false,
   barClassName = 'px-3 py-2 sm:px-4 sm:py-3',
   bodyClassName = 'p-3 sm:p-4',
+  // `layout="split"` only. Mirror whatever the loaded two-pane shell does:
+  //   sideCollapsed     — the rail is collapsed on desktop: reserve a
+  //                       zero-width track there and keep the rail's mobile
+  //                       band, which a collapsible page still renders.
+  //   splitColsClass    — the desktop grid tracks. Defaults off
+  //                       `sideCollapsed` so a collapsed rail can't reserve a
+  //                       320px column of nothing; pass your own when the
+  //                       page's rail isn't 320px wide.
+  //   sideClassName     — the rail's own box (borders, padding, scroll).
+  //   sideHero          — reserve a large circular block (avatar, portrait)
+  //                       at the top of the rail.
+  //   sideBlocks        — small blocks under the hero: a nav list at
+  //                       `grid-cols-1`, a stat grid at `grid-cols-2`.
+  sideCollapsed = false,
+  splitColsClass = sideCollapsed ? 'lg:grid-cols-[0px_1fr]' : 'lg:grid-cols-[320px_1fr]',
+  sideClassName = 'flex flex-col gap-3 border-b lg:border-b-0 lg:border-r border-port-border p-3 lg:p-4 lg:h-full lg:overflow-hidden',
+  sideHero = false,
+  sideBlocks = 4,
+  sideBlockColsClass = 'grid-cols-1',
   // Flex shape of the title/action row. The defaults are the common cases
   // (PageHeader's wrapping row for `bar`, stack-then-row for `inline`); pages
   // that break at a different width — or never stack at all — pass their own,
@@ -79,11 +105,74 @@ export default function PageSkeleton({
     </div>
   ));
 
+  const stackedCards = <div className="space-y-4">{cardBlocks}</div>;
+
+  const tabRows = repeat(tabs);
+  const sideBlockRows = repeat(sideBlocks);
+  const renderTabStrip = (bordered) => tabRows.length > 0 ? (
+    <div className={`shrink-0 flex gap-1 overflow-hidden ${bordered ? 'border-b border-port-border' : ''}`}>
+      {tabRows.map((_, i) => (
+        <div key={i} className="h-[44px] w-20 sm:w-24 flex items-center px-2">
+          <div className="h-4 w-full rounded bg-port-card animate-pulse" />
+        </div>
+      ))}
+    </div>
+  ) : null;
+
+  // `split` pages are the two-pane shells. The rail stacks above the main pane
+  // below `lg` (same as the loaded page), so the mobile band is reserved too.
+  if (layout === 'split') {
+    const sideRail = (
+      <>
+        <div className={`h-5 w-2/3 rounded bg-port-card animate-pulse ${sideHero ? 'mx-auto' : ''}`} />
+        {sideHero && (
+          <div className="mx-auto h-24 w-24 sm:h-32 sm:w-32 lg:h-40 lg:w-40 rounded-full bg-port-card animate-pulse" />
+        )}
+        {sideBlockRows.length > 0 && (
+          <div className={`grid gap-1.5 ${sideBlockColsClass}`}>
+            {sideBlockRows.map((_, i) => (
+              <div key={i} className="h-[52px] rounded border border-port-border bg-port-card animate-pulse" />
+            ))}
+          </div>
+        )}
+      </>
+    );
+
+    return (
+      <div
+        className={`relative flex flex-col lg:grid ${splitColsClass} overflow-hidden ${fullHeight ? 'h-full' : ''}`}
+        role="status"
+        aria-busy="true"
+        aria-label={label}
+      >
+        {sideCollapsed ? (
+          <>
+            {/* Desktop: the rail is collapsed to a zero-width track, but the
+                track still has to exist or the main pane lands in column 1. */}
+            <div className="hidden lg:block overflow-hidden min-w-0" />
+            <div className={`lg:hidden ${sideClassName}`}>{sideRail}</div>
+          </>
+        ) : (
+          <div className={sideClassName}>{sideRail}</div>
+        )}
+        {/* The main pane owns the scroll on a `fullHeight` split, same as the
+            loaded page — the root is `overflow-hidden`, so without this the
+            reserved cards are clipped instead of scrolling. */}
+        <div className={`flex-1 min-h-0 min-w-0 ${fullHeight ? 'overflow-y-auto overflow-x-hidden' : 'overflow-hidden'} ${padded ? bodyClassName : ''}`}>
+          {tabRows.length > 0 && <div className="mb-4 lg:mb-6">{renderTabStrip(true)}</div>}
+          {stackedCards}
+        </div>
+      </div>
+    );
+  }
+
+  // Built after the `split` return so a two-pane page doesn't allocate the
+  // stack/grid tree (and its sidebar card) it never renders.
   const body = layout === 'grid'
     ? <div className={`grid grid-cols-1 gap-4 items-start ${gridColsClass}`}>{cardBlocks}</div>
     : (
       <div className={`grid grid-cols-1 gap-6 items-start ${sidebar ? 'lg:grid-cols-[1fr_360px]' : ''}`}>
-        <div className="space-y-4">{cardBlocks}</div>
+        {stackedCards}
         {sidebar && (
           <div className="rounded-lg border border-port-border bg-port-card p-4 sm:p-6 animate-pulse">
             <div className="h-5 w-1/3 rounded bg-port-border mb-4" />
@@ -97,17 +186,6 @@ export default function PageSkeleton({
       </div>
     );
 
-  const tabRows = repeat(tabs);
-  const renderTabStrip = (bordered) => tabRows.length > 0 ? (
-    <div className={`shrink-0 flex gap-1 overflow-hidden ${bordered ? 'border-b border-port-border' : ''}`}>
-      {tabRows.map((_, i) => (
-        <div key={i} className="h-[44px] w-20 sm:w-24 flex items-center px-2">
-          <div className="h-4 w-full rounded bg-port-card animate-pulse" />
-        </div>
-      ))}
-    </div>
-  ) : null;
-
   // `bar` pages are the flex-column shells: the header bar and tab strip are
   // full-bleed, only the body region takes padding and owns the scroll.
   if (header === 'bar') {

client/src/components/ui/PageSkeleton.test.jsx

@@ -159,4 +159,88 @@ describe('PageSkeleton', () => {
     const infinite = render(<PageSkeleton cards={Infinity} sidebar={false} />);
     expect(cardCount(infinite.container)).toBe(64);
   });
+
+  describe('layout="split"', () => {
+    const sideBlocks = (container) => container.querySelectorAll('.h-\\[52px\\]');
+
+    it('reserves the two-pane grid tracks and drops the stack sidebar', () => {
+      const { container } = render(<PageSkeleton layout="split" cards={2} />);
+      expect(status().className).toContain('lg:grid');
+      expect(status().className).toContain('lg:grid-cols-[320px_1fr]');
+      // The stack layout's right sidebar must not sneak into the main pane.
+      expect(container.innerHTML).not.toContain('lg:grid-cols-[1fr_360px]');
+      expect(cardCount(container)).toBe(2);
+    });
+
+    it('collapses the rail track by default when sideCollapsed, without a caller override', () => {
+      // Otherwise a collapsed rail reserves 320px of nothing on desktop.
+      render(<PageSkeleton layout="split" sideCollapsed />);
+      expect(status().className).toContain('lg:grid-cols-[0px_1fr]');
+      expect(status().className).not.toContain('lg:grid-cols-[320px_1fr]');
+    });
+
+    it('takes caller-supplied grid tracks over the derived default', () => {
+      render(<PageSkeleton layout="split" splitColsClass="lg:grid-cols-[240px_1fr]" />);
+      expect(status().className).toContain('lg:grid-cols-[240px_1fr]');
+    });
+
+    it('renders the tab strip INSIDE the main pane, not above the split', () => {
+      const { container } = render(<PageSkeleton layout="split" tabs={3} sideBlocks={0} />);
+      const strip = container.querySelector('.h-\\[44px\\]').closest('.flex-1');
+      // The strip's pane is the main pane — the one holding the cards.
+      expect(strip).not.toBeNull();
+      expect(strip.querySelectorAll('.rounded-lg.border.border-port-border.bg-port-card').length)
+        .toBeGreaterThan(0);
+    });
+
+    it('reserves the hero block and the rail blocks in the requested column count', () => {
+      const { container } = render(
+        <PageSkeleton layout="split" sideHero sideBlocks={4} sideBlockColsClass="grid-cols-2" />
+      );
+      expect(container.querySelector('.rounded-full')).not.toBeNull();
+      expect(sideBlocks(container)).toHaveLength(4);
+      expect(container.innerHTML).toContain('grid gap-1.5 grid-cols-2');
+    });
+
+    it('omits the hero and the rail blocks when they are not requested', () => {
+      const { container } = render(<PageSkeleton layout="split" sideHero={false} sideBlocks={0} />);
+      expect(container.querySelector('.rounded-full')).toBeNull();
+      expect(sideBlocks(container)).toHaveLength(0);
+    });
+
+    it('keeps a zero-width desktop track plus the mobile band when sideCollapsed', () => {
+      const { container } = render(<PageSkeleton layout="split" sideCollapsed sideBlocks={2} />);
+      // The empty desktop track keeps the main pane in grid column 2.
+      expect(container.querySelector('.hidden.lg\\:block')).not.toBeNull();
+      // The rail itself only survives below `lg`, where the page stacks.
+      const rail = sideBlocks(container)[0].closest('.lg\\:hidden');
+      expect(rail).not.toBeNull();
+    });
+
+    it('owns the full height only when fullHeight is set', () => {
+      const tall = render(<PageSkeleton layout="split" fullHeight />);
+      expect(status().className).toContain('h-full');
+      tall.unmount();
+
+      render(<PageSkeleton layout="split" />);
+      expect(status().className).not.toContain('h-full');
+    });
+
+    it('gives the main pane the scroll on a fullHeight split, since the root hides overflow', () => {
+      const tall = render(<PageSkeleton layout="split" fullHeight sideBlocks={0} />);
+      expect(tall.container.querySelector('.flex-1').className).toContain('overflow-y-auto');
+      tall.unmount();
+
+      // Without fullHeight the shell doesn't own a viewport, so nothing scrolls.
+      const short = render(<PageSkeleton layout="split" sideBlocks={0} />);
+      expect(short.container.querySelector('.flex-1').className).not.toContain('overflow-y-auto');
+    });
+
+    it('pads the main pane with bodyClassName only when padded', () => {
+      const { container } = render(
+        <PageSkeleton layout="split" padded bodyClassName="p-3 lg:p-4" sideBlocks={0} />
+      );
+      expect(container.querySelector('.flex-1').className).toContain('p-3 lg:p-4');
+    });
+  });
 });

client/src/pages/ChiefOfStaff.jsx

@@ -9,6 +9,7 @@ import { Play, Pause, Square, Clock, CheckCircle, AlertCircle, Cpu, ChevronDown,
 import toast from '../components/ui/Toast';
 import BrailleSpinner from '../components/BrailleSpinner';
 import TabPills from '../components/ui/TabPills';
+import PageSkeleton from '../components/ui/PageSkeleton';
 
 // Import from modular components
 import {
@@ -637,10 +638,24 @@ export default function ChiefOfStaff() {
   };
 
   if (loading) {
+    // Reserve the loaded two-pane shell (#4144) — `/cos` is an `isFullWidth`
+    // route, so a centered spinner reserved nothing and the whole page jumped
+    // into place on first paint. `desktopPanelCollapsed` comes from
+    // localStorage, so the skeleton already knows which rail width to hold.
     return (
-      <div className="flex items-center justify-center h-64">
-        <BrailleSpinner text="Loading" />
-      </div>
+      <PageSkeleton
+        layout="split"
+        label="Loading Chief of Staff"
+        fullHeight
+        padded
+        bodyClassName="p-3 lg:p-4"
+        sideCollapsed={desktopPanelCollapsed}
+        sideClassName="flex flex-col gap-3 border-b lg:border-b-0 lg:border-r border-port-accent-2/20 bg-gradient-to-b from-port-card/80 to-port-card/40 p-3 lg:px-4 lg:py-6 lg:h-full lg:overflow-hidden"
+        sideHero
+        sideBlocks={4}
+        sideBlockColsClass="grid-cols-2"
+        tabs={TABS.length}
+      />
     );
   }
 

client/src/pages/ChiefOfStaff.test.jsx

@@ -85,6 +85,27 @@ const renderConfigTab = () => render(
   </MemoryRouter>,
 );
 
+// #4144 — `/cos` is an `isFullWidth` route, so its `<main>` is a bare
+// `relative overflow-hidden`. The old centered `h-64` BrailleSpinner reserved
+// none of the loaded two-pane shell, and the whole page jumped into place on
+// first paint. jsdom does no layout, so the guard pins the structure that
+// reserves those dimensions: the busy region IS the two-pane grid.
+describe('ChiefOfStaff loading skeleton', () => {
+  it('reserves the two-pane shell instead of a centered spinner while loading', async () => {
+    // Hold the first fetch open so the loading branch is what renders.
+    api.getCosStatus.mockReturnValue(new Promise(() => {}));
+    const { container } = renderConfigTab();
+
+    const busy = await screen.findByRole('status');
+    expect(busy).toHaveAttribute('aria-busy', 'true');
+    expect(busy).toHaveAttribute('aria-label', 'Loading Chief of Staff');
+    expect(busy.className).toContain('lg:grid-cols-[320px_1fr]');
+    expect(busy.className).toContain('h-full');
+    // The old spinner shell — a fixed 16rem box centering its child.
+    expect(container.querySelector('.h-64')).toBeNull();
+  });
+});
+
 describe('ChiefOfStaff handleForceEvaluate', () => {
   it('does not toast success or advance the status message when the evaluate fails', async () => {
     api.forceCosEvaluate.mockRejectedValue(new Error('evaluate failed'));