feat(core): add SidebarLayout body slot for custom page layouts - #437
Draft
interacsean wants to merge 4 commits into
Draft
feat(core): add SidebarLayout body slot for custom page layouts#437interacsean wants to merge 4 commits into
interacsean wants to merge 4 commits into
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
… by type (#1643) `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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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.
What
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.
Addresses tailor-inc/platform-planning#1643, which merges two field reports with one root cause:
Both were working around the same wall: the only handle on the content region is the
childrenrender prop, which renders inside the padded, scrolling, header-topped inset. So consumers reached for:has()+!importantoverrides on[data-slot="sidebar-inset"]/[data-slot="sidebar-wrapper"], plus aMutationObserveron[data-state]and a hidden-trigger.click()to track sidebar collapse.API
bodyreplaces the region beside the sidebar. As a flex sibling it reflows on collapse for free. Supersedesheader/children; warns in dev when combined.SidebarLayout.ContentContainer— the stock content column (inset padding, pinned header slot, scroll region), extracted. It now owns the scroll ref, souseAppShellScrollContainer()keeps working inside a custom body. The default path renders the same component — one implementation, not two.SidebarLayout.Outlet/SidebarLayout.Trigger— page outlet and collapse toggle, for composing custom bodies.useAppShellSidebar()—{ open, isMobile, setOpen, toggle }, inert outside aSidebarLayout. Replaces theMutationObserver+ hidden-.click()workaround.SidebarInsetwidth changew-[calc(100%-var(--sidebar-width))]→min-w-0.That calc hardcoded "exactly one sibling, exactly
--sidebar-widthwide" — the assumptionbodybreaks. It dates to the initial commit's scaffold (carried through the file split untouched, no comment, no test) and was really a workaround for a missingmin-w-0:SidebarInsetisw-full flex-1with nomin-w-0, so its flexmin-width: autoresolved againstw-fulland overflowed the row by the sidebar's width.min-w-0is the flex idiom already used five times elsewhere in that file.Verified on
/dashboard/productsat 1280px:min-width: 0px, inset exactly 1024 = 1280 − 256, zero overflow on document, wrapper, or row.Verified
md:px-8paddinguseAppShellSidebar()useAppShellScrollContainer()resolves inside a custom bodybody) all unchangedStill to come
Pilot scope — docs, changeset, and tests for the new surface are not in this PR yet.
Open design question:
bodyis configured at app level, but both reported use cases are page-level. The vite example drives it offuseLocation(), which works but means shell config has to know page routes. Options written up indecisions/page-level-body-columns.md.