feat(sidebar): the app-shell navigation family

[mplatts]

Closes #622

Draft, and deliberately so. #622 is a two-phase brief: the spec gets signed off before the code lands. The spec is below, written as decisions with rationale. The code is pushed alongside it so the decisions are readable as working software rather than prose, but the spec is the thing to review first - if a decision goes the other way, the code follows it.

Summary

Five function components, all CSS-first, no hook, no new dependencies:

Component	What it is
sidebar_shell/1	flex wrapper holding the sidebar beside your page content
sidebar/1	the <nav> landmark, with header / footer slots and the collapse + sheet machinery
sidebar_group/1	a labelled, optionally collapsible run of items
sidebar_item/1	icon + label + badge + active state, or a parent with nested sub-items
sidebar_trigger/1	the rail toggle (target="collapse") or the mobile burger (target="mobile")

Three collapse modes (icon, offcanvas, none), side="left"|"right", multi-sidebar by construction.

---

Phase 1: the spec

1. Composition anatomy

Decision: five function components, with slots collapsing the rest.

shadcn's ~25 sub-components exist because React has no slot primitive. Phoenix does, so:

• SidebarHeader / SidebarFooter → <:header> / <:footer> slots on sidebar/1
• SidebarContent → the default inner_block
• SidebarMenu + SidebarMenuItem + SidebarMenuButton → one sidebar_item/1
• SidebarMenuSub + SidebarMenuSubItem → sidebar_item/1 nested inside sidebar_item/1 (self-recursive, which a slot cannot be - this is why items are a function and not a slot)
• SidebarMenuBadge → the badge attr
• SidebarGroupLabel + SidebarGroupContent → attrs on sidebar_group/1
• SidebarProvider → sidebar_shell/1, which owns nothing but layout and the inert target
• SidebarRail → dropped as a separate component, see deviations

Groups and items are functions rather than slots specifically because sub-items need arbitrary depth. Everything that does not need recursion is a slot, per accordion.ex / navigation_menu.ex house style.

2. The namespace question

Decision: sidebar_* prefix family, exported from use PetalComponents.

Survey of what the names have to dodge:

• lib/petal_components.ex exports nothing starting with sidebar. Menu owns the adjacent vertical_menu / menu_group / vertical_menu_item; no collision.
• Phoenix.Component and phx.new core_components: no sidebar*.
• Downstream is the real risk. Petal Pro and Petal Boilerplate both ship their own sidebar_layout/1 built on vertical_menu, and plenty of hand-rolled apps define one too. An import conflict is a hard compile error - you cannot shadow an import with a local of the same name/arity - so exporting sidebar_layout would break those installs on mix deps.update.

So: the layout wrapper is named sidebar_shell/1, not sidebar_layout/1. That is the single highest-collision name in the family and it is the one we do not take. The other five names are self-prefixed and collide with essentially nothing.

I considered the PetalComponents.Chat treatment (namespaced, opt-in, excluded from use PetalComponents) and rejected it. Chat is excluded because its members have generic names (markdown/1) that no amount of care fixes. The sidebar family is the opposite case: the names are already namespaced by their prefix. Chat is also a niche add-on, whereas the sidebar is the flagship block component - making it the one family that imports differently would be a permanent ergonomics tax to dodge a one-time, loud, one-line-fixable compile error.

One thing I'd like your call on, and did NOT ship: lib/petal_components.ex is defmacro __using__(_) - it ignores opts, so a consumer who does collide has no way to say use PetalComponents, except: [sidebar: 1]. Their only escape is dropping use PetalComponents in that one module and hand-importing. Adding except: / only: passthrough is a ~10 line non-breaking change that would defuse this and every future name collision. It felt like scope creep on a component PR and it touches the library's front door, so it's yours to decide. Happy to add it here or in a separate PR.

3. Collapse behaviour and where state lives

Decision: the DOM owns the live state, the server owns the initial state. No cookie, no hook, no round trip.

• The trigger flips data-collapsed="true"|"false" on the sidebar via JS.toggle_attribute. CSS does everything else. Toggling is instant and costs nothing.
• The initial value is the collapsed attr, rendered server-side into that same attribute.

That second point is the whole answer to "how does a live_redirect not flash the wrong state": there is no client-side read-then-apply step to flash. The attribute arrives correct in the first byte of HTML. shadcn needs the cookie read in middleware for exactly this reason; in LiveView the app already has somewhere to put it.

Escape hatches, in increasing order of durability:

1. Do nothing - state is per-page-load, which is right for a lot of apps.
2. Keep it in an assign and pass collapsed={@sidebar_collapsed}, plus on_click={JS.push("toggle_sidebar")} on the trigger so the server hears about it. Survives live_patch and live_redirect within the LiveView's lifetime.
3. Persist that assign to a session cookie in your own handle_event and read it in mount (or in the plug pipeline for dead renders). Survives everything.

The component ships no persistence plumbing at all, per the issue's non-goals. on_click and on_close are JS attrs composed ahead of the component's own commands (the compose_js/2 house pattern), which is the whole seam.

Non-goal I'm respecting loudly: Cmd+B is not shipped. phx-window-keydown + phx-key matches on key only, with no modifier predicate, so a hook-free Cmd+B would fire on every bare b - including inside text inputs. The issue says to flag a hook for sign-off rather than sneak one in, so: do you want a hook for this? It'd be ~15 lines and the library's fourth. My lean is no for v1; apps that want it can bind it themselves and call PetalComponents.Sidebar.toggle_sidebar/1, which is public for exactly this.

4. Mobile behaviour

Decision: shares slide_over's interaction grammar, does not reuse the module.

slide_over/1 is a modal dialog with a title bar, a close button, role="dialog" and aria-modal. A nav sheet is a navigation landmark. Wrapping the nav in a dialog would either double the landmark or lose it, and slide_over push-pushes a close_slide_over event that the app must handle - which would drag server state into a component that otherwise needs none. So the sheet is built inline, matching the dev.exs mobile menu grammar point for point:

dev.exs mobile menu	<.sidebar>
focus_wrap fence	focus_wrap around the panel
inert on background regions	inert set on #<id>-main by the trigger, removed on close
Escape closes	phx-window-keydown + phx-key="escape"
phx-remove={JS.focus(to: burger)}	JS.focus(to: "#<id>-trigger") in the close command
scrim / click-away	phx-click on .pc-sidebar__scrim

Two mechanics worth flagging in review, because both are slightly clever:

Escape is gated on a selector, not on server state. dev.exs can :if-render the overlay because @nav_open is an assign; here the state is in the DOM. So the keydown runs JS.exec("data-close", to: "#<id>[data-mobile-open='true']") - if the sheet is shut the selector matches nothing and the whole command is a no-op. That's how Escape on desktop avoids yanking focus to the trigger.

focus_wrap's sentinels are CSS-disabled unless the sheet is open. focus_wrap traps Tab whenever it's in the DOM, which on desktop would fence the entire page inside the sidebar. Since it can't be conditionally rendered, this rule turns it off instead:

.pc-sidebar:not([data-mobile-open="true"]) .pc-sidebar__panel > [tabindex="0"][aria-hidden="true"] { display: none; }

display: none removes the sentinels from the tab order, so the trap only exists while the sheet is open. It leans on focus_wrap's rendered shape rather than its ids, but it is leaning on an implementation detail - if you'd rather not, the alternative is making the sheet's open state server-owned like dev.exs, at the cost of every consumer needing a handle_event. Happy to switch.

5. Multi-sidebar support

Decision: supported structurally in v1, documented via the inspector example.

There is no shared/global state to collide over - every id, data attribute and JS target is derived from the sidebar's own id. Two sidebars in one shell with different ids simply cannot cross-wire, and there are tests asserting that for both the sidebars and their triggers. side="right" + collapsible="offcanvas" is the inspector-panel shape, shipped as a showcase example.

The one shared thing is the shell's -main region, which both would mark inert - correct behaviour, since either open sheet should fence the page.

6. Theming surface

Decision: two custom properties on .pc-sidebar.

.pc-sidebar { --pc-sidebar-width: 16rem; --pc-sidebar-icon-width: 3.5rem; }

Both are read by the width rules, so an app retunes by setting them on .pc-sidebar (or per-instance via style), never by overriding pc-* internals.

The breakpoint is not a variable and cannot be. CSS media queries can't read custom properties. It's fixed at md (48rem) to match the rest of the library. Container queries would make it configurable-ish and are a fair follow-up, but they'd change the layout contract, so not in v1.

---

Relationship to menu / navigation_menu / user_dropdown_menu

This is the question I most want checked, since the brief explicitly warns against forking a fourth navigation implementation.

What it does NOT duplicate. navigation_menu is a horizontal top-nav with hover-opened mega-menu panels - different axis, different interaction, no overlap. user_dropdown_menu is a popover of account links; the sidebar <:footer> slot is designed to hold one, and the issue's own playground spec asks for exactly that composition.

Where it genuinely overlaps: vertical_menu. Both render a vertical run of nav links with icons, active state and nested disclosures. The honest position:

• This is not a layout shell around vertical_menu. They have incompatible API shapes. vertical_menu is data-driven (menu_items={[%{name:, label:, path:, icon:, menu_items: [...]}]} + current_page={:atom}, with active state inferred by matching name against current_page). sidebar_item is slot-composed, with active passed explicitly and never inferred. The issue's non-goals rule out a data-driven list API here, and it rules out inferring active state - so sidebar_item could not have been vertical_menu underneath without violating both.
• vertical_menu also can't collapse. Its markup has no seam for an icon rail: the label is a plain <div> with no hook for the collapsed treatment, submenus are toggled by inline style="display:..." set server-side (which fights any CSS-driven collapse), and there's no title/sr-only handling for iconified state. Making it collapsible would be a breaking change to its markup.
• vertical_menu also emits one <nav> per group, which is wrong for an app shell (a shell wants one landmark with one accessible name). <.sidebar> emits exactly one <nav aria-label> and groups are plain <div>s.

So they are two shapes of the same idea, and I did not fork the styling doctrine - pc-sidebar-item follows pc-vertical-menu-item's structure and gray ramp deliberately.

What I did not do, and think is yours to call: the issue says "the spec decides vertical_menu's future, not this build." My read is that <.sidebar> supersedes vertical_menu for app shells, and vertical_menu should stay as the data-driven primitive for apps that want to pass a list (petal_pro's sidebar_layout is built on it and shouldn't be disturbed). I'd deprecate nothing in this PR. Flag if you disagree.

Dogfood target. The dev.exs mobile menu (its comment already says "the sidebar primitive planned for 4.9 replaces this and inherits its grammar") - I've confirmed the component can express it: grouped items, active state, the full sheet grammar, and link_type="button" + phx-value-* globals for the phx-click="select" wiring it uses. I have not done the swap in this PR - it's cross-cutting and would muddy the review of a new component. Happy to ticket it as the immediate follow-up, or do it here if you'd rather see it proven.

---

Deviations from the issue's API sketch

Sketch	Shipped	Why
sidebar_layout or provider wrapper	sidebar_shell/1	sidebar_layout is the one name that would break Petal Pro / Boilerplate installs on import conflict. See §2.
sidebar_rail/1 as its own component	dropped	The rail is a state of the sidebar (collapsible="icon" + collapsed), not a separate element. A sixth exported name to render a click strip is collision surface for no expressive gain - sidebar_trigger already gives you the click target and can be placed in the header, footer or your topbar.
sidebar_trigger takes id (of the sidebar it toggles)	takes for, and id is its own	id meaning "some other element's id" reads wrong and makes focus-restore targeting ambiguous. for is the reference; id defaults to "<for>-trigger" so the close command can restore focus to it. sidebar_shell uses for for the same reason.
sidebar_item link_type default "live_redirect"	same	kept
sidebar_item icon: name / function / raw SVG	same	matches menu.ex's menu_icon convention exactly
single trigger that does both jobs	target="collapse" / target="mobile"	shadcn branches on isMobile in JS. Without a hook there's no server-side breakpoint, so one trigger toggling both states would silently flip the desktop rail while you were on mobile. Two explicitly-targeted triggers (each hidden at the other breakpoint by CSS) is honest and matches how real shells are laid out - burger in the topbar, rail toggle in the sidebar header.
Cmd+B on the trigger	not shipped	needs a hook; flagged for sign-off. See §3.

Also worth noting: #640 (feat/resizable) is an obvious pairing and I deliberately did not touch it. This is built against origin/main only and ships no resize machinery (v1 non-goal). If resizable_group/panel/handle land, a drag-to-resize sidebar should compose them and set --pc-sidebar-width from the panel size rather than growing its own.

---

Styles

New pc-sidebar* section in assets/default.css, appended inside its own @layer components { } block.

That last bit matters and is worth a maintainer's eye: everything after the showcase-props section (line ~7016) currently sits outside any cascade layer, so those rules outrank consumer utilities passed via class. Appending into that tail would have quietly broken <.sidebar class="..."> overrides. The new block re-opens a layer. The pre-existing unlayered carousel section above it is untouched but is probably a latent bug worth its own issue.

Otherwise: --pc-radius consumed via the usual max(calc(...)) derivation, gray ramp for all neutrals, dark: throughout, focus-visible rings only (no persistent :focus fills), and prefers-reduced-motion reducing every transition to instant.

Accessibility

• One <nav> landmark per sidebar with an aria-label (defaults to "Sidebar", overridable)
• aria-current="page" on the active item, at any nesting depth; passed by the app, never inferred
• Collapsible groups and parent items follow the WAI-ARIA disclosure pattern (aria-expanded on the trigger, aria-controls pointing at the panel)
• Collapsed icon rail keeps labels in the accessibility tree via sr-only (not display: none), with title carrying the same text for sighted hover
• Mobile sheet: focus_wrap fence, inert background, Escape, scrim dismiss, focus restored to the trigger
• prefers-reduced-motion: reduce drops all transitions

Hook

None. Collapse, disclosure, the sheet, Escape and focus restoration are all Phoenix.LiveView.JS + CSS. npm test is unchanged at 157 because there is no JS to test. The only thing that wanted a hook was Cmd+B, and it's flagged rather than shipped.

Tests

	before	after
mix test	913 (0 failures, 1 skipped)	972 (0 failures, 1 skipped) - 59 new
npm test	157	157 - unchanged, no hook
mix credo	14 refactoring, 31 readability	identical - zero new entries

mix format --check-formatted and mix compile --force --warnings-as-errors both clean.

Every attr, every value in every values: list, every slot and every link_type has a rendering assertion. ARIA is asserted explicitly rather than via class checks (aria-current, aria-expanded, aria-controls, aria-label, the inert/focus_wrap wiring), and there are tests specifically asserting two sidebars and two triggers do not cross-wire.

Verified by hand in the playground

Light and dark at 1280px, the collapsed icon rail, and the mobile sheet at 390x844 (both closed, confirming the sidebar leaves the flow entirely, and open with the scrim over the content region). Screenshots below.

Three things the browser caught that the test suite could not, all fixed:

1. The mobile sheet never slid in. [data-side="left"] .pc-sidebar__panel { transform: translateX(-100%) } carries a :not() plus two attribute selectors; the open rule was one selector lighter, so it lost the tie and the panel stayed off-screen while the scrim dimmed the page. The open rule now carries matching weight. Pure CSS specificity - invisible to rendered_to_string.
2. Two triggers for one sidebar collided on id. Both defaulted to "<for>-trigger", which is also the focus-restore target. Only target="mobile" claims it now; there's a test for it.
3. A 3.5rem rail clipped a two-item header (brand mark plus toggle). --pc-sidebar-icon-width is 4rem.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.63%. Comparing base (871b2cf) to head (051eb1a).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #645      +/-   ##
==========================================
+ Coverage   92.40%   92.63%   +0.23%     
==========================================
  Files         119      121       +2     
  Lines        5066     5230     +164     
==========================================
+ Hits         4681     4845     +164     
  Misses        385      385              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mplatts]

Screenshots

Collapsed rail — labels drop to sr-only

Mobile off-canvas at 390x844, open and closed

Light / dark

Verified independently

Check	Result
mix test	972 tests, 0 failures, 1 skipped (+59)
npm test	157 passing, unchanged (no hook)
mix format / compile -Werror	clean
mix credo	zero new entries vs main
new dependencies	none
slugs : clauses	61 : 61
CSS inside @layer	confirmed

The sidebar_shell naming call is correct, and I can prove it from Petal Pro

The PR argues sidebar_layout would break Pro/Boilerplate installs on import conflict. I checked the actual dependent repo rather than taking it on trust, and it holds:

• petal_pro defines sidebar_layout/1 at lib/petal_pro_web/components/pro_components/sidebar_layout.ex:76.
• PetalProComponents does import PetalProWeb.SidebarLayout (line 32) in the same macro whose callers also use PetalComponents (petal_pro_web.ex:100).
• So sidebar_layout/1 would have been in scope from both sides — a genuine ambiguity error, not a hypothetical.

Better still, this exact collision has already happened twice, and Pro carries the scar tissue in a comment:

# petal_components 4.13 added `combo_box/1` and `data_table/1`, both of
# which Pro already ships its own version of. Re-importing those two
# modules with `except:` narrows the earlier `use PetalComponents` import
import PetalComponents.ComboBox, except: [combo_box: 1]
import PetalComponents.DataTable, except: [data_table: 1]

sidebar_layout would have been the third. Avoiding it by naming the wrapper sidebar_shell costs nothing and saves a downstream fix.

I also checked the other seven names this PR claims — sidebar, sidebar_group, sidebar_item, sidebar_trigger, toggle_sidebar, show_sidebar, hide_sidebar — against Pro's sidebar_menu, sidebar_menu_item, sidebar_menu_group, sidebar_user_profile, sidebar_auth_cta, sidebar_tabs_container and sidebar_menu_items. No overlap. Boilerplate defines none.

That also strengthens the open question raised in the PR: adding except:/only: passthrough to use PetalComponents would defuse this whole class of problem rather than solving it one name at a time. Given it has now bitten three times, that looks worth its own issue.

Three bugs the browser caught that tests couldn't

The mobile sheet never sliding in (CSS specificity tie against the per-side translateX(-100%)), two triggers both claiming the <for>-trigger focus-restore id, and a 3.5rem rail clipping a two-item header. That's the third PR in this batch where driving a real browser found something the unit tests structurally could not.

Not verified: keyboard-only and prefers-reduced-motion were confirmed by reading the CSS/markup rather than driven, since agent-browser eval is blocked in that sandbox.

_{Images live on the pr-assets branch, which exists only to host PR screenshots - it never merges to main and ships in no Hex release.}

[nhobes]

Phase 1 spec, reconstructed from the build

#622 asked for a written spec and maintainer sign-off before any component code landed. That gate did not happen: the branch shipped 1,969 lines across 8 files first, and the PR description was written afterwards as the spec. This comment exists so you can rule on the design without reading the diff.

It is not a re-run of the author's write-up. For each question #622 required the spec to settle, it states what the build actually chose (read out of the code, not the prose), the evidence, what was foregone, and where the choice looks risky. Where the PR description and the code disagree, the code wins and the disagreement is called out.

Read the risk register first. One item there is a build-breaking regression for petal.build itself, and it is the exact class of failure #622 flagged as critical.

---

Q1. Composition anatomy

Chosen: five function components, all exported flat. Slots absorb the rest.

Component	File	Role
sidebar_shell/1	lib/petal_components/sidebar.ex:115	flex wrapper, renders one content region as <for>-main
sidebar/1	lib/petal_components/sidebar.ex:163	the <nav> landmark plus collapse/sheet machinery
sidebar_group/1	lib/petal_components/sidebar.ex:224	labelled, optionally collapsible run of items
sidebar_item/1	lib/petal_components/sidebar.ex:291 and :308	two clauses: leaf link, or parent disclosure
sidebar_trigger/1	lib/petal_components/sidebar.ex:364	rail toggle or mobile burger, branched by target

Header and footer are slots on sidebar/1 (sidebar.ex:156-158). Content is inner_block. Groups and items are functions rather than slots because sidebar_item nests inside sidebar_item (sidebar.ex:328), and a slot cannot recurse.

Foregone

• shadcn's ~25 sub-component decomposition. Reasonable: Phoenix has slots, React does not.
• sidebar_rail/1 from the issue's API sketch, dropped entirely. The rail is now a state (collapsible="icon" + collapsed), not an element. There is no thin click-strip at the sidebar edge, only whatever sidebar_trigger you place yourself.
• A separate content component. Fine.

Risk

• Dropping the rail means the collapsed sidebar has no edge affordance. shadcn's SidebarRail is how most users discover they can expand it. Every example in this PR puts the toggle in the header or footer instead (lib/petal_components/showcase/sidebar.ex:38-45, :60). Worth deciding deliberately, not by omission.
• sidebar_item/1 clause 1 takes no id, clause 2 mints one. An item that gains children later silently gains a generated id. Harmless, but the id attr doc does not say so.

---

Q2. The namespace question

Chosen: a flat sidebar_* prefix family, all five names added to the blanket use PetalComponents import (lib/petal_components.ex, Sidebar inserted into the import PetalComponents.{...} list).

Foregone

• PetalComponents.Chat treatment (namespaced module, excluded from use). Rejected in the PR body on the grounds that Chat is excluded for having generic names while sidebar_* names are self-prefixed.
• sidebar_layout/1 as the wrapper name. Renamed to sidebar_shell/1 specifically to dodge Petal Pro's sidebar_layout/1. That dodge is real and correct: petal_pro/lib/petal_pro_web/components/pro_components/sidebar_layout.ex:76.

Risk: the survey missed a live collision, and it is the flagship name

The PR body says the layout wrapper "is the single highest-collision name in the family and it is the one we do not take", and that "the other five names are self-prefixed and collide with essentially nothing". That is wrong for the bare sidebar/1.

petal_marketing (petal.build itself) has:

• lib/petal_pro_web/components/main_layout.ex:4 - use PetalComponents
• lib/petal_pro_web/components/main_layout.ex:69 - calls <.sidebar ...>
• lib/petal_pro_web/components/main_layout.ex:387 - def sidebar(assigns) do
• mix.exs:62 - {:petal_components, "~> 4.10"}, so the next 4.x picks this up on mix deps.update

The Elixir semantics matter here, and are worse than "silent shadow". I reproduced both on 1.19.4:

• Local def with no prior call to the imported name: local wins, compile succeeds, warning only. The consumer keeps their own <.sidebar> and never sees the library's.
• Local def after the module has already called the name: hard failure.

error: imported PetalComponents.Sidebar.sidebar/1 conflicts with local function

main_layout.ex is the second case exactly: call at line 69, definition at line 387. So publishing this to Hex breaks the build of the site that sells the library, on a routine dependency bump. Any downstream app-shell module with the same call-above-definition shape breaks identically, and that shape is the norm in layout modules.

Notably the two silent-shadow cases are arguably worse than the loud one: an app that defines sidebar/1 below its call sites gets a clean compile and a component that quietly is not the library's.

Petal Pro is clear on all five names (sidebar_menu/1, sidebar_menu_group/1, sidebar_menu_item/1, sidebar_user_profile/1, sidebar_auth_cta/1, sidebar_tabs_container/1, sidebar_layout/1 - none collide). Boilerplate is clear. phx.new core_components ships no sidebar*. The collision is specifically the ungeneric-enough sidebar/1.

Options, cheapest first

1. Rename sidebar/1. It is the only unprefixed name in a family that is otherwise self-namespaced, so sidebar_nav/1 or sidebar_panel/1 restores the property the PR claims the family already has. Costs one rename across the module, tests, showcase, dev.exs, CHANGELOG.
2. Ship the family as PetalComponents.Sidebar, excluded from use PetalComponents, per the Chat precedent. The PR's argument against this (Chat is niche, sidebar is flagship) cuts both ways: flagship means more downstream apps already have one.
3. Add except:/only: passthrough to use PetalComponents (currently defmacro __using__(_) ignores its opts entirely) and accept a loud break for consumers who collide. This is the author's own flagged suggestion. It does not fix petal_marketing without also editing petal_marketing, and it does nothing for the silent-shadow case.
4. Ship as-is and fix petal_marketing separately. Then every downstream app with a hand-rolled sidebar gets the same surprise, which is what Component: Sidebar #622 wrote this question to prevent.

This is the one decision that is expensive to reverse after a Hex publish.

---

Q3. Collapse behaviour and where state lives

Chosen: DOM owns live state, server owns initial state, no persistence shipped.

• data-collapsed on .pc-sidebar, rendered server-side from the collapsed attr (sidebar.ex:170, attr at :145).
• The collapse trigger flips it with JS.toggle_attribute (toggle_sidebar/2, sidebar.ex:408). CSS reads it (assets/default.css:7776-7815).
• Three modes icon / offcanvas / none (sidebar.ex:139), all CSS, no hook.
• Escape hatches documented in the moduledoc (sidebar.ex:49-65): assign + on_click={JS.push(...)}, or your own cookie.

The "no flash on live_redirect" requirement is met by construction: nothing is read back from the client, so there is nothing to flash.

Foregone

• Cookie persistence (shadcn's approach). Explicitly not shipped, consistent with the issue's non-goals.
• A Cmd+B hook. Not shipped, flagged for your call. phx-window-keydown matches on key only with no modifier predicate, so hook-free Cmd+B would fire on bare b, including inside inputs. toggle_sidebar/1 is public so apps can bind it themselves.

Risk

• Client-only state means the collapse choice dies on full page load. The PR is honest about this and the playground copy says so (dev.exs:~6292). It is the right v1 default, but it is a product decision, not a technical one, and it is yours.
• The offcanvas collapse trigger carries no aria-expanded (sidebar.ex:383 gates it to target == "mobile"), even though in offcanvas mode it fully shows and hides the nav. That is a disclosure without disclosure semantics.

---

Q4. Mobile behaviour

Chosen: shares slide_over's interaction grammar, does not reuse the module. Sheet built inline.

• focus_wrap around the panel (sidebar.ex:184)
• inert on the shell's content region, set by show_sidebar/2 and cleared by hide_sidebar/2 (sidebar.ex:427, :443)
• Escape via phx-window-keydown (sidebar.ex:173)
• Scrim click-away (sidebar.ex:177-182)
• Focus restored to #<id>-trigger (sidebar.ex:445)
• Breakpoint hard-coded at md / 48rem (assets/default.css:7776, :7819)

This matches the dev.exs mobile menu grammar the issue named as the reference, point for point.

Foregone

• Reusing slide_over/1. Sound: it is role="dialog" + aria-modal, which would either double or destroy the nav landmark, and it pushes a close_slide_over event the app must handle.

Two mechanics that need your eyes because both lean on implementation details

1. Escape is gated by selector, not state. JS.exec("data-close", to: "##{@id}[data-mobile-open='true']") (sidebar.ex:173). If the sheet is shut the selector matches nothing and the command no-ops. Clever and correct, but it means a global keydown listener is live on every page carrying a sidebar.
2. focus_wrap's sentinels are disabled with CSS, not conditional rendering:

   .pc-sidebar:not([data-mobile-open="true"]) .pc-sidebar__panel > [tabindex="0"][aria-hidden="true"] { display: none; }

   (assets/default.css:7642). This depends on focus_wrap's rendered shape, which is a LiveView internal. If Phoenix changes it, the desktop sidebar starts trapping Tab for the whole page and no test in this repo catches it. The alternative is server-owned sheet state, at the cost of a handle_event in every consumer.

Also risky

• .pc-sidebar__scrim uses bg-gray-950/50 with no dark variant (assets/default.css:7852), where modal and slide_over use bg-black/50 dark:bg-black/60. Dark-mode sheet separation will read differently from every other overlay in the library.
• The #<id>-trigger id is a hard contract in three places (sidebar.ex:427, :443, :445), and sidebar_trigger only claims it when target="mobile" (sidebar.ex:369-374). A consumer who passes a custom id to the mobile trigger silently loses focus restore and aria-expanded sync, with no doc warning on the attr.

---

Q5. Multi-sidebar support

Claimed: supported structurally in v1. side="left"|"right", independent ids, no shared state.

Actually: the claim is false as coded for two sidebars in one shell.

• sidebar_shell/1 renders exactly one content region, id derived from its own for attr: assign(assigns, :main_id, "#{for}-main") (sidebar.ex:116, rendered at :121).
• show_sidebar/2 and hide_sidebar/2 derive the inert target from the sidebar's id: to: "##{id}-main" (sidebar.ex:427, :443).

These agree only when the shell's for equals the sidebar's id, which is true for exactly one sidebar per shell. Put a second sidebar in the same shell and its sheet targets #<second-id>-main, an element that does not exist. The sheet opens over content that stays fully interactive and keyboard-reachable behind it.

The claim is stated in three shipped places: sidebar.ex:137 (the side attr doc, which ships to HexDocs and the MCP schema), lib/petal_components/showcase/sidebar.ex:130 (the inspector example description), and the CHANGELOG entry.

The test that appears to cover this does not: test/petal/sidebar_test.exs:211 renders two sidebars in a bare <div>, not in a shared sidebar_shell, so each one's derived -main is trivially distinct and the assertion passes. The inspector showcase example (showcase/sidebar.ex:128) contains one sidebar, so the two-sidebar composition it describes is never rendered anywhere in the PR.

Two ways out

1. Add attr :main_id to sidebar/1, defaulting to "#{id}-main", thread it through show_sidebar/hide_sidebar, and add a test with two sidebars inside one real shell asserting both target the shell's actual main id.
2. Narrow the spec to one sidebar per shell, and correct the attr doc, the showcase description and the CHANGELOG to match.

Either is fine. Shipping the sentence without the behaviour is not, because it goes to HexDocs and the MCP schema.

---

Q6. Theming surface

Chosen: two custom properties on .pc-sidebar.

.pc-sidebar { --pc-sidebar-width: 16rem; --pc-sidebar-icon-width: 4rem; }

(assets/default.css:7612, :7615.) Both feed the width rules, so apps retune by setting them on .pc-sidebar or per instance via style, never by overriding pc-* internals.

Foregone

• A breakpoint variable. Correctly ruled out: media queries cannot read custom properties. Fixed at md. Container queries are named as a follow-up that would change the layout contract.

Worth your eye

• The new CSS is appended in its own re-opened @layer components { } block (assets/default.css:7602-7866) because everything after the showcase-props section sits outside any cascade layer, and unlayered rules outrank consumer utilities passed via class. That is the right call here and the comment explains it. It also means the pre-existing unlayered tail (the carousel section above) is a latent bug in main that this PR documents but does not fix. It should get its own issue.

---

Beyond the six questions

vertical_menu's future

#622 said the spec decides this. The build's position: <.sidebar> supersedes vertical_menu for app shells, vertical_menu stays as the data-driven primitive, nothing is deprecated in this PR. The argument is solid: vertical_menu is data-driven with inferred active state, sidebar_item is slot-composed with explicit active, and the issue's non-goals rule out both of vertical_menu's defining traits. vertical_menu also emits one <nav> per group, which is wrong for a shell.

This still needs your ruling, because it is now two overlapping vertical-nav components in one library, forever.

Definition-of-done items not met

• Footer user menu. Component: Sidebar #622 required the playground example to compose user_dropdown_menu or dropdown in the footer. Both the playground (dev.exs:~6212) and the showcase (showcase/sidebar.ex:38-45) put a plain sidebar_item there instead. This matters beyond checklist compliance: .pc-sidebar__panel is overflow-hidden (assets/default.css:7628), so a non-portalled popup in the footer will likely be clipped. The composition the issue asked for is the one that would have found this.
• dev.exs dogfood swap. Confirmed expressible, not done, not ticketed.
• Phase 1 sign-off. The gate this comment exists to close.

Author-escalated decisions still open

1. Cmd+B hook: yes or no. Author's lean is no for v1.
2. except:/only: passthrough on use PetalComponents (currently defmacro __using__(_) discards opts). ~10 lines, non-breaking, touches the library's front door.
3. vertical_menu's future, above.

---

Risk register

#	Risk	Severity	Evidence
1	sidebar/1 collides with downstream locals. petal.build's own main_layout.ex gets a hard CompileError on the next dep bump	Blocking	main_layout.ex:4,69,387; mix.exs:62
2	Multi-sidebar inert target is broken; the claim ships to HexDocs and the MCP schema	Blocking	sidebar.ex:116 vs :427,443; sidebar.ex:137; showcase/sidebar.ex:130
3	Footer user-menu composition never built, hiding a likely clip under overflow-hidden	High	assets/default.css:7628; showcase/sidebar.ex:38-45
4	Icon rail leaves an invisible tab stop on .pc-sidebar-group__toggle, and parent items keep aria-expanded on panels CSS permanently hides	High (a11y)	assets/default.css:7795-7804; sidebar.ex:317,328
5	focus_wrap sentinel suppression depends on a LiveView internal, untested	Medium	assets/default.css:7642
6	#<id>-trigger is an undocumented hard contract; a custom mobile trigger id silently breaks focus restore	Medium	sidebar.ex:337,427,443,445
7	Scrim is bg-gray-950/50 with no dark variant, unlike every other overlay	Low	assets/default.css:7852
8	No rail affordance; the toggle must be placed by hand	Low, by design	sidebar_rail dropped
9	CHANGELOG ### Unreleased will conflict with sibling component PRs	Housekeeping	CHANGELOG.md:2

---

Approve / adjust

Answer inline. Everything below is a design call, not a code review item.

Namespace (must answer before merge)

• Keep sidebar/1 as-is and fix petal_marketing separately
• Rename sidebar/1 (suggest sidebar_nav/1), keep the rest flat
• Move the whole family to opt-in PetalComponents.Sidebar, Chat-style
• Add except:/only: to use PetalComponents (here, or a separate PR)

Multi-sidebar

• Fix it: add main_id to sidebar/1 and test two sidebars in one real shell
• Scope it out: one sidebar per shell, correct the attr doc, showcase text and CHANGELOG

Anatomy

• Five components with slots is right
• Bring back sidebar_rail/1 as an edge affordance
• Something else:

Collapse state

• Client-only default with documented escape hatches is right for v1
• Want cookie persistence in v1 after all

Mobile

• Inline sheet sharing slide_over's grammar is right
• The CSS-disabled focus_wrap sentinels are acceptable
• Prefer server-owned sheet state instead

Open calls

• Cmd+B hook: yes / no
• vertical_menu: leave as-is / soft-deprecate / deprecate
• Footer user-menu example: build it here / follow-up issue
• dev.exs mobile-menu dogfood swap: here / follow-up issue
• Unlayered default.css tail: open an issue

Theming

• Two custom properties and a fixed md breakpoint is the right v1 surface

Once the namespace and multi-sidebar rows are answered, everything else in the risk register is bounded polish and the branch can come out of draft.