fix(sidebar): smooth collapse transitions, mobile rewrite, and new props

[hkfoster]

Summary

Incremental improvements to the sidebar component on top of the v2.4.0 modernization.

New features

• mobileBreakpoint prop on Provider — configurable viewport width for mobile detection
• contentClassName prop on Sidebar root — pass-through class for the inner content container
• Controlled mobile state — open prop now controls the mobile sidebar too, not just desktop

Fixes

• Replaced Base UI Dialog mobile sidebar with a plain <aside> + backdrop for simpler, more predictable transitions
• Collapsible sections now animate closed smoothly when the sidebar collapses instead of snapping shut
• Removed hidden class from Sidebar.MenuSub so sub-menus participate in collapse animations
• Removed inertValue React-version helper — SidebarSlidingView now sets inert imperatively for React 18 compatibility
• Removed inert and data-open attributes from SidebarCollapsibleContent

Styling

• bg-kumo-tint → bg-(--sidebar-active-bg) CSS variable for active/hover/focus backgrounds

• Icon opacity 0.5 → 0.4; chevron gains hover opacity transition

• Header gains shrink-0 and animated padding on collapse

• Content scroll area gains animated gap transition and tabIndex={-1} on viewport

• Sliding views container gains max-w-(--sidebar-width) to prevent overflow

• Mobile sidebar uses --sidebar-animation-duration CSS variable for slide transition

• Reviews

• bonk has reviewed the change

• automated review not possible because: compound component with cross-cutting animation/a11y/layout concerns

• Tests

• Tests included/updated

[pkg-pr-new]

npm i https://pkg.pr.new/@cloudflare/kumo@567

commit: 0b2e3e8

[github-actions]

Docs Preview

View docs preview

Commit: 0b2e3e8

[github-actions]

Visual Regression Report — 29 changed, 10 unchanged

29 screenshot(s) with visual changes:

Button / Basic

90 px (0.09%) changed

Before	After	Diff

Button / Variant: Primary

166 px (0.16%) changed

Before	After	Diff

Button / Variant: Secondary

306 px (0.3%) changed

Before	After	Diff

Button / Variant: Ghost

101 px (0.1%) changed

Before	After	Diff

Button / Variant: Destructive

585 px (0.58%) changed

Before	After	Diff

Button / With Icon

422 px (0.42%) changed

Before	After	Diff

Button / Icon Only

32 px (0.03%) changed

Before	After	Diff

Button / Loading State

4 px (0%) changed

Before	After	Diff

Button / Disabled State

117 px (0.12%) changed

Before	After	Diff

Button / Title

433 px (0.43%) changed

Before	After	Diff

Dialog / Dialog With Actions

200 px (0.2%) changed

Before	After	Diff

Dialog / Dialog Basic

114 px (0.11%) changed

Before	After	Diff

Dialog / Dialog Alert

1,707 px (1.68%) changed

Before	After	Diff

Dialog / Dialog Confirmation

1,097 px (1.08%) changed

Before	After	Diff

Select / Select Basic

300 px (0.3%) changed

Before	After	Diff

Select / Select Sizes

892 px (0.48%) changed

Before	After	Diff

Select / Select Without Label

105 px (0.1%) changed

Before	After	Diff

Select / Select With Description

889 px (0.75%) changed

Before	After	Diff

Select / Select Placeholder

672 px (0.66%) changed

Before	After	Diff

Select / Select With Tooltip

294 px (0.29%) changed

Before	After	Diff

Select / Select Custom Rendering

576 px (0.57%) changed

Before	After	Diff

Select / Select Loading

0 px (0%) changed

Before	After	Diff

Select / Select Multiple

556 px (0.55%) changed

Before	After	Diff

Select / Select Complex

1,305 px (1.1%) changed

Before	After	Diff

Select / Select Disabled Options

829 px (0.82%) changed

Before	After	Diff

Select / Select Disabled Items

224 px (0.22%) changed

Before	After	Diff

Select / Select Grouped With Disabled

450 px (0.44%) changed

Before	After	Diff

Select / Select Long List

1,428 px (1.21%) changed

Before	After	Diff

Select (Open)

0 px (0%) changed

Before	After	Diff

10 screenshot(s) unchanged

• Button / Variant: Outline
• Button / Variant: Secondary Destructive
• Button / Sizes
• Button / Link as Button
• Dialog / Dialog With Select
• Dialog / Dialog With Combobox
• Dialog / Dialog With Dropdown
• Dialog (Open)
• Select / Select With Error
• Select / Select Grouped

---

Generated by Kumo Visual Regression

[mattrothenberg]

/bonk review from an a11y perspective please

[ask-bonk]

a11y Review

What's working well ✅

• Escape-to-close, focus return on close, and aria-modal on the mobile dialog are solid additions.
• Keeping aria-hidden + inert on inactive SlidingViews preserves the expected accessibility mapping.
• aria-hidden="true" on the backdrop prevents AT from surfacing it as a stray interactive element.

Issues to address 🔧

1. Mobile sidebar lacks focus trap
   Replacing Base UI Dialog removed the built-in focus trap. Since the mobile <aside> declares role="dialog" and aria-modal="true", keyboard users expect focus to be contained while it's open. Currently they can Tab out of the sidebar into the main content, which is not marked inert. This violates the ARIA dialog pattern and WCAG 2.4.3.

   Suggestion: Add a focus trap (e.g., via react-focus-lock or a small custom hook that intercepts Tab/Shift+Tab at the edges) and mark the rest of the page inert while the mobile sheet is open.

2. Mobile dialog has no accessible name
   The <aside role="dialog" aria-modal={openMobile}> does not have aria-label or aria-labelledby, so screen readers announce a generic "dialog" without context.

   Suggestion: Add an accessible name:

   role="dialog"
   aria-label="Navigation"
   aria-modal={openMobile}
   

3. Collapsible content removed inert
   Removing inert from SidebarCollapsibleContent means focusable children inside closed sections are no longer explicitly removed from the tab sequence. aria-hidden + grid-rows-[0fr] don't guarantee that focusable descendants are unreachable in all browsers.

   The auto-expand onFocus handler on SidebarCollapsible partially mitigates this, but a more robust pattern is to restore inert (or visibility: hidden) once the close animation finishes. Note that the existing test explicitly asserts inert is absent, so restoring it would need a test update too.

Overall direction is good — just need to close the focus-trap gap before this can ship from an a11y standpoint.

[ask-bonk]

Submitted the a11y review on PR #567.

Summary of findings:

• Positive: Escape-to-close, focus return, aria-modal, and backdrop aria-hidden are all good additions.
• Critical gap: The mobile sidebar replaced Base UI Dialog with a plain <aside> but lost focus trapping in the process. With role="dialog" + aria-modal, keyboard users expect focus to stay inside the sheet — currently they can Tab out into the main content.
• Missing label: The mobile dialog has no aria-label or aria-labelledby, so screen readers announce a generic unnamed dialog.
• Collapsible inert removal: Taking inert off SidebarCollapsibleContent means focusable children in closed sections may still be reachable in some browsers. The auto-expand-on-focus behavior helps, but restoring inert after the close animation would be more robust.

The review is posted at: #567 (review)

github run

[mattrothenberg]

/bonk re-review. I think we've addressed the a11y issues

[ask-bonk]

a11y re-review

Thanks for addressing the previous feedback — the fixes are solid and the semantic shift from Dialog to <nav> is the right call.

✅ What's working well

• aria-label="Navigation" on the mobile <nav> gives screen-reader users clear landmark context.
• inert restored on SidebarCollapsibleContent — closed sections are reliably removed from the tab sequence. Good test coverage here too.
• Removed role="dialog" / aria-modal sidesteps the strict Dialog focus-trap requirement; <nav> is an appropriate landmark for this pattern.
• Escape-to-close, initial focus move, and inert/aria-hidden on SlidingViews are all implemented correctly.

🔧 Remaining issues

1. Focus return on Tab-out creates a ping-pong

When users Tab forward past the last focusable item in the mobile sidebar, the focusout handler closes the drawer and the useEffect (around line 501) returns focus to triggerRef.current. That steals focus from the element the user was navigating to, creating a disorienting extra Tab stop.

Consider either:

• Option A (focus trap): Intercept Tab/Shift+Tab at the drawer edges and wrap focus inside — this matches the modal-like backdrop + Escape pattern.
• Option B (no steal): Track why the drawer closed. Only return focus when closed via Escape, backdrop click, or an explicit close action — skip the return when the user intentionally tabbed out.

2. Main content behind the backdrop is still reachable

The mobile sidebar has a visual backdrop (opacity-80) but the main page content is not marked inert or aria-hidden. Keyboard users can Tab into elements behind the backdrop, and screen-reader users can still navigate to obscured content with reading keys.

Since the Provider doesn't own the main-content sibling, consider documenting that consumers should wrap their main content and conditionally apply inert when the mobile drawer is open, or expose a hook/state that makes this straightforward.

3. No mobile test coverage

The matchMedia stub (around line 71 in sidebar.test.tsx) always returns matches: false, so useIsMobile never returns true. This means the mobile focus-management, Escape-to-close, aria-hidden toggle, and imperative inert logic are all untested.

Consider adding a mobile-mode helper and at least a few mobile-specific tests (e.g. opening moves focus into the nav, Escape closes and returns focus, inert is removed on open, backdrop click closes).

---

Overall direction is much improved. Closing the focus-steal gap and adding at least one mobile test would get this to ship-ready from an a11y standpoint.

[ask-bonk]

Posted the a11y re-review on PR #567: #567 (comment)

Summary of the review:

• Fixed and acknowledged: aria-label="Navigation" on the mobile <nav>, inert restored on SidebarCollapsibleContent, and the pragmatic shift away from role="dialog"/aria-modal to a navigation landmark.
• Three remaining issues flagged:
  1. Focus steal on Tab-out — when users Tab past the last item in the mobile drawer, the focusout handler closes it and the focus-return effect yanks focus back to the trigger. Suggested either adding a real focus trap or tracking the close reason to avoid stealing focus on intentional Tab-out.
  2. Main content not inert behind backdrop — the page behind the mobile drawer is still reachable via Tab and screen-reader reading keys despite the visual overlay. Suggested documenting that consumers should conditionally apply inert to their main content wrapper.
  3. No mobile tests — matchMedia is stubbed to desktop in all tests, so none of the mobile focus/escape/inert logic is exercised. Suggested adding a mobile test helper and a few mobile-specific cases.

github run

[mattrothenberg]

/bonk re-review please

[ask-bonk]

a11y re-review follow-up

✅ Issues addressed

1. Focus steal on Tab-out — resolved
The focusout handler now sets shouldRestoreFocusRef.current = false before closing the mobile drawer, and the focus-return useEffect only restores focus when that flag is true. The new test "should close on focus leave without stealing focus back" explicitly asserts that focus remains on the element outside the drawer. Clean implementation.

2. Mobile test coverage — resolved
Four mobile-specific tests were added covering:

• closed state: inert + aria-hidden
• open: focus moves to first focusable item
• Escape: closes drawer and returns focus to trigger
• focus leave: closes drawer without focus steal

🔧 Remaining minor item

Main content behind backdrop
useSidebar already exposes openMobile, so consumers can apply inert to their main content wrapper. Consider adding a brief note in the docs or JSDoc reminding consumers to do so — otherwise keyboard users may still Tab into obscured content despite the visual overlay.

Overall this is in good shape from an a11y standpoint.

[ask-bonk]

Submitted the a11y re-review on PR #567.

Summary:

• Focus steal on Tab-out — resolved ✅
  The focusout handler sets shouldRestoreFocusRef.current = false before closing, and the focus-return effect only restores focus when the flag is true. The new test explicitly verifies focus stays outside the drawer.

• Mobile test coverage — resolved ✅
  Four mobile-specific tests were added covering closed inert/aria-hidden, focus on open, Escape close with focus return, and focus leave without focus steal. All 35 sidebar tests pass.

• Main content behind backdrop — minor remaining item 🔧
  useSidebar already exposes openMobile, so consumers can conditionally inert their main content. Suggested adding a brief docs note reminding consumers to do so.

Overall the PR is in good shape from an a11y standpoint.

github run