feat: add Sidebar.Trigger, collapseMode, and peekOnHover; fix a11y roles and label truncation - #867
feat: add Sidebar.Trigger, collapseMode, and peekOnHover; fix a11y roles and label truncation#867ravisuhag wants to merge 8 commits into
Conversation
- Sidebar.Main now uses role=list and groups render as role=listitem, forming valid nested lists (previously items were orphaned listitems and groups were accidental region landmarks) - Remove invalid role=banner from Sidebar.Header (banner is a page-level landmark and must not be nested inside the navigation landmark) - Truncate group labels and more-menu labels with ellipsis, matching item labels; add min-width: 0 so ellipsis engages inside flex rows - Show a tooltip with the full label when an expanded item's text is clipped (previously tooltips only appeared when collapsed) - Consolidate the label typography shared by item, group, and more-menu text into one rule; switch white-space: nowrap to text-wrap: nowrap for consistency with other components
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughSidebar collapse now supports icon and hidden modes, hover peeking, popup-aware state, and configurable collapse tooltips. Sequence Diagram(s)sequenceDiagram
participant SidebarTrigger
participant SidebarRoot
participant SidebarItem
participant Tooltip
SidebarTrigger->>SidebarRoot: Toggle open state
SidebarRoot->>SidebarItem: Provide collapsed and peeking state
SidebarItem->>Tooltip: Show tooltip when label is clipped
Tooltip->>SidebarItem: Report tooltip state
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Adds Sidebar.Trigger for wiring a toggle button without managing state,
plus two new root props: collapseMode ("icon" | "hidden") to control
whether collapsing shrinks to an icon rail or hides completely behind a
floating panel with a backdrop, and peekOnHover to preview a collapsed
sidebar on hover without changing the real open state.
Also fixes a bug where peeking expanded the sidebar's width but left
item and group labels invisible (CSS collapse-hiding rules keyed only
on data-closed, which stays true during a peek), and updates the docs
with the new props, corrected behavior claims, and accessibility notes.
ravisuhag
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/raystack/components/sidebar/sidebar-misc.tsx (1)
150-152: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCollapsible group header misses the
trailingIconmodifier class.The non-collapsible branch (line 104-106) conditionally adds
styles['nav-group-header-with-trailing']whentrailingIconis set, but the collapsible branch'sAccordionPrimitive.Headerdoesn't, even thoughtrailingIconis rendered here too (line 171-174). Collapsible groups with a trailing icon will miss the intended header layout adjustment.🐛 Proposed fix
<AccordionPrimitive.Header - className={cx(styles['nav-group-header'], classNames?.header)} + className={cx( + styles['nav-group-header'], + trailingIcon && styles['nav-group-header-with-trailing'], + classNames?.header + )} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/sidebar/sidebar-misc.tsx` around lines 150 - 152, Update the collapsible branch’s AccordionPrimitive.Header className to conditionally include styles['nav-group-header-with-trailing'] when trailingIcon is set, matching the non-collapsible header behavior and preserving the existing nav-group-header and classNames?.header classes.
🧹 Nitpick comments (2)
packages/raystack/components/sidebar/__tests__/sidebar.test.tsx (1)
174-180: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest doesn't verify what its title claims.
hides header/main/footer content when collapsed and hiddenonly assertsdata-collapse-mode/data-floatingattributes onnav; it never checks thatSidebar.Header/Main/Footercontent is actually hidden (e.g.not.toBeVisible()or absence from the accessibility tree). A regression that stops the CSS from hiding content wouldn't be caught by this test.♻️ Suggested strengthening
it('hides header/main/footer content when collapsed and hidden', () => { render(<BasicSidebar open={false} collapseMode='hidden' />); const nav = screen.getByRole('navigation'); expect(nav).toHaveAttribute('data-collapse-mode', 'hidden'); expect(nav).not.toHaveAttribute('data-floating'); + expect(screen.getByText(HEADER_TEXT)).not.toBeVisible(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/sidebar/__tests__/sidebar.test.tsx` around lines 174 - 180, Strengthen the test named “hides header/main/footer content when collapsed and hidden” by asserting that the rendered Sidebar.Header, Sidebar.Main, and Sidebar.Footer content is hidden or absent from the accessibility tree when open is false and collapseMode is hidden. Keep the existing navigation attribute assertions unchanged.packages/raystack/components/sidebar/sidebar-misc.tsx (1)
114-116: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClipped-label tooltip pattern not extended to group and "more" trigger labels.
SidebarItemnow tracks a ref and comparesscrollWidth/clientWidthto conditionally show a tooltip only when its expanded label is actually clipped. Bothnav-group-labeland theSidebarMoretrigger label share the same new truncation/ellipsis styling but never get an equivalent tooltip, so a clipped group or "More"-button label offers no way to see the full text — a gap relative to the PR's stated "tooltips for clipped labels when expanded" behavior.
packages/raystack/components/sidebar/sidebar-misc.tsx#L114-L116: wrap the group's label<span>with the same ref +scrollWidth > clientWidthcontrolledTooltippattern used inSidebarItem(apply to both the non-collapsible and collapsible label spans).packages/raystack/components/sidebar/sidebar-more.tsx#L54-L54: apply the same clip-detectionTooltipwrapping to the trigger's label<span>when the sidebar is expanded (i.e. notisCollapsed).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/sidebar/sidebar-misc.tsx` around lines 114 - 116, The group label spans in packages/raystack/components/sidebar/sidebar-misc.tsx#L114-L116 and the trigger label span in packages/raystack/components/sidebar/sidebar-more.tsx#L54 lack clipped-label tooltips. Extend the SidebarItem ref plus scrollWidth/clientWidth detection and Tooltip pattern to both non-collapsible and collapsible group labels, and apply it to SidebarMore’s label only when isCollapsed is false; preserve the existing label styling and collapsed behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/www/src/content/docs/components/sidebar/index.mdx`:
- Line 53: Revise the sidebar navigation description to avoid beginning three
consecutive sentences with “Use,” while preserving the explanations of
collapsible, hideCollapsedItemTooltip, collapseMode, and peekOnHover.
In `@packages/raystack/components/sidebar/sidebar-root.tsx`:
- Around line 96-138: Update the floating-state logic around showFloating so an
active peek is only treated as floating while the sidebar remains closed. When
open becomes true during peeking, ensure showFloating becomes false immediately
so the normal open layout is rendered, while preserving floating behavior for
hidden open sidebars and closed peeking sidebars.
In `@packages/raystack/components/sidebar/sidebar.module.css`:
- Around line 55-61: Update the floating-reveal selectors for .header, .main,
and .footer, including the related collapse-hiding rules around the referenced
sections, so their specificity matches or exceeds the closed-hiding selectors in
the hidden collapse mode. Ensure data-floating overrides opacity, visibility,
and pointer-events while peeking, without changing the normal data-closed
behavior when the sidebar is not floating.
---
Outside diff comments:
In `@packages/raystack/components/sidebar/sidebar-misc.tsx`:
- Around line 150-152: Update the collapsible branch’s AccordionPrimitive.Header
className to conditionally include styles['nav-group-header-with-trailing'] when
trailingIcon is set, matching the non-collapsible header behavior and preserving
the existing nav-group-header and classNames?.header classes.
---
Nitpick comments:
In `@packages/raystack/components/sidebar/__tests__/sidebar.test.tsx`:
- Around line 174-180: Strengthen the test named “hides header/main/footer
content when collapsed and hidden” by asserting that the rendered
Sidebar.Header, Sidebar.Main, and Sidebar.Footer content is hidden or absent
from the accessibility tree when open is false and collapseMode is hidden. Keep
the existing navigation attribute assertions unchanged.
In `@packages/raystack/components/sidebar/sidebar-misc.tsx`:
- Around line 114-116: The group label spans in
packages/raystack/components/sidebar/sidebar-misc.tsx#L114-L116 and the trigger
label span in packages/raystack/components/sidebar/sidebar-more.tsx#L54 lack
clipped-label tooltips. Extend the SidebarItem ref plus scrollWidth/clientWidth
detection and Tooltip pattern to both non-collapsible and collapsible group
labels, and apply it to SidebarMore’s label only when isCollapsed is false;
preserve the existing label styling and collapsed behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 530d358a-c5c9-4425-8a97-83cf38cad7ca
📒 Files selected for processing (13)
apps/www/src/content/docs/components/sidebar/demo.tsapps/www/src/content/docs/components/sidebar/index.mdxapps/www/src/content/docs/components/sidebar/props.tspackages/raystack/components/sidebar/__tests__/sidebar.test.tsxpackages/raystack/components/sidebar/index.tsxpackages/raystack/components/sidebar/sidebar-item.tsxpackages/raystack/components/sidebar/sidebar-misc.tsxpackages/raystack/components/sidebar/sidebar-more.tsxpackages/raystack/components/sidebar/sidebar-root.tsxpackages/raystack/components/sidebar/sidebar-trigger.tsxpackages/raystack/components/sidebar/sidebar.module.csspackages/raystack/components/sidebar/sidebar.tsxpackages/raystack/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/raystack/components/sidebar/sidebar-item.tsx
- Keep deprecated tooltipMessage working as an alias for collapseTooltip - Track visual open state in data-open/data-closed so a hover peek turns off every collapse-hiding rule, fixing the broken peek with collapseMode="hidden" and removing the fragile CSS override block - Clear the peek (and its pending timer) when the sidebar opens for real, so pinning it open no longer leaves a stuck fixed overlay - Hold a hover peek while a portaled popup (More menu, truncation tooltip) is open instead of collapsing it mid-use - Open item tooltips away from the sidebar edge based on position - Restore standalone rendering of Item/Group/More outside <Sidebar> via an internal fallback context; public useSidebar still throws - Let hideCollapsedItemTooltip disable the clipped-label tooltip too - Ignore already-handled Escape keydowns in the floating panel
The floating overlay concept caused both reported bugs: opening a hidden-mode sidebar floated it over content instead of expanding like icon mode, and a peek jumped to the viewport edge via position:fixed — outside any container (like the docs demos), it landed away from the cursor and flickered in a mouseenter/mouseleave loop. - Expanding now works the same in both collapse modes: in place, pushing content; hidden mode only changes the collapsed look - A peek expands in place too, marked data-peeking with a lifted shadow; no fixed positioning, no backdrop, no Escape handler - Merge the item's two Tooltip branches into one always-controlled Tooltip, fixing a base-ui controlled/uncontrolled warning fired on every peek transition - Update docs and tests to match
A peek now paints over the adjacent content: the sidebar keeps only its collapsed footprint in the layout — a negative inline margin cancels everything beyond the rail/strip width — so neighbors never move while the panel slides out. Still no fixed positioning, so it stays anchored inside any container. Margin transitions in lockstep with width. A real open (pinning) still expands in place and pushes content.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/raystack/components/sidebar/sidebar-item.tsx (1)
120-138: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPopup-open counter can leak if the item unmounts while its tooltip is open.
onPopupOpenChangeis only called fromonOpenChange, with no unmount cleanup. If thisSidebarItemunmounts whiletooltipOpenistrue(e.g. filtered out of a list, route change while hovering),sidebar-root.tsx'sopenPopupCountRefnever gets its matching decrement and stays inflated forever, permanently preventing that sidebar's hover-peek from collapsing on mouse-leave (until a realopen=truetransition resets it).🐛 Proposed fix
return ( <Tooltip open={tooltipOpen} onOpenChange={open => { const el = textRef.current; const next = open && (isCollapsed || (el != null && el.scrollWidth > el.clientWidth)); // Report to the sidebar so an open tooltip holds a hover peek. if (next !== tooltipOpen) onPopupOpenChange(next); setTooltipOpen(next); }} > + {useEffect(() => { + return () => { + if (tooltipOpen) onPopupOpenChange(false); + }; + }, [tooltipOpen, onPopupOpenChange])} <Tooltip.Trigger render={content} />(Move the
useEffectout of JSX in the actual implementation — shown inline here only to anchor the fix location.) The same gap likely exists inSidebarMore's<Menu onOpenChange={open => onPopupOpenChange(open)}>, worth checking there too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/sidebar/sidebar-item.tsx` around lines 120 - 138, Add unmount cleanup for the open-popup state in SidebarItem: when tooltipOpen is true during unmount, call onPopupOpenChange(false) so sidebar-root.tsx receives the matching decrement. Implement this through a useEffect cleanup outside the JSX, and apply the same cleanup pattern to SidebarMore’s menu open state if it can unmount while open.
🧹 Nitpick comments (2)
packages/raystack/components/sidebar/sidebar.module.css (1)
68-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the repeated
240pxopen-width into a CSS variable.The literal
240pxnow appears in 5+ places (peek width, both positioncalc()margin formulas, and the pre-existing collapse-hidden max-width comment). A future width change risks updating some occurrences but not others, silently breaking the peek-margin math.♻️ Proposed refactor
+.root { + --sidebar-open-width: 240px; +} + .root[data-peeking] { - width: 240px; + width: var(--sidebar-open-width); z-index: var(--rs-z-index-portal); box-shadow: var(--rs-shadow-lifted); } .root[data-peeking][data-position="left"] { - margin-right: calc(var(--rs-space-12) - 240px); + margin-right: calc(var(--rs-space-12) - var(--sidebar-open-width)); }Also applies to: 75-76, 79-80, 83-84, 87-88, 106-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/sidebar/sidebar.module.css` around lines 68 - 69, Extract the shared 240px open-width value into a CSS custom property in the sidebar stylesheet, then replace the repeated width literals and related position calc() margin values—including the collapse-hidden max-width usage—with that variable. Update the affected selectors around .root[data-peeking] and the referenced position/collapse rules while preserving their existing layout behavior.packages/raystack/components/sidebar/__tests__/sidebar.test.tsx (1)
578-623: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the clipped-label mock into a shared helper.
The
Object.defineProperty(text, 'scrollWidth'/'clientWidth', ...)pair is duplicated verbatim across this test and the neighboring one (and again around line 541-577). A smallsimulateClippedLabel(el, { scrollWidth, clientWidth })helper would remove the repetition and make the intent clearer at each call site.♻️ Proposed helper
+function simulateClippedLabel(el: HTMLElement, scrollWidth = 300, clientWidth = 100) { + Object.defineProperty(el, 'scrollWidth', { value: scrollWidth, configurable: true }); + Object.defineProperty(el, 'clientWidth', { value: clientWidth, configurable: true }); +}Then each test collapses to
simulateClippedLabel(text);.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/sidebar/__tests__/sidebar.test.tsx` around lines 578 - 623, Extract the repeated scrollWidth/clientWidth Object.defineProperty setup from the sidebar tooltip tests into a shared simulateClippedLabel helper, supporting optional width values with the existing defaults. Replace the duplicated setup in the neighboring clipped-label tests, including the cases around the referenced test blocks, with calls to this helper while preserving each test’s behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/raystack/components/sidebar/sidebar-root.tsx`:
- Around line 187-201: Update the sidebar markup in the aside component and its
collapse button: remove aria-expanded from the navigation landmark, and apply
aria-expanded plus aria-controls to the toggle button only when collapsible is
true. Keep aria-expanded synchronized with open, ensure aria-controls references
the sidebar region’s stable id, and leave non-collapsible sidebars without
aria-expanded.
---
Outside diff comments:
In `@packages/raystack/components/sidebar/sidebar-item.tsx`:
- Around line 120-138: Add unmount cleanup for the open-popup state in
SidebarItem: when tooltipOpen is true during unmount, call
onPopupOpenChange(false) so sidebar-root.tsx receives the matching decrement.
Implement this through a useEffect cleanup outside the JSX, and apply the same
cleanup pattern to SidebarMore’s menu open state if it can unmount while open.
---
Nitpick comments:
In `@packages/raystack/components/sidebar/__tests__/sidebar.test.tsx`:
- Around line 578-623: Extract the repeated scrollWidth/clientWidth
Object.defineProperty setup from the sidebar tooltip tests into a shared
simulateClippedLabel helper, supporting optional width values with the existing
defaults. Replace the duplicated setup in the neighboring clipped-label tests,
including the cases around the referenced test blocks, with calls to this helper
while preserving each test’s behavior.
In `@packages/raystack/components/sidebar/sidebar.module.css`:
- Around line 68-69: Extract the shared 240px open-width value into a CSS custom
property in the sidebar stylesheet, then replace the repeated width literals and
related position calc() margin values—including the collapse-hidden max-width
usage—with that variable. Update the affected selectors around
.root[data-peeking] and the referenced position/collapse rules while preserving
their existing layout behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c346aacd-8e2a-41cc-b3ef-cfe6802db4ad
📒 Files selected for processing (8)
apps/www/src/content/docs/components/sidebar/index.mdxapps/www/src/content/docs/components/sidebar/props.tspackages/raystack/components/sidebar/__tests__/sidebar.test.tsxpackages/raystack/components/sidebar/sidebar-item.tsxpackages/raystack/components/sidebar/sidebar-misc.tsxpackages/raystack/components/sidebar/sidebar-more.tsxpackages/raystack/components/sidebar/sidebar-root.tsxpackages/raystack/components/sidebar/sidebar.module.css
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/raystack/components/sidebar/sidebar-more.tsx
- apps/www/src/content/docs/components/sidebar/props.ts
- packages/raystack/components/sidebar/sidebar-misc.tsx
| <aside | ||
| className={cx(styles.root, className)} | ||
| data-position={position} | ||
| data-variant={variant} | ||
| data-open={visualOpen ? '' : undefined} | ||
| data-closed={!visualOpen ? '' : undefined} | ||
| data-collapse-disabled={!collapsible ? '' : undefined} | ||
| data-collapse-mode={collapseMode} | ||
| data-peeking={isPeeking ? '' : undefined} | ||
| onMouseEnter={handleMouseEnter} | ||
| onMouseLeave={handleMouseLeave} | ||
| aria-label='Navigation Sidebar' | ||
| aria-expanded={open} | ||
| role='navigation' | ||
| {...props} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the referenced sidebar implementation and surrounding code.
printf '--- files ---\n'
git ls-files | rg 'packages/raystack/components/sidebar/sidebar-root\.tsx|packages/raystack/components/sidebar' || true
printf '\n--- target file outline ---\n'
ast-grep outline packages/raystack/components/sidebar/sidebar-root.tsx --view expanded || true
printf '\n--- referenced lines with context ---\n'
sed -n '140,240p' packages/raystack/components/sidebar/sidebar-root.tsx
printf '\n--- all aria-expanded/usages in sidebar root ---\n'
rg -n "aria-expanded|aria-controls|resizeHandle|collapsible|open|handleOpenChange|sidebarId" packages/raystack/components/sidebar/sidebar-root.tsx packages/raystack/components/sidebar -S || trueRepository: raystack/apsara
Length of output: 244
🌐 Web query:
MDN aria-expanded applied focusable interactive control toggles visibility avoid including elements do not control expanded state
💡 Result:
The aria-expanded attribute is a WAI-ARIA state used to indicate to assistive technology (AT) whether a focusable, interactive control is currently expanded or collapsed, and consequently whether the content it controls is visible or hidden [1][2][3]. Key implementation principles include: Application to Controls: The attribute must be applied to the focusable, interactive element that triggers the visibility change, not the controlled content itself [1][4]. Common examples include buttons for menus, accordions, and disclosure widgets [1]. Communication of State: - true: Indicates the controlled content is currently visible [1][2]. - false: Indicates the controlled content is currently hidden [1][2]. - Omitted/undefined: Indicates the element does not control an expandable region or that the state is not applicable [1][5][2]. Avoid Misuse: Because the presence of aria-expanded explicitly signals that an element controls the expanded state of another object, it should not be included on elements that do not perform this function [1][6][3]. Additionally, it should only be used for dynamic in-page changes handled by JavaScript; it is inappropriate for controls that trigger a full page refresh [2][7][3]. Best Practices: - Associating Content: Use the aria-controls attribute on the controlling element to explicitly reference the ID of the controlled element, reinforcing the relationship [1][8][4]. - Dynamic Updates: JavaScript must be used to update the aria-expanded value in real-time when the state changes, as screen readers rely on this to announce status updates to users [2][7][3]. - Default Values: Ensure the attribute is set to the correct initial state (e.g., false for a collapsed accordion) to accurately reflect the UI's starting condition [2][3].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-expanded
- 2: https://www.a11y-collective.com/blog/aria-expanded/
- 3: https://www.boia.org/blog/what-does-aria-expanded-mean
- 4: https://dustinwhisman.com/writing/accessibility-how-tos/how-to-avoid-accessibility-issues-with-interactive-elements/
- 5: https://developer.mozilla.org/en-US/docs/Web/API/Element/ariaExpanded
- 6: https://github.com/mdn/content/blob/main/files/en-us/web/accessibility/aria/reference/attributes/aria-expanded/index.md
- 7: https://www.accessibility-developer-guide.com/examples/sensible-aria-usage/expanded/
- 8: https://www.w3.org/WAI/ARIA/apg/patterns/disclosure/examples/disclosure-faq/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- files ---'
git ls-files | rg 'packages/raystack/components/sidebar/sidebar-root\.tsx|packages/raystack/components/sidebar' || true
printf '%s\n' ''
printf '%s\n' '--- target file outline ---'
ast-grep outline packages/raystack/components/sidebar/sidebar-root.tsx --view expanded || true
printf '%s\n' ''
printf '%s\n' '--- referenced lines with context ---'
sed -n '140,250p' packages/raystack/components/sidebar/sidebar-root.tsx
printf '%s\n' ''
printf '%s\n' '--- sidebar aria/controls/collapsible usages ---'
rg -n "aria-expanded|aria-controls|resizeHandle|collapsible|open|handleOpenChange|sidebarId|role=\"button\"|onClick=\\(\\) => open" packages/raystack/components/sidebar packages/raystack/components/sidebar/sidebar-root.tsx -S || trueRepository: raystack/apsara
Length of output: 27947
Move aria-expanded/aria-controls off the sidebar landmark.
The toggleable state belongs on the collapse button, not the <aside role="navigation">, per aria-expanded guidance; the control also needs aria-controls to identify the sidebar region it shows/hides. Apply these attributes only when collapsible is true and update them in sync with open; when collapsible is false, leave the non-collapsible <aside> without aria-expanded.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/raystack/components/sidebar/sidebar-root.tsx` around lines 187 -
201, Update the sidebar markup in the aside component and its collapse button:
remove aria-expanded from the navigation landmark, and apply aria-expanded plus
aria-controls to the toggle button only when collapsible is true. Keep
aria-expanded synchronized with open, ensure aria-controls references the
sidebar region’s stable id, and leave non-collapsible sidebars without
aria-expanded.
…ding Resets Sidebar.Header's background for the inset variant, which previously stayed opaque and could paint over adjacent shadowed content. Also documents the existing data-collapse-hidden opt-in on SidebarHeader (its content is arbitrary so it can't auto-hide like nav items) and adds a regression test for the wiring. Fixes #840
- Derive all sidebar widths from two CSS variables and drop the dead 220px base width and duplicated peek-margin rules - Share base styles between nav items, More menu items, and the More trigger instead of copying them - Report tooltip/menu open state via effects so unmounting mid-popup can no longer leave a hover peek stuck open - Replace the div-based resize handle with a native <button> - Split sidebar-misc.tsx into sidebar-header/footer/group files and dedupe the group's accordion and plain branches - Simplify the More context to a plain boolean and remove small DOM noise (data-active="false", aria-disabled="false")
1 participant
Summary
Started as an a11y/truncation audit of the Sidebar component and grew into a broader pass: it also adds a
Sidebar.Triggersubcomponent and two new collapse behaviors (collapseMode,peekOnHover) designed collaboratively to keep the prop surface small and every combination meaningful.New:
Sidebar.TriggerSidebar.Header(or elsewhere) without wiring upopen/onOpenChangeyourself.collapsible={false}.useSidebarandSidebarContextValueare now exported from the package for custom header/footer/item content that needs to read collapse state.New:
collapseMode— what "collapsed" looks likecollapseMode="icon"(default): unchanged — shrinks to an icon rail, pushes content.collapseMode="hidden": collapses to a thin edge strip and reveals as a floating panel with a backdrop when opened. Closes on backdrop click or Escape.position: fixed, so it won't line up correctly under an ancestor with its owntransform/filter/contain.Sidebar.Triggernested insideSidebar.Header/Main/Footergets hidden along with everything else whencollapseMode="hidden"is closed — docs call this out and show the working pattern (Trigger as a direct child of<Sidebar>, or a separate toggle outside it entirely).New:
peekOnHoveropenstate — similar to VS Code's auto-hide sidebar. Reverts on mouse leave; clicking the resize handle or aSidebar.Triggerpins it open for real.collapsible={false}or while already open (documented).Bug fix found during review
data-closedalone, which stays true throughout a peek (it tracksopen, notisPeeking). Added floating-state overrides for every affected rule (.nav-text,.nav-group-header,.nav-group-label,.nav-group-chevron,[data-collapse-hidden]), placed after the hide rules so they win the cascade tie-break. Verified live in a browser via computed styles before/after the fix.Accessibility (original scope)
Sidebar.Mainis nowrole="list"(wasrole="group"); items renderrole="listitem", which needs a list parent.role="listitem"(was an unlabeled<section>, which mapped to aregionlandmark). Together with the innerrole="list"items container, groups now form valid nested lists (ul > li > ul).role="banner"fromSidebar.Header— banner is a page-level landmark and must not nest inside navigation.collapseMode="hidden"floating panel; documented that the floating panel doesn't trap focus or setaria-modal.Text truncation (original scope)
min-width: 0so ellipsis reliably engages inside flex rows.Cleanup
.nav-text,.nav-group-label,.more-menu-item-textinto one shared rule.white-space: nowrap→text-wrap: nowrap, matching button/breadcrumb/filter-chip.tooltipMessageprop (superseded bycollapseTooltip).Testing
Sidebar.Trigger,collapseMode,peekOnHover); 2152/2152 across the full package.collapseMode="hidden"floating panel opens/closes via backdrop click and Escape, andpeekOnHoverexpands with fully visible labels and reverts on mouse-leave (confirmed via computed styles, not just visual inspection).