ECHOES-1408 Allow icon-less accordion child items in SidebarNavigation#715
Conversation
✅ Deploy Preview for echoes-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR extends Layout.SidebarNavigation to support icon-less accordion child rows and refactors shared navigation-row logic into a reusable base implementation, while also adding ariaLabel support and updating stories/tests to match the new API shape.
Changes:
- Introduces
SidebarNavigationAccordionChildItem(exposed asSidebarNavigation.AccordionItem.Item) to allow accordion children with optional icons. - Refactors shared row rendering into
SidebarNavigationBaseItemand centralizes shared prop/types inSidebarNavigationTypes.ts. - Updates Storybook stories and adds/extends unit tests for the new child item + new
ariaLabelcoverage.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| stories/layout/sidebar-navigation/SidebarNavigationAccordionItem-stories.tsx | Updates stories to use the new SidebarNavigation.AccordionItem.Item API and demonstrates icon-less children. |
| stories/layout/sidebar-navigation/SidebarNavigation-stories.tsx | Migrates sidebar navigation stories to the new accordion child item API. |
| stories/layout/Layout-stories.tsx | Updates layout story usage to the new accordion child item API. |
| src/components/layout/sidebar-navigation/SidebarNavigationTypes.ts | Adds shared sidebar navigation item base prop/types. |
| src/components/layout/sidebar-navigation/SidebarNavigationItem.tsx | Refactors SidebarNavigationItem to reuse the new base item. |
| src/components/layout/sidebar-navigation/SidebarNavigationBaseItem.tsx | Adds shared implementation for sidebar navigation link rows (tooltip, active styles, icon handling). |
| src/components/layout/sidebar-navigation/SidebarNavigationAccordionItem.tsx | Updates accordion item props (adds ariaLabel, icon type alias, and attempted ref support). |
| src/components/layout/sidebar-navigation/SidebarNavigationAccordionChildItem.tsx | Adds new accordion child item component with optional icon support. |
| src/components/layout/sidebar-navigation/index.ts | Exports new component/types and introduces the SidebarNavigation.AccordionItem.Item namespace. |
| src/components/layout/sidebar-navigation/tests/SidebarNavigationItem-test.tsx | Adds tests for ariaLabel, keyboard navigation, and ref forwarding. |
| src/components/layout/sidebar-navigation/tests/SidebarNavigationAccordionItem-test.tsx | Updates accordion tests to use SidebarNavigationAccordionChildItem and tests icon-less children. |
| src/components/layout/sidebar-navigation/tests/SidebarNavigationAccordionChildItem-test.tsx | Adds comprehensive tests for the new accordion child item component. |
| src/components/layout/index.ts | Re-exports the new sidebar navigation types from the top-level layout module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8488af9 to
d2e33ac
Compare
Code Review ✅ ApprovedIntroduces OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|




NOTE: as we stand, with the sidebar still showing a "thin band" when collapsed, allowing no icons "breaks" the appearance, but this PR is the first in a series that will get rid of the "thin band" collapsed sidebar, and this will not be shipped on its own.
Summary
Layout.SidebarNavigation.AccordionItem.Itemfor navigation rows inside sidebar accordion sectionsValidation
yarn ts-checkyarn jest src/components/layout/sidebar-navigation