Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions decisions/page-level-body-columns.md
Original file line number Diff line number Diff line change
@@ -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 `<SidebarLayout>` 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" && <TocRail />}
<SidebarLayout.ContentContainer header={<SidebarLayout.DefaultHeader />}>
<SidebarLayout.Outlet />
</SidebarLayout.ContentContainer>
</>
);
};
```

- **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 `<SidePanel>` anywhere in its own tree; it portals into the dock.

```tsx
export default function ManualsPage() {
const [selected, setSelected] = useState(null);
return (
<>
<SidePanel side="left" width={280}>
<TocTree selected={selected} onSelect={setSelected} />
</SidePanel>
<Layout>…</Layout>
</>
);
}
```

- **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.
26 changes: 8 additions & 18 deletions examples/vite-app/src/App.tsx
Original file line number Diff line number Diff line change
@@ -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[] = [
Expand All @@ -29,21 +27,12 @@ const App = () => {
return (
<AppShell title="File-Based Routing Demo" searchSources={searchSources}>
<SidebarLayout
header={
<SidebarLayout.DefaultHeader
actions={[
<Button key="notifications" variant="outline" size="icon" aria-label="Notifications">
<BellIcon />
</Button>,
<Button key="account" variant="outline" size="icon" aria-label="Account">
<CircleUserIcon />
</Button>,
// Opt back into the appearance switcher — `actions` replaces the
// default right-hand cluster, so include it explicitly to keep it.
<AppearanceSwitcher key="appearance" />,
]}
/>
}
// `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={<PanelsBody />}
sidebar={
<SidebarLayout.DefaultSidebar>
<SidebarItem to="/" />
Expand All @@ -52,6 +41,7 @@ const App = () => {
<SidebarItem to="/dashboard/orders" />
<SidebarItem to="/dashboard/products" />
<SidebarItem to="/dashboard/document-progress" />
<SidebarItem to="/dashboard/panels" />
</SidebarGroup>
<SidebarItem to="/date-picker" />
<SidebarItem to="/data-table" />
Expand Down
70 changes: 70 additions & 0 deletions examples/vite-app/src/pages/dashboard/panels/page.tsx
Original file line number Diff line number Diff line change
@@ -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<number | null>(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 (
<Layout>
<Layout.Header title="Body slot demo" />
<Layout.Column>
<p className="rounded-md border bg-card p-3 text-sm">
<strong>useAppShellScrollContainer():</strong>{" "}
{scrollTop === null
? "not resolved ✗"
: `resolved ✓ — scrollTop ${Math.round(scrollTop)}`}
</p>
{PANEL_SECTIONS.map((section) => (
<section key={section} id={sectionId(section)} className="scroll-mt-4">
<h2 className="text-lg font-semibold tracking-tight">{section}</h2>
<p className="mt-2 text-sm text-muted-foreground">
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.
</p>
<p className="mt-2 text-sm text-muted-foreground">
Collapse the sidebar (⌘B) and every column reflows — they are flex siblings of the
sidebar, so nothing is pinned to a hardcoded width.
</p>
</section>
))}
</Layout.Column>
</Layout>
);
};

PanelsPage.appShellPageProps = {
meta: {
title: "Body Slot",
icon: <Columns3 />,
},
} satisfies AppShellPageProps;

export default PanelsPage;
11 changes: 11 additions & 0 deletions examples/vite-app/src/panel-sections.ts
Original file line number Diff line number Diff line change
@@ -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, "-")}`;
120 changes: 120 additions & 0 deletions examples/vite-app/src/panels-body.tsx
Original file line number Diff line number Diff line change
@@ -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 = (
<SidebarLayout.DefaultHeader
actions={[
<Button key="notifications" variant="outline" size="icon" aria-label="Notifications">
<BellIcon />
</Button>,
<Button key="account" variant="outline" size="icon" aria-label="Account">
<CircleUserIcon />
</Button>,
<AppearanceSwitcher key="appearance" />,
]}
/>
);

/**
* 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 (
<aside className="hidden w-64 shrink-0 flex-col overflow-y-auto border-r bg-background md:flex">
<div className="flex h-14 shrink-0 items-center justify-between gap-2 px-4">
<span className="text-sm font-semibold">Contents</span>
<Button variant="ghost" size="sm" onClick={toggle}>
{open ? "Hide nav" : "Show nav"}
</Button>
</div>
<nav className="flex flex-col gap-0.5 px-2 pb-4">
{PANEL_SECTIONS.map((section) => (
<a
key={section}
href={`#${sectionId(section)}`}
className="rounded-md px-2 py-1.5 text-sm text-muted-foreground hover:bg-accent hover:text-accent-foreground"
>
{section}
</a>
))}
</nav>
</aside>
);
};

/**
* 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 }) => (
<aside className="hidden w-96 shrink-0 flex-col overflow-y-auto border-l bg-background md:flex">
<div className="flex h-14 shrink-0 items-center justify-between gap-2 border-b px-4">
<span className="text-sm font-semibold">Assistant</span>
<Button variant="ghost" size="sm" onClick={onClose}>
Close
</Button>
</div>
<div className="flex flex-col gap-3 p-4">
<div className="rounded-lg bg-muted p-3 text-sm">
This panel is flush against the viewport edge — no `:has()` overrides, no `!important`, no
injected global CSS.
</div>
<div className="rounded-lg border p-3 text-sm text-muted-foreground">
The content column to the left still has its normal breadcrumb header and `md:px-8` padding,
and still scrolls on its own.
</div>
</div>
</aside>
);

/**
* 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 && <TocRail />}
<SidebarLayout.ContentContainer
header={
onPanelsPage ? (
<div className="flex items-center gap-2">
<div className="min-w-0 flex-1">{Header}</div>
<Button
variant="outline"
size="icon"
aria-label="Toggle assistant"
onClick={() => setAssistantOpen((prev) => !prev)}
>
<PanelRightIcon />
</Button>
</div>
) : (
Header
)
}
>
<SidebarLayout.Outlet />
</SidebarLayout.ContentContainer>
{onPanelsPage && assistantOpen && <AssistantPanel onClose={() => setAssistantOpen(false)} />}
</>
);
};
1 change: 1 addition & 0 deletions examples/vite-app/src/routes.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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": {};
Expand Down
8 changes: 7 additions & 1 deletion packages/core/src/components/sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,12 @@ function SidebarInset({ className, ...props }: React.ComponentProps<"main">) {
<main
data-slot="sidebar-inset"
className={cn(
"astw:bg-background astw:relative astw:flex astw:min-h-0 astw:w-full astw:flex-1 astw:flex-col",
// min-w-0 (not a width calc): as a flex item its `min-width: auto`
// would otherwise resolve against `w-full`, flooring it at the full row
// width and overflowing the row by the sidebar's width. min-w-0 lets it
// shrink to whatever space is left — which is what makes the `body`
// slot able to place sibling columns beside it.
"astw:bg-background astw:relative astw:flex astw:min-h-0 astw:min-w-0 astw:w-full astw:flex-1 astw:flex-col",
"astw:px-4 astw:md:peer-data-[variant=inset]:px-8 astw:md:peer-data-[variant=inset]:py-2", // astw:md:peer-data-[variant=inset]:peer-data-[state=collapsed]:pl-2
className,
)}
Expand Down Expand Up @@ -772,6 +777,7 @@ function SidebarMenuSubButton({
}

export {
SidebarContext,
Sidebar,
SidebarContent,
SidebarFooter,
Expand Down
Loading
Loading