Repository navigation
feat(SideNav): collapsible.collapsedWidth takes 'rail' (default) or 0, which hides the collapsed nav - #10276
feat(SideNav): collapsible.collapsedWidth takes 'rail' (default) or 0, which hides the collapsed nav#10276cixzhang wants to merge 2 commits into
Conversation
…, which hides the collapsed nav 'rail' keeps today's icon rail. 0 hides the collapsed nav completely: it takes no width and is inert, it slides out by the rail's existing edge reveal, and focus inside it moves to the SideNavCollapseButton rendered outside the nav with the same controlled config. The outside button registers with that shared state so the nav can find it. The SideNav record gains FR8, a Public concepts row and a verification row. Rebuilt from AKnassa's #3890 on current main, narrowed to the owner's two values. Co-authored-by: AK <144495202+AKnassa@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
|
/approve-spec ffa538b |
github-actions
Bot
added
needs:spec-owner-review
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
…out in a row Co-authored-by: AK <144495202+AKnassa@users.noreply.github.com>
|
/approve-spec 0d52bef |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested. With collapsedWidth: 0 the collapsed nav takes no width, is inert and hands focus to the outside toggle, and the default rail is unchanged; three fixes remain before approval:
- In
docs/specs/components/SideNav/SideNav.spec.mdline 49, escape the pipe in'rail' | 0as\|, because GitHub-flavoured Markdown splits the table cell there and the rendered Public contract row loses its FR8 clause and its "omitted props behave as before" promise. - In the same record, name FR8 in the Compatibility and migration bullet (lines 72-75), which still says nothing but FR7 changes the public API, accepted input or accessibility semantics, and make AR3 (lines 168-179) defer to FR8 for a nav collapsed to
0. - Under
AppShell variant="section", a nav collapsed to0still leaves the side-nav panel's 1 px divider at the page's inline-start edge, so drop that divider while the nav is hidden and cover the case in the story or a test.
Non-blocking suggestions sit inline: a development warning when a nav collapses to 0 and finds no outside toggle, FR8 citing FR6's motion and its implementation gap, and a few comment trims.
[Automated review]
| | Area | Contract | | ||
| | ----------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | Public contract | `resizable` also takes useResizable's output, beside `true` and its config object (FR7); no other prop, value, or theme target changes, and omitted props behave as before. | | ||
| | Public contract | `resizable` also takes useResizable's output, beside `true` and its config object (FR7); `SideNavCollapsibleConfig.collapsedWidth?: 'rail' | 0 = 'rail'`, inherited by `SideNavControlledCollapsible`, sets the collapsed width (FR8); no other prop, value, or theme target changes, and omitted props behave as before. | |
There was a problem hiding this comment.
The pipe inside this code span splits the table cell under GitHub-flavoured Markdown, so the rendered Public contract row ends at collapsedWidth?: 'rail' and drops the rest of the clause: the FR8 field and "omitted props behave as before". Escape it as 'rail' \| 0, as Carousel.spec.md does for boolean \| SnapPaging.
| | FR5 | An expanded nested group MUST NOT clip its rows once it has finished opening, so a nested row's focus ring paints in full. The group MAY clip while collapsed and while it opens or closes. | `component:SideNav/DEC-1` | Settled intent; implementation may be staged | | ||
| | FR6 | Collapsing or expanding the rail plays one motion on one clock, `spec:AST-105` FR4's one shared motion, the theme's curve tokens, never a spring: layout snaps to the final rail width at once, with no width animation, and the rail reveals or covers its labels by a clip, its edge riding the main content's glide and its labels and collapsed icons fading on the same clock (`spec:AST-105` FR9's inline two-state form; the clip and the ride are the shared edge reveal's, and SideNav's own code keeps only the fades; the rail lends no pane to its resize controller, so a collapse plays once; implementation gap: the clip and the ride still run in SideNav's own rail motion code, not in the shared edge reveal): a collapse ends in the real collapsed state, icons only, and an expand ends in the real expanded state, with its labels. On a collapse or an expand the rail's old state is one transient copy, inert and hidden from assistive technology, removed when the motion ends. On a collapse, the transient copy paints at the rail's own level, just below the rail and with no `z-index` of its own (`spec:AST-027` FR1, FR6); page content positioned above the nav also covers the leaving copy. The main content moves on the same clock, as `component:AppShell` FR17 states. A change made before the rail first paints lands at once, unless the rail's own toggle made it, so the initial state never animates. Under reduced motion the rail follows `design:reduced-motion` DR2 under a coarse primary pointer and DR3 otherwise. Collapsing or expanding keeps the nav and its items mounted, a `resizable` rail included, so item state such as an open nested group, and focus, persist (AR3 governs focus on what the collapsed rail does not render). | `component:SideNav/DEC-2`; `component:AppShell` FR17; `spec:AST-105` FR8, FR9 | Verified by `SideNavRailMotion.test.tsx` and `SideNavRailMotion.interaction.browser.spec.ts`; the `reducedMotion: 'always'` theme case pending | | ||
| | FR7 | `resizable` also takes useResizable's output, beside `true` and its config object (owner decision, 2026-11-04); the region's `autoSaveId` then saves the width. When `resizable` is the hook's output and `collapsible` sets its own collapsed state, the region wins: the rail follows the region's `isCollapsed`, the toggle calls its `collapse` and `expand`, and SideNav warns in development, as `resizable`'s keys already override `collapsible`'s. When `resizable` is the hook's output, the region's own `collapsible` option also decides whether the rail collapses; a collapsible prop that disagrees is ignored, with the same warning. | `component:Resizable` FR9 | Verified by `resizableSlot.test.tsx` | | ||
| | FR8 | `collapsible.collapsedWidth` sets the collapsed nav's width: `'rail'` (default) keeps the icon rail; `0` hides the nav completely: it takes no width, its contents are inert, focus inside it moves to the outside toggle (`SideNavCollapseButton`) when it collapses, and it slides by `spec:AST-105` FR9's edge reveal. No other values are accepted. (Owner decision, Nov 6 2026 (Cindy).) | Owner decision, Nov 6 2026 (Cindy); `spec:AST-105` FR9 | Verified by `SideNav.test.tsx` `collapsedWidth` cases and real-browser collapse and expand | |
There was a problem hiding this comment.
The rest of the record does not yet reflect FR8: Compatibility and migration (lines 72-75) still says nothing but FR7 changes the public API, accepted input or accessibility semantics, and AR3 still sends focus to the collapse toggle inside the SideNav. Please name FR8 in that bullet and have AR3 defer to FR8 for a nav collapsed to 0.
| width: spacingVars['--spacing-12'], | ||
| }, | ||
| // `collapsedWidth: 0`: the collapsed nav takes no width. | ||
| rootHidden: { |
There was a problem hiding this comment.
Under AppShell variant="section" this nav goes to 0 px, but AppShell's side-nav panel keeps hasDivider, so a 1 px rule stays at the page's inline-start edge. Please drop the panel's divider while the nav is hidden (for example through SideNavShellContext) and cover the section variant in the story or a test.
| rootCollapsed: { | ||
| width: spacingVars['--spacing-12'], | ||
| }, | ||
| // `collapsedWidth: 0`: the collapsed nav takes no width. |
There was a problem hiding this comment.
Non-blocking (comment style): this restates width: 0 on the next lines, so it can go.
| // A hidden nav's contents take no focus and no pointer; a caller's own | ||
| // `inert` still applies. |
There was a problem hiding this comment.
Non-blocking (comment style): the first clause restates what inert does. Suggest keeping only the constraint:
// A caller's own `inert` still applies.| const sharesState = | ||
| (association.onCollapsedChange != null && | ||
| association.onCollapsedChange === other.onCollapsedChange) || | ||
| (association.handleRef != null && | ||
| association.handleRef === other.handleRef); |
There was a problem hiding this comment.
Non-blocking: matching by function identity means an outside button given an equivalent but different onCollapsedChange (two inline arrows, say) is never found, and focus falls to the page with no signal. A development warning when a nav collapses to 0 with focus inside and no registered toggle would surface that wiring mistake.
|
(Posted by Cindy's agent.) Superseded by #10312: the owner moved full hiding from SideNav to the shell, as AppShell |
Rebuild of #3890 by AKnassa on current
main, narrowed to the owner's two values. AKnassa is co-author of the commit and credited in the changeset.User impact
An app with a focused single-pane layout, such as chat, can now collapse its SideNav to nothing instead of to the icon rail. With
collapsible={{…, collapsedWidth: 0}}, the collapsed nav takes no width, so the content gets the whole row. A keyboard user can't Tab into links they can't see, because the hidden nav is inert. If focus was inside the nav when it collapses, focus moves to the SideNavCollapseButton the app renders outside the nav, not to the page. Apps that don't setcollapsedWidthkeep the icon rail exactly as before.Public delta
SideNavCollapsibleConfig.collapsedWidth?: 'rail' | 0, default'rail', inherited bySideNavControlledCollapsible. No other value is accepted, and no existing field is renamed or deprecated. There is noisAnimatedprop: the hidden nav slides by the rail's existing edge reveal (spec:AST-105FR9).Change
SideNav.tsx: a collapsed nav withcollapsedWidth: 0takes width 0 and isinert(a caller's owninertstill applies). When it collapses with focus inside, focus moves to the outside toggle through the sharedmoveFocusLeftBehindpath. The rail's existing collapse motion (the two-state edge reveal) plays unchanged.SideNavCollapseButton.tsxandSideNavCollapseContext.ts: a button rendered outside the nav registers with the collapse state it shares with that nav (the same controlledonCollapsedChange, or the deprecatedhandleRef), so the nav finds its own toggle and never another nav's. The registry is internal.SideNav.spec.md: FR8 (owner decision, Nov 6 2026), a Public concepts row, and a verification row.Core/SideNav"Collapsed width 0", docs, changeset.Test plan
SideNav.test.tsxcollapsedWidthcases: onmain's source, "makes a nav collapsed to 0 inert…" and "moves focus inside a nav collapsing to 0 to the toggle outside it" fail, and "keeps the icon rail by default…" passes. All three pass here.tscare clean, andpnpm check:knowledgeandpnpm check:changesetspass. The api-report entry is edited by hand; CI's api-report check confirms it.Writing rubric results
R1 narration: none; FR8 and the new rows state the behavior in the present tense.
R2 research: none in the record.
R3 evidence: FR8 cites the owner decision and
spec:AST-105FR9; its verification row names the tests and story, with no pull request.R4 rejected alternatives: none in the record.
R5 change references: none.
R6 open questions: none.