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. 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..16e117f8 100644 --- a/packages/core/src/components/sidebar/sidebar-layout.tsx +++ b/packages/core/src/components/sidebar/sidebar-layout.tsx @@ -1,30 +1,11 @@ -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"; - -export type SidebarLayoutProps = { - /** - * Custom content renderer. - * - * @example - * ```tsx - * - * {({ Outlet }) => ( - * <> - * - * - * - * - * )} - * - * ``` - */ - children?: (props: { Outlet: () => React.ReactNode }) => React.ReactNode; +import { ContentContainer } from "./content-container"; +import { Trigger } from "./sidebar-trigger"; +type SidebarLayoutCommonProps = { /** * Custom sidebar content. Replaces the whole sidebar region. * @@ -36,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. * @@ -62,78 +65,104 @@ export type SidebarLayoutProps = { header?: React.ReactNode; /** - * Whether the sidebar is open by default on desktop. + * Custom content renderer. * - * @default true + * @example + * ```tsx + * + * {({ Outlet }) => ( + * <> + * + * + * + * + * )} + * + * ``` */ - defaultOpen?: boolean; + 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 & { /** - * 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`. + * 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. * - * @default true + * 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 + * + * The header lives inside `body` too — pass it to `ContentContainer` so it + * stays pinned above that column's scroll region. + * + * @example + * ```tsx + * + * + * }> + * + * + * + * + * } + * /> + * ``` */ - collapsible?: boolean; + body: React.ReactNode; + + /** Place your header inside `body`, via ``. */ + header?: never; + + /** 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; + 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); + const { sidebar, header, children, body, defaultOpen, collapsible } = props; 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 ?? } -
- - + {sidebar ?? } + {body ?? ( + }> + {children ? children({ Outlet: AppShellOutlet }) : } + + )}
); @@ -143,3 +172,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";