From 0a3cb28eb7a150587640a8aa8593896c59c6e047 Mon Sep 17 00:00:00 2001 From: interacsean Date: Wed, 12 Aug 2026 12:40:03 +1000 Subject: [PATCH 1/4] feat(core): add SidebarLayout body slot for custom page layouts (#1643) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an eject seam for everything to the right of the sidebar, so consumers can arrange their own columns — a table-of-contents rail, an edge-docked assistant panel — without overriding AppShell internals. - `body` prop on SidebarLayout replaces the region beside the sidebar. As a flex sibling it reflows on sidebar collapse for free. Supersedes `header` and `children`; warns in dev when combined. - `SidebarLayout.ContentContainer` extracts the stock content column (inset padding, pinned header slot, scroll region) and now owns the scroll ref, so `useAppShellScrollContainer()` keeps working inside a custom body. The default path renders the same component — one implementation, not two. - `SidebarLayout.Outlet` / `SidebarLayout.Trigger` expose the page outlet and the collapse toggle for composing custom bodies. - `useAppShellSidebar()` exposes collapse state and toggling, replacing MutationObserver-on-[data-state] plus hidden-trigger .click() workarounds. - `SidebarInset` uses `min-w-0` instead of `w-[calc(100%-var(--sidebar-width))]`. That calc hardcoded "exactly one 16rem sibling" and dated to the initial scaffold; min-w-0 is the flex idiom already used elsewhere in the file and is what lets sibling columns share the row. Pilot — docs, changeset and tests still to come. Co-Authored-By: Claude Opus 5 --- examples/vite-app/src/App.tsx | 26 ++-- .../src/pages/dashboard/panels/page.tsx | 70 ++++++++++ examples/vite-app/src/panel-sections.ts | 11 ++ examples/vite-app/src/panels-body.tsx | 120 ++++++++++++++++++ examples/vite-app/src/routes.generated.ts | 1 + packages/core/src/components/sidebar.tsx | 8 +- .../components/sidebar/content-container.tsx | 97 ++++++++++++++ packages/core/src/components/sidebar/index.ts | 2 + .../src/components/sidebar/sidebar-layout.tsx | 115 +++++++++-------- .../components/sidebar/sidebar-trigger.tsx | 37 ++++++ .../sidebar/use-app-shell-sidebar.ts | 67 ++++++++++ packages/core/src/index.ts | 5 + 12 files changed, 490 insertions(+), 69 deletions(-) create mode 100644 examples/vite-app/src/pages/dashboard/panels/page.tsx create mode 100644 examples/vite-app/src/panel-sections.ts create mode 100644 examples/vite-app/src/panels-body.tsx create mode 100644 packages/core/src/components/sidebar/content-container.tsx create mode 100644 packages/core/src/components/sidebar/sidebar-trigger.tsx create mode 100644 packages/core/src/components/sidebar/use-app-shell-sidebar.ts diff --git a/examples/vite-app/src/App.tsx b/examples/vite-app/src/App.tsx index bdba02fa..8731cab6 100644 --- a/examples/vite-app/src/App.tsx +++ b/examples/vite-app/src/App.tsx @@ -1,15 +1,13 @@ import { AppShell, - AppearanceSwitcher, - Button, SidebarGroup, SidebarItem, SidebarLayout, type SearchSource, } from "@tailor-platform/app-shell"; -import { BellIcon, CircleUserIcon } from "lucide-react"; import { searchOrders, searchRecentOrders } from "./fake-search"; import { labels } from "./i18n-labels"; +import { PanelsBody } from "./panels-body"; // Demonstrates multiple search sources in the command palette const searchSources: SearchSource[] = [ @@ -29,21 +27,12 @@ const App = () => { return ( - - , - , - // Opt back into the appearance switcher — `actions` replaces the - // default right-hand cluster, so include it explicitly to keep it. - , - ]} - /> - } + // `body` replaces everything to the right of the sidebar. PanelsBody + // renders the stock content column via SidebarLayout.ContentContainer + // (so the header/padding/scrolling are unchanged) and adds page-specific + // columns beside it on /dashboard/panels. The header that used to live + // on the `header` prop moved inside PanelsBody. + body={} sidebar={ @@ -52,6 +41,7 @@ const App = () => { + diff --git a/examples/vite-app/src/pages/dashboard/panels/page.tsx b/examples/vite-app/src/pages/dashboard/panels/page.tsx new file mode 100644 index 00000000..62566c14 --- /dev/null +++ b/examples/vite-app/src/pages/dashboard/panels/page.tsx @@ -0,0 +1,70 @@ +import { + Layout, + useAppShellScrollContainer, + type AppShellPageProps, +} from "@tailor-platform/app-shell"; +import { Columns3 } from "lucide-react"; +import { useEffect, useState } from "react"; +import { PANEL_SECTIONS, sectionId } from "../../../panel-sections"; + +/** + * Demo page for the `SidebarLayout` `body` slot (issue #1643). + * + * The two extra columns are contributed by `PanelsBody` in `src/panels-body.tsx` + * — the page itself is an ordinary page and knows nothing about them. It exists + * to prove the content column still behaves normally when the body is ejected: + * breadcrumb header, `md:px-8` inset padding, and a working + * `useAppShellScrollContainer()`. + */ +const PanelsPage = () => { + const scrollRef = useAppShellScrollContainer(); + const [scrollTop, setScrollTop] = useState(null); + + // Regression check: the scroll container is now provided by ContentContainer + // rather than SidebarLayout, so this must still resolve inside a `body`. + useEffect(() => { + const el = scrollRef.current; + if (!el) return; + const onScroll = () => setScrollTop(el.scrollTop); + onScroll(); + el.addEventListener("scroll", onScroll, { passive: true }); + return () => el.removeEventListener("scroll", onScroll); + }, [scrollRef]); + + return ( + + + +

+ useAppShellScrollContainer():{" "} + {scrollTop === null + ? "not resolved ✗" + : `resolved ✓ — scrollTop ${Math.round(scrollTop)}`} +

+ {PANEL_SECTIONS.map((section) => ( +
+

{section}

+

+ Filler copy so the content column scrolls independently of the two side columns. The + rail on the left and the assistant on the right stay put while this scrolls, and each + has its own scrollbar. +

+

+ Collapse the sidebar (⌘B) and every column reflows — they are flex siblings of the + sidebar, so nothing is pinned to a hardcoded width. +

+
+ ))} + + + ); +}; + +PanelsPage.appShellPageProps = { + meta: { + title: "Body Slot", + icon: , + }, +} satisfies AppShellPageProps; + +export default PanelsPage; diff --git a/examples/vite-app/src/panel-sections.ts b/examples/vite-app/src/panel-sections.ts new file mode 100644 index 00000000..0f357625 --- /dev/null +++ b/examples/vite-app/src/panel-sections.ts @@ -0,0 +1,11 @@ +export const PANEL_SECTIONS = [ + "Introduction", + "Installing", + "Configuration", + "Authoring documents", + "Review workflow", + "Publishing", + "Troubleshooting", +] as const; + +export const sectionId = (title: string) => `section-${title.toLowerCase().replace(/\s+/g, "-")}`; diff --git a/examples/vite-app/src/panels-body.tsx b/examples/vite-app/src/panels-body.tsx new file mode 100644 index 00000000..bbf32d30 --- /dev/null +++ b/examples/vite-app/src/panels-body.tsx @@ -0,0 +1,120 @@ +import { + AppearanceSwitcher, + Button, + SidebarLayout, + useAppShellSidebar, + useLocation, +} from "@tailor-platform/app-shell"; +import { BellIcon, CircleUserIcon, PanelRightIcon } from "lucide-react"; +import { useState } from "react"; +import { PANEL_SECTIONS, sectionId } from "./panel-sections"; + +const Header = ( + + + , + , + , + ]} + /> +); + +/** + * Reproduces knowledge#312 — a page-specific table-of-contents rail sitting + * flush between the main nav sidebar and the content column. + */ +const TocRail = () => { + // Proves the supported replacement for the MutationObserver hack: read the + // main sidebar's collapsed state without touching [data-state] in the DOM. + const { open, toggle } = useAppShellSidebar(); + + return ( + + ); +}; + +/** + * Reproduces knowledge#345 — an assistant panel docked flush against the + * viewport edge, while the content column keeps its normal inset chrome. + */ +const AssistantPanel = ({ onClose }: { onClose: () => void }) => ( + +); + +/** + * The `body` slot is configured once at the app level, so page-specific columns + * are driven off the current route. Everything here is a flex row beside the + * sidebar, so it all reflows when the sidebar collapses. + */ +export const PanelsBody = () => { + const location = useLocation(); + const [assistantOpen, setAssistantOpen] = useState(true); + const onPanelsPage = location.pathname === "/dashboard/panels"; + + return ( + <> + {onPanelsPage && } + +
{Header}
+ + + ) : ( + Header + ) + } + > + + + {onPanelsPage && assistantOpen && setAssistantOpen(false)} />} + + ); +}; diff --git a/examples/vite-app/src/routes.generated.ts b/examples/vite-app/src/routes.generated.ts index b1e3ade7..ecd5e603 100644 --- a/examples/vite-app/src/routes.generated.ts +++ b/examples/vite-app/src/routes.generated.ts @@ -21,6 +21,7 @@ export type GeneratedRouteParams = { "/dashboard/long-content": {}; "/dashboard/orders": {}; "/dashboard/orders/:id": { id: string }; + "/dashboard/panels": {}; "/dashboard/products": {}; "/data-table": {}; "/data-table-lab": {}; diff --git a/packages/core/src/components/sidebar.tsx b/packages/core/src/components/sidebar.tsx index 86d5e6d4..76ad6937 100644 --- a/packages/core/src/components/sidebar.tsx +++ b/packages/core/src/components/sidebar.tsx @@ -390,7 +390,12 @@ function SidebarInset({ className, ...props }: React.ComponentProps<"main">) {
` for the + * built-in top bar, your own node to replace it, or omit it entirely for a + * bare content column. + */ + header?: React.ReactNode; + + /** Extra classes for the content column itself (the `
` element). */ + className?: string; + + /** Scrolling page content. Usually ``. */ + children?: React.ReactNode; +}; + +/** + * ContentContainer — the stock AppShell content column. + * + * This is what `SidebarLayout` renders beside the sidebar by default: the + * padded `
` inset, a pinned `header` slot, and the scroll region that + * owns vertical scrolling for page content (and backs + * `useAppShellScrollContainer()`). + * + * You only reach for it directly when using the `body` slot to lay out your own + * columns — drop it in among them and the main column keeps its normal inset + * chrome and scroll behaviour, instead of you rebuilding both by hand. + * + * @example + * ```tsx + * + * + * }> + * + * + * + * + * } + * /> + * ``` + */ +export function ContentContainer({ header, className, children }: ContentContainerProps) { + // Handle to the content scroll region, exposed to pages via + // `useAppShellScrollContainer()`. The shell is viewport-bounded, so this is + // the element that scrolls page content (what `window` used to be). + const scrollContainerRef = useRef(null); + + return ( + + {header} + {/* Content scroll region. The shell is viewport-bounded (h-svh on the + sidebar wrapper), so regular pages scroll here; pages that pin + their own chrome (e.g. with a DataTable) size to fit + and don't scroll this area. Exposed to pages via + `useAppShellScrollContainer()` and the `data-appshell-scroll-container` + marker — the supported handle for what used to be `window` scroll. */} + }> +
page bounds everything internally, so this area + // never needs to scroll: clip instead. This also makes the fade + // inert (no scroll range) and avoids any sub-pixel-overflow bar. + "astw:has-data-[layout-fill]:overflow-hidden", + // Reserve the scrollbar gutter so content doesn't shift when the + // bar toggles across navigations (no-op with overlay scrollbars). + "astw:[scrollbar-gutter:stable]", + )} + > + {children} +
+ + + ); +} +ContentContainer.displayName = "SidebarLayout.ContentContainer"; diff --git a/packages/core/src/components/sidebar/index.ts b/packages/core/src/components/sidebar/index.ts index 47ee4a05..369d72b9 100644 --- a/packages/core/src/components/sidebar/index.ts +++ b/packages/core/src/components/sidebar/index.ts @@ -4,3 +4,5 @@ export { SidebarSeparator } from "./sidebar-separator"; export { DefaultSidebar, type DefaultSidebarProps } from "./default-sidebar"; export { DefaultHeader, type DefaultHeaderProps } from "./default-header"; export { SidebarLayout, type SidebarLayoutProps } from "./sidebar-layout"; +export { ContentContainer, type ContentContainerProps } from "./content-container"; +export { useAppShellSidebar, type AppShellSidebarState } from "./use-app-shell-sidebar"; diff --git a/packages/core/src/components/sidebar/sidebar-layout.tsx b/packages/core/src/components/sidebar/sidebar-layout.tsx index 1d4aeefc..d61e1af6 100644 --- a/packages/core/src/components/sidebar/sidebar-layout.tsx +++ b/packages/core/src/components/sidebar/sidebar-layout.tsx @@ -1,15 +1,16 @@ -import { useRef, type RefObject } from "react"; -import { SidebarProvider, SidebarInset } from "@/components/sidebar"; +import { SidebarProvider } from "@/components/sidebar"; import { AppShellOutlet } from "@/components/content"; -import { AppShellScrollContainerProvider } from "@/contexts/scroll-container-context"; import { DefaultSidebar } from "./default-sidebar"; import { DefaultHeader } from "./default-header"; -import { cn } from "@/lib/utils"; +import { ContentContainer } from "./content-container"; +import { Trigger } from "./sidebar-trigger"; export type SidebarLayoutProps = { /** * Custom content renderer. * + * Ignored when `body` is set — `body` replaces the region this renders into. + * * @example * ```tsx * @@ -43,6 +44,9 @@ export type SidebarLayoutProps = { * (e.g. add a notification bell), pass `` with * its `actions` slot rather than reconstructing the header from scratch. * + * Ignored when `body` is set — with `body` you place the header yourself, + * inside (or outside) ``. + * * @default * @example * ```tsx @@ -61,6 +65,47 @@ export type SidebarLayoutProps = { */ header?: React.ReactNode; + /** + * Replaces everything to the right of the sidebar — the escape hatch for + * page layouts the default content column can't express, such as a + * table-of-contents rail or an assistant panel docked flush against the + * viewport edge. + * + * Whatever you pass becomes a flex row alongside the sidebar, so it widens + * and narrows with the sidebar automatically. Compose it from the namespaced + * building blocks rather than rebuilding them: + * + * - `` — the stock content column (inset + * padding, pinned header slot, scroll region, `useAppShellScrollContainer()`) + * - `` — the current page + * - `` — the built-in top bar + * - `` — the sidebar collapse toggle + * - `useAppShellSidebar()` — subscribe to the sidebar's collapsed state + * + * Setting `body` takes over the whole region, so `header` and `children` no + * longer apply and are ignored. + * + * @example + * ```tsx + * + * + * }> + * + * + * + * + * } + * /> + * ``` + */ + body?: React.ReactNode; + /** * Whether the sidebar is open by default on desktop. * @@ -79,11 +124,13 @@ export type SidebarLayoutProps = { }; export function SidebarLayout(props: SidebarLayoutProps) { - const Children = props.children ? props.children({ Outlet: AppShellOutlet }) : null; - // Handle to the content scroll region, exposed to pages via - // `useAppShellScrollContainer()`. The shell is viewport-bounded, so this is - // the element that scrolls page content (what `window` used to be). - const scrollContainerRef = useRef(null); + if (props.body && (props.header || props.children)) { + console.warn( + "[AppShell] SidebarLayout received `body` alongside `header` and/or `children`. " + + "`body` replaces the entire region to the right of the sidebar, so those props are ignored. " + + "Place your header inside `body` (e.g. via ) instead.", + ); + } return (
{props.sidebar ?? } - - {props.header ?? } - {/* Content scroll region. The shell is viewport-bounded (h-svh on the - sidebar wrapper), so regular pages scroll here; pages that pin - their own chrome (e.g. with a DataTable) size to fit - and don't scroll this area. Exposed to pages via - `useAppShellScrollContainer()` and the `data-appshell-scroll-container` - marker — the supported handle for what used to be `window` scroll. */} - } - > -
page bounds everything internally, so this area - // never needs to scroll: clip instead. This also makes the fade - // inert (no scroll range) and avoids any sub-pixel-overflow bar. - "astw:has-data-[layout-fill]:overflow-hidden", - // Reserve the scrollbar gutter so content doesn't shift when the - // bar toggles across navigations (no-op with overlay scrollbars). - "astw:[scrollbar-gutter:stable]", - )} - > - {Children ?? } -
- - + {props.body ?? ( + }> + {props.children ? props.children({ Outlet: AppShellOutlet }) : } + + )}
); @@ -143,3 +154,7 @@ export function SidebarLayout(props: SidebarLayoutProps) { // remains available as a top-level export for backwards compatibility. SidebarLayout.DefaultSidebar = DefaultSidebar; SidebarLayout.DefaultHeader = DefaultHeader; +// Building blocks for the `body` slot. +SidebarLayout.ContentContainer = ContentContainer; +SidebarLayout.Outlet = AppShellOutlet; +SidebarLayout.Trigger = Trigger; diff --git a/packages/core/src/components/sidebar/sidebar-trigger.tsx b/packages/core/src/components/sidebar/sidebar-trigger.tsx new file mode 100644 index 00000000..de764e15 --- /dev/null +++ b/packages/core/src/components/sidebar/sidebar-trigger.tsx @@ -0,0 +1,37 @@ +import { SidebarTrigger as SidebarTriggerPrimitive } from "@/components/sidebar"; + +export type TriggerProps = { + /** Extra classes for the trigger button. */ + className?: string; + /** + * Runs before the sidebar toggles. The toggle itself is not cancellable — + * use `useAppShellSidebar()` if you need to drive the state directly. + */ + onClick?: React.MouseEventHandler; +}; + +/** + * Trigger — the sidebar collapse toggle, as used by the built-in header. + * + * Render it when composing your own header (or a `body` layout) so users keep + * a way to collapse the nav. Prefer this over reaching for the built-in + * trigger through the DOM. + * + * @example + * ```tsx + * + * + *

My header

+ * + * } + * > + * + * + * ``` + */ +export const Trigger = ({ className, onClick }: TriggerProps) => ( + +); +Trigger.displayName = "SidebarLayout.Trigger"; diff --git a/packages/core/src/components/sidebar/use-app-shell-sidebar.ts b/packages/core/src/components/sidebar/use-app-shell-sidebar.ts new file mode 100644 index 00000000..9a74aa23 --- /dev/null +++ b/packages/core/src/components/sidebar/use-app-shell-sidebar.ts @@ -0,0 +1,67 @@ +import * as React from "react"; +import { SidebarContext } from "@/components/sidebar"; + +export type AppShellSidebarState = { + /** + * Whether the sidebar is expanded on desktop — the same signal the DOM + * exposes as `[data-slot="sidebar"][data-state]`. + * + * On mobile the sidebar is an overlay sheet rather than an in-flow column, + * so this does not describe how much room the content has; branch on + * `isMobile` when that distinction matters. + */ + open: boolean; + + /** Whether the viewport is below the mobile breakpoint (768px). */ + isMobile: boolean; + + /** Expand or collapse the sidebar. */ + setOpen: (open: boolean) => void; + + /** + * Toggle the sidebar — the same action as `` and the + * ⌘B / Ctrl+B shortcut, including the mobile and tablet overlay behaviour. + */ + toggle: () => void; +}; + +const noop = () => {}; + +// Stable inert value so the hook is safe to call outside a SidebarLayout, in +// the same spirit as `useAppShellScrollContainer()` handing back an empty ref +// rather than throwing. +const FALLBACK: AppShellSidebarState = { + open: true, + isMobile: false, + setOpen: noop, + toggle: noop, +}; + +/** + * Read and control the AppShell sidebar's collapsed state. + * + * Use this instead of observing `[data-slot="sidebar"][data-state]` or clicking + * the trigger through the DOM — those reach into internals that can change + * between releases. + * + * ```tsx + * const { open, toggle } = useAppShellSidebar(); + * return ; + * ``` + * + * Outside a `SidebarLayout` there is no sidebar to describe, so this reports + * `open: true` with no-op setters rather than throwing. + */ +export function useAppShellSidebar(): AppShellSidebarState { + const context = React.useContext(SidebarContext); + + return React.useMemo(() => { + if (!context) return FALLBACK; + return { + open: context.open, + isMobile: context.isMobile, + setOpen: context.setOpen, + toggle: context.toggleSidebar, + }; + }, [context]); +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index d136eaa0..87f81d0a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -20,8 +20,13 @@ export { type SidebarLayoutProps, type DefaultSidebarProps, type DefaultHeaderProps, + type ContentContainerProps, } from "./components/sidebar/index"; +// Sidebar collapse state — the supported alternative to observing +// `[data-slot="sidebar"][data-state]` or clicking the trigger via the DOM. +export { useAppShellSidebar, type AppShellSidebarState } from "./components/sidebar/index"; + // Guard component for conditional rendering export { WithGuard, type WithGuardProps } from "./components/with-guard"; From e7e62a5f1c9ec777aa384a2b1b47911d84167102 Mon Sep 17 00:00:00 2001 From: interacsean Date: Wed, 12 Aug 2026 16:14:30 +1000 Subject: [PATCH 2/4] docs(decisions): options for page-level body columns (#1643) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `body` slot is configured at app level, but both field reports that motivated it are page-scoped. Writes up three options — route-aware body (works today), a portal-based page component, and page-metadata declaration — plus one rejected approach. Recommends the portal component: both reported panels are stateful and page-scoped, and it is the only option that keeps the panel inside the page's React tree, so page state flows in without being lifted into the shell. Co-Authored-By: Claude Opus 5 --- decisions/page-level-body-columns.md | 107 +++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 decisions/page-level-body-columns.md diff --git a/decisions/page-level-body-columns.md b/decisions/page-level-body-columns.md new file mode 100644 index 00000000..f120f353 --- /dev/null +++ b/decisions/page-level-body-columns.md @@ -0,0 +1,107 @@ +# Decision: page-level control of `SidebarLayout` body columns + +> Status: **Open — options below, recommendation is Option B.** +> Context: follow-up to the `body` slot ([#1643](https://github.com/tailor-inc/platform-planning/issues/1643)). + +## Problem + +`body` is configured once, at app level, where `` is rendered. But both +field reports that motivated it are **page-level**: + +- [knowledge#312](https://github.com/tailor-professional-service/knowledge/discussions/312) — a TOC rail on the manuals page +- [knowledge#345](https://github.com/tailor-professional-service/knowledge/discussions/345) — a chat panel on the supplier-evaluation page + +So a page can't contribute a column; only the shell can. Something has to bridge that gap. + +**Constraint worth naming up front:** both panels need _page_ state. The TOC tracks which +document is selected and which nodes are expanded; the chat panel is scoped to the supplier +being evaluated. Any option that renders the panel outside the page's React tree forces that +state up into the shell or into a second data fetch. + +## Option A — Route-aware body (works today, no new API) + +The body component branches on `useLocation()`. This is what `examples/vite-app/src/panels-body.tsx` does. + +```tsx +const AppBody = () => { + const { pathname } = useLocation(); + return ( + <> + {pathname === "/manuals" && } + }> + + + + ); +}; +``` + +- **For:** zero new API, ships today, fully explicit, easy to reason about. +- **Against:** shell config accumulates knowledge of page routes; route strings get duplicated + and drift; the panel lives far from the page that owns it; the branch grows with every page; + page state has to be lifted into the shell or threaded through a bespoke context. +- **Fits:** app-wide or coarse-grained panels (a global assistant, a nav rail for one section). + +## Option B — Portal slot component (recommended) + +`SidebarLayout` renders empty dock containers as flex siblings of `ContentContainer`. A page +renders `` anywhere in its own tree; it portals into the dock. + +```tsx +export default function ManualsPage() { + const [selected, setSelected] = useState(null); + return ( + <> + + + + … + + ); +} +``` + +- **For:** the page owns its panel; it mounts and unmounts with navigation automatically; no + shell config at all. React portals preserve context from where they're _declared_, so page + state and context flow into the panel with no lifting — which is exactly what both reports need. +- **Against:** new public component; portal indirection to explain; needs a defined ordering rule + when two panels claim the same side; renders nothing on the first SSR pass (no hydration + mismatch — server and first client render both produce `null`). + +## Option C — Declare panels in page metadata + +Pages declare panels alongside their existing meta; the shell reads the matched route and renders them. + +```tsx +ManualsPage.appShellPageProps = { + meta: { title: "Manuals" }, + panels: { left: TocRail }, +} satisfies AppShellPageProps; +``` + +- **For:** declarative, fits the existing module/resource system, no portal, shell keeps full + control of layout. +- **Against:** the panel renders **outside** the page's tree, so it can't see page state — it + needs its own fetching and its own state, or a shared store. That's disqualifying for both + reported cases. Also static component references only, so no props from the page. +- **Fits:** genuinely static, self-sufficient rails. + +## Rejected — imperative registration hook + +`usePagePanel({ side, children })`, with the shell rendering whatever pages register. This is a +worse portal: passing nodes through context state means they reconcile against the _shell's_ +tree, so panel children remount whenever the shell re-renders, and registration during render +is a setState-in-render hazard. Option B gets the same ergonomics with correct semantics. + +## Recommendation + +**Option B**, keeping **Option A** as the supported path for app-wide panels — they compose +fine, since a portal-based `SidePanel` and a route-aware body are both just children of the +same flex row. + +The deciding factor is the page-state constraint: B is the only option where the panel sits in +the page's React tree, and both reported panels are stateful and page-scoped. C is worth +revisiting only if a static-rail use case shows up that A doesn't already cover. + +Not urgent — `body` unblocks both reports today via Option A. This is about whether the +ergonomics are good enough that consumers stop hand-rolling, which is the actual goal. From 7e697030c51e532cd202974f1c243278632f97de Mon Sep 17 00:00:00 2001 From: interacsean Date: Thu, 13 Aug 2026 17:57:16 +1000 Subject: [PATCH 3/4] refactor(core): make SidebarLayout body and header mutually exclusive by type (#1643) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SidebarLayoutProps` becomes a union of two variants: the default layout (`header` + `children`) and the ejected one (`body`). Each marks the other's props `never`, so passing `body` alongside `header`/`children` is a compile error rather than a runtime warning after the confusion has already shipped. This matters most for the case where a header is configured once at the AppShell level and then a second one is placed inside `body` — previously the outer one was silently dropped. The runtime warn stays as a backstop: types are erased, so JS consumers and `as any` escapes still get told rather than losing a header silently. Verified the union accepts `header`/`children`/bare/`body`-only and rejects `body`+`header` and `body`+`children`. Co-Authored-By: Claude Opus 5 --- .../src/components/sidebar/sidebar-layout.tsx | 134 +++++++++++------- 1 file changed, 86 insertions(+), 48 deletions(-) diff --git a/packages/core/src/components/sidebar/sidebar-layout.tsx b/packages/core/src/components/sidebar/sidebar-layout.tsx index d61e1af6..34b4bcea 100644 --- a/packages/core/src/components/sidebar/sidebar-layout.tsx +++ b/packages/core/src/components/sidebar/sidebar-layout.tsx @@ -5,27 +5,7 @@ import { DefaultHeader } from "./default-header"; import { ContentContainer } from "./content-container"; import { Trigger } from "./sidebar-trigger"; -export type SidebarLayoutProps = { - /** - * Custom content renderer. - * - * Ignored when `body` is set — `body` replaces the region this renders into. - * - * @example - * ```tsx - * - * {({ Outlet }) => ( - * <> - * - * - * - * - * )} - * - * ``` - */ - children?: (props: { Outlet: () => React.ReactNode }) => React.ReactNode; - +type SidebarLayoutCommonProps = { /** * Custom sidebar content. Replaces the whole sidebar region. * @@ -37,6 +17,28 @@ export type SidebarLayoutProps = { */ sidebar?: React.ReactNode; + /** + * Whether the sidebar is open by default on desktop. + * + * @default true + */ + defaultOpen?: boolean; + + /** + * Whether the sidebar can be collapsed. + * When set to `false`, the sidebar is always visible and cannot be toggled. + * `defaultOpen` is ignored when this is `false`. + * + * @default true + */ + collapsible?: boolean; +}; + +/** + * The default layout: AppShell owns the content column, you customise its + * header and content. + */ +type SidebarLayoutDefaultProps = SidebarLayoutCommonProps & { /** * Custom header content. Replaces the whole top-bar region. * @@ -44,9 +46,6 @@ export type SidebarLayoutProps = { * (e.g. add a notification bell), pass `` with * its `actions` slot rather than reconstructing the header from scratch. * - * Ignored when `body` is set — with `body` you place the header yourself, - * inside (or outside) ``. - * * @default * @example * ```tsx @@ -65,6 +64,34 @@ export type SidebarLayoutProps = { */ header?: React.ReactNode; + /** + * Custom content renderer. + * + * @example + * ```tsx + * + * {({ Outlet }) => ( + * <> + * + * + * + * + * )} + * + * ``` + */ + children?: (props: { Outlet: () => React.ReactNode }) => React.ReactNode; + + /** Not available alongside `header`/`children` — see the `body` overload. */ + body?: never; +}; + +/** + * The eject: you own the whole region beside the sidebar, including where the + * header goes. `header` and `children` are unavailable here by construction — + * they describe a content column you are now supplying yourself. + */ +type SidebarLayoutBodyProps = SidebarLayoutCommonProps & { /** * Replaces everything to the right of the sidebar — the escape hatch for * page layouts the default content column can't express, such as a @@ -82,8 +109,8 @@ export type SidebarLayoutProps = { * - `` — the sidebar collapse toggle * - `useAppShellSidebar()` — subscribe to the sidebar's collapsed state * - * Setting `body` takes over the whole region, so `header` and `children` no - * longer apply and are ignored. + * The header lives inside `body` too — pass it to `ContentContainer` so it + * stays pinned above that column's scroll region. * * @example * ```tsx @@ -104,27 +131,38 @@ export type SidebarLayoutProps = { * /> * ``` */ - body?: React.ReactNode; + body: React.ReactNode; - /** - * Whether the sidebar is open by default on desktop. - * - * @default true - */ - defaultOpen?: boolean; + /** Place your header inside `body`, via ``. */ + header?: never; - /** - * Whether the sidebar can be collapsed. - * When set to `false`, the sidebar is always visible and cannot be toggled. - * `defaultOpen` is ignored when this is `false`. - * - * @default true - */ - collapsible?: boolean; + /** Place your content inside `body`, via ``. */ + children?: never; +}; + +/** + * Either the default layout (`header` + `children`) or the ejected one (`body`) + * — never both. Passing `body` alongside `header`/`children` is a type error, + * because `body` replaces the very region those two describe. + */ +export type SidebarLayoutProps = SidebarLayoutDefaultProps | SidebarLayoutBodyProps; + +// Widened view of the union for use inside the component, where both branches +// are handled at once. +type SidebarLayoutAnyProps = SidebarLayoutCommonProps & { + header?: React.ReactNode; + children?: (props: { Outlet: () => React.ReactNode }) => React.ReactNode; + body?: React.ReactNode; }; export function SidebarLayout(props: SidebarLayoutProps) { - if (props.body && (props.header || props.children)) { + const { sidebar, header, children, body, defaultOpen, collapsible } = + props as SidebarLayoutAnyProps; + + // The union already makes this a type error. Types are erased at runtime + // though, so keep the guard for JS consumers and `as any` escapes — the + // failure mode otherwise is a silently dropped header. + if (body && (header || children)) { console.warn( "[AppShell] SidebarLayout received `body` alongside `header` and/or `children`. " + "`body` replaces the entire region to the right of the sidebar, so those props are ignored. " + @@ -134,15 +172,15 @@ export function SidebarLayout(props: SidebarLayoutProps) { return (
- {props.sidebar ?? } - {props.body ?? ( - }> - {props.children ? props.children({ Outlet: AppShellOutlet }) : } + {sidebar ?? } + {body ?? ( + }> + {children ? children({ Outlet: AppShellOutlet }) : } )}
From 1d02b113292d394a78e533fe2a6f9aa2cf844b38 Mon Sep 17 00:00:00 2001 From: interacsean Date: Thu, 13 Aug 2026 18:16:36 +1000 Subject: [PATCH 4/4] refactor(core): drop SidebarLayout body/header runtime warn (#1643) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The union already makes `body` + `header`/`children` a compile error, so the runtime guard only ever fired for consumers who had bypassed the types. Not worth the shipped bytes. Removing it also removed the need for the widened internal props alias — the component destructures straight off the union, since every member declares all six props. Co-Authored-By: Claude Opus 5 --- .../src/components/sidebar/sidebar-layout.tsx | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/packages/core/src/components/sidebar/sidebar-layout.tsx b/packages/core/src/components/sidebar/sidebar-layout.tsx index 34b4bcea..16e117f8 100644 --- a/packages/core/src/components/sidebar/sidebar-layout.tsx +++ b/packages/core/src/components/sidebar/sidebar-layout.tsx @@ -147,28 +147,8 @@ type SidebarLayoutBodyProps = SidebarLayoutCommonProps & { */ export type SidebarLayoutProps = SidebarLayoutDefaultProps | SidebarLayoutBodyProps; -// Widened view of the union for use inside the component, where both branches -// are handled at once. -type SidebarLayoutAnyProps = SidebarLayoutCommonProps & { - header?: React.ReactNode; - children?: (props: { Outlet: () => React.ReactNode }) => React.ReactNode; - body?: React.ReactNode; -}; - export function SidebarLayout(props: SidebarLayoutProps) { - const { sidebar, header, children, body, defaultOpen, collapsible } = - props as SidebarLayoutAnyProps; - - // The union already makes this a type error. Types are erased at runtime - // though, so keep the guard for JS consumers and `as any` escapes — the - // failure mode otherwise is a silently dropped header. - if (body && (header || children)) { - console.warn( - "[AppShell] SidebarLayout received `body` alongside `header` and/or `children`. " + - "`body` replaces the entire region to the right of the sidebar, so those props are ignored. " + - "Place your header inside `body` (e.g. via ) instead.", - ); - } + const { sidebar, header, children, body, defaultOpen, collapsible } = props; return (