Skip to content

Commit dabbef1

Browse files
jaymantricursoragent
authored andcommitted
[origin] Fix sidebar navigation roles (#28838)
## Summary - Fixes DES-67 by removing default ARIA menu semantics from Origin Sidebar navigation containers. - Keeps explicit `role` forwarding on `Sidebar.Menu` as an escape hatch for consumers that implement complete menu/menuitem semantics and keyboard behavior. - Clarifies in stories/tests that Sidebar navigation is a layout/navigation grouping primitive; command menus should use Origin Menu, and tree semantics should use `Sidebar.Tree`. ## Accessibility decision `Sidebar.Menu` and expandable submenu containers no longer default to `role=\"menu\"` because sidebar navigation items do not implement full ARIA menu keyboard behavior. Consumers can still pass `role=\"menu\"` explicitly when they own the corresponding `menuitem` semantics and interaction model. ## Storybook preview Origin Storybook: https://dev.dev.sparkinfra.net/app/origin-storybook-pr-28838/ - `Components/Sidebar/Default`: https://dev.dev.sparkinfra.net/app/origin-storybook-pr-28838/?path=/story/components-sidebar--default - `Components/Sidebar/WithTreeItems`: https://dev.dev.sparkinfra.net/app/origin-storybook-pr-28838/?path=/story/components-sidebar--with-tree-items - `Components/Sidebar/AllItemVariants`: https://dev.dev.sparkinfra.net/app/origin-storybook-pr-28838/?path=/story/components-sidebar--all-item-variants ## Verification - `mise exec -- corepack yarn workspace @lightsparkdev/origin test:unit src/components/Sidebar/Sidebar.unit.test.tsx` - `mise exec -- corepack yarn workspace @lightsparkdev/origin exec prettier --check src/components/Sidebar/parts.tsx src/components/Sidebar/Sidebar.unit.test.tsx src/components/Sidebar/Sidebar.stories.tsx` - `mise exec -- corepack yarn workspace @lightsparkdev/origin exec eslint src/components/Sidebar/parts.tsx src/components/Sidebar/Sidebar.unit.test.tsx src/components/Sidebar/Sidebar.stories.tsx` - `mise exec -- corepack yarn workspace @lightsparkdev/origin types` Jira: https://lightspark.atlassian.net/browse/DES-67 Co-authored-by: Cursor <cursoragent@cursor.com> GitOrigin-RevId: 76341209322fdce512d9c5204eeba89f2985e363
1 parent d297d00 commit dabbef1

3 files changed

Lines changed: 44 additions & 11 deletions

File tree

packages/origin/src/components/Sidebar/Sidebar.stories.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ type Story = StoryObj<typeof Sidebar.Root>;
4545

4646
/**
4747
* Default expanded sidebar with groups and items.
48+
* Sidebar.Menu is a roleless navigation grouping primitive. If consumers opt
49+
* into role="menu", they own valid menuitem semantics and menu keyboard behavior.
4850
*/
4951
export const Default: Story = {
5052
args: { collapsed: false },
@@ -667,14 +669,15 @@ export const WithDrilldownItems: Story = {
667669
};
668670

669671
/**
670-
* TreeItem variant - expandable items with horizontal chevron that rotates 90° when expanded.
672+
* TreeItem visual variant - expandable rows with a horizontal chevron. For
673+
* true ARIA tree semantics, wrap items in Sidebar.Tree.
671674
*/
672675
export const WithTreeItems: Story = {
673676
render: () => (
674677
<Sidebar.Root>
675678
<Sidebar.Content>
676679
<Sidebar.Group>
677-
<Sidebar.GroupLabel>Tree Navigation</Sidebar.GroupLabel>
680+
<Sidebar.GroupLabel>Nested Navigation</Sidebar.GroupLabel>
678681
<Sidebar.Menu>
679682
<Sidebar.Item
680683
icon={<CentralIcon name="IconHome" size={20} />}
@@ -845,7 +848,9 @@ export const AllItemVariants: Story = {
845848
<Sidebar.Separator />
846849

847850
<Sidebar.Group>
848-
<Sidebar.GroupLabel>Tree (Horizontal Chevron)</Sidebar.GroupLabel>
851+
<Sidebar.GroupLabel>
852+
Tree-Style Row (Horizontal Chevron)
853+
</Sidebar.GroupLabel>
849854
<Sidebar.Menu>
850855
<Sidebar.TreeItem
851856
icon={<CentralIcon name="IconSquareBehindSquare1" size={20} />}

packages/origin/src/components/Sidebar/Sidebar.unit.test.tsx

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,39 @@ describe("SidebarContext", () => {
496496
});
497497

498498
describe("Accessibility", () => {
499+
describe("Menu", () => {
500+
it("does not use ARIA menu semantics by default", () => {
501+
render(
502+
<Sidebar.Root>
503+
<Sidebar.Content>
504+
<Sidebar.Menu data-testid="menu">
505+
<Sidebar.Item>Dashboard</Sidebar.Item>
506+
</Sidebar.Menu>
507+
</Sidebar.Content>
508+
</Sidebar.Root>,
509+
);
510+
511+
expect(screen.getByTestId("menu")).not.toHaveAttribute("role");
512+
expect(screen.queryByRole("menu")).not.toBeInTheDocument();
513+
});
514+
515+
it("forwards explicit menu role as an escape hatch", () => {
516+
render(
517+
<Sidebar.Root>
518+
<Sidebar.Content>
519+
<Sidebar.Menu aria-label="Custom command menu" role="menu">
520+
<div role="menuitem">Custom command</div>
521+
</Sidebar.Menu>
522+
</Sidebar.Content>
523+
</Sidebar.Root>,
524+
);
525+
526+
expect(
527+
screen.getByRole("menu", { name: "Custom command menu" }),
528+
).toBeInTheDocument();
529+
});
530+
});
531+
499532
describe("GroupLabel", () => {
500533
it("is visually hidden when collapsed", () => {
501534
render(
@@ -582,7 +615,8 @@ describe("Accessibility", () => {
582615

583616
const submenu = document.getElementById(submenuId!);
584617
expect(submenu).toBeInTheDocument();
585-
expect(submenu).toHaveAttribute("role", "menu");
618+
expect(submenu).not.toHaveAttribute("role");
619+
expect(screen.queryByRole("menu")).not.toBeInTheDocument();
586620
});
587621
});
588622

packages/origin/src/components/Sidebar/parts.tsx

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -332,12 +332,7 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(function Menu(
332332
ref,
333333
) {
334334
return (
335-
<div
336-
ref={ref}
337-
className={clsx(styles.menu, className)}
338-
role="menu"
339-
{...props}
340-
>
335+
<div ref={ref} className={clsx(styles.menu, className)} {...props}>
341336
{children}
342337
</div>
343338
);
@@ -519,7 +514,6 @@ export const ExpandableItem = React.forwardRef<
519514
id={submenuId}
520515
className={styles.submenu}
521516
data-variant={submenuVariant}
522-
role="menu"
523517
>
524518
{children}
525519
</div>

0 commit comments

Comments
 (0)