Skip to content

Commit 32577d1

Browse files
Olayinka AdelakunOlayinka Adelakun
authored andcommitted
fix(a11y): fix aria-allowed-attr, invalid sidebar list structure, and session filter label mismatch on flow editor
1 parent 54a8903 commit 32577d1

9 files changed

Lines changed: 169 additions & 25 deletions

File tree

src/frontend/src/components/core/appHeaderComponent/components/FlowMenu/__tests__/FlowMenu.spec.tsx

Lines changed: 91 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ jest.mock("@/customization/hooks/use-custom-navigate", () => ({
4242
}));
4343
jest.mock("@/stores/flowsManagerStore", () => ({
4444
__esModule: true,
45-
default: (sel) =>
45+
default: (sel: (s: object) => unknown) =>
4646
sel({
4747
autoSaving: false,
4848
saveLoading: false,
@@ -51,11 +51,12 @@ jest.mock("@/stores/flowsManagerStore", () => ({
5151
}));
5252
jest.mock("@/stores/alertStore", () => ({
5353
__esModule: true,
54-
default: (sel) => sel({ setSuccessData: jest.fn() }),
54+
default: (sel: (s: object) => unknown) =>
55+
sel({ setSuccessData: jest.fn() }),
5556
}));
5657
jest.mock("@/stores/flowStore", () => ({
5758
__esModule: true,
58-
default: (sel) =>
59+
default: jest.fn((sel: (s: object) => unknown) =>
5960
sel({
6061
onFlowPage: true,
6162
isBuilding: false,
@@ -68,19 +69,44 @@ jest.mock("@/stores/flowStore", () => ({
6869
locked: false,
6970
},
7071
}),
72+
),
7173
}));
7274
jest.mock("@/stores/shortcuts", () => ({
7375
__esModule: true,
74-
useShortcutsStore: (sel) => sel({ changesSave: "mod+s" }),
76+
useShortcutsStore: (sel: (s: object) => unknown) =>
77+
sel({ changesSave: "mod+s" }),
7578
}));
7679

7780
// Avoid pulling utils that depend on darkStore
7881
jest.mock("@/utils/utils", () => ({
7982
__esModule: true,
80-
cn: (...args) => args.filter(Boolean).join(" "),
83+
cn: (...args: unknown[]) => (args.filter(Boolean) as string[]).join(" "),
8184
getNumberFromString: () => 0,
8285
}));
8386

87+
jest.mock("@/components/ui/popover", () => ({
88+
Popover: ({ children }: { children: React.ReactNode }) => <>{children}</>,
89+
PopoverAnchor: ({ children }: { children: React.ReactNode }) => (
90+
<>{children}</>
91+
),
92+
PopoverTrigger: ({ children }: { children: React.ReactNode }) => (
93+
<>{children}</>
94+
),
95+
PopoverContent: ({
96+
children,
97+
"aria-label": ariaLabel,
98+
}: {
99+
children: React.ReactNode;
100+
"aria-label"?: string;
101+
}) => (
102+
<div data-testid="popover-content" aria-label={ariaLabel}>
103+
{children}
104+
</div>
105+
),
106+
}));
107+
108+
import React from "react";
109+
84110
// styleUtils imports lucide dynamic icons; stub to avoid resolution
85111
jest.mock("lucide-react/dynamicIconImports", () => ({}), { virtual: true });
86112

@@ -101,4 +127,64 @@ describe("FlowMenu MenuBar", () => {
101127
fireEvent.click(screen.getByTestId("save-flow-button"));
102128
expect(mockSave).toHaveBeenCalled();
103129
});
130+
131+
describe("Accessibility — aria labels", () => {
132+
const flowStoreMock = jest.requireMock("@/stores/flowStore");
133+
134+
afterEach(() => {
135+
flowStoreMock.default.mockReset();
136+
flowStoreMock.default.mockImplementation((sel: (s: object) => unknown) =>
137+
sel({
138+
onFlowPage: true,
139+
isBuilding: false,
140+
currentFlow: {
141+
id: "1",
142+
name: "Flow",
143+
folder_id: "f1",
144+
icon: "Workflow",
145+
gradient: "0",
146+
locked: false,
147+
},
148+
}),
149+
);
150+
});
151+
152+
it("menu_bar_display is a button element for keyboard and screen reader access", () => {
153+
render(<MenuBar />);
154+
const trigger = screen.getByTestId("menu_bar_display");
155+
expect(trigger.tagName).toBe("BUTTON");
156+
});
157+
158+
it("menu_bar_display button has aria-label matching the flow name", () => {
159+
render(<MenuBar />);
160+
const trigger = screen.getByTestId("menu_bar_display");
161+
expect(trigger).toHaveAttribute("aria-label", "Flow");
162+
});
163+
164+
it("menu_bar_display aria-label falls back to untitled flow when name is absent", () => {
165+
flowStoreMock.default.mockImplementation((sel: (s: object) => unknown) =>
166+
sel({
167+
onFlowPage: true,
168+
isBuilding: false,
169+
currentFlow: {
170+
id: "1",
171+
name: undefined,
172+
folder_id: "f1",
173+
icon: "Workflow",
174+
gradient: "0",
175+
locked: false,
176+
},
177+
}),
178+
);
179+
render(<MenuBar />);
180+
const trigger = screen.getByTestId("menu_bar_display");
181+
expect(trigger).toHaveAttribute("aria-label", "Untitled Flow");
182+
});
183+
184+
it("popover content has an aria-label of 'Flow settings' for screen readers", () => {
185+
render(<MenuBar />);
186+
const content = screen.getByTestId("popover-content");
187+
expect(content).toHaveAttribute("aria-label", "Flow settings");
188+
});
189+
});
104190
});

src/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,11 @@ export const MenuBar = memo((): JSX.Element => {
130130
/>
131131
</div>
132132
<PopoverTrigger asChild>
133-
<div
133+
<button
134+
type="button"
134135
className="group relative -mr-5 flex shrink-0 cursor-pointer items-center gap-2 text-sm sm:whitespace-normal"
135136
data-testid="menu_bar_display"
137+
aria-label={currentFlowName || t("flow.untitledFlow")}
136138
>
137139
<span
138140
ref={measureRef}
@@ -150,7 +152,7 @@ export const MenuBar = memo((): JSX.Element => {
150152
"sm:group-hover:translate-x-0 sm:group-hover:opacity-100",
151153
)}
152154
/>
153-
</div>
155+
</button>
154156
</PopoverTrigger>
155157
<div className={"ml-5 hidden shrink-0 items-center sm:flex"}>
156158
{!autoSaving && (
@@ -195,6 +197,7 @@ export const MenuBar = memo((): JSX.Element => {
195197
className="flex w-96 flex-col gap-4 p-4"
196198
align="center"
197199
sideOffset={15}
200+
aria-label={t("flow.flowSettings")}
198201
>
199202
<FlowSettingsComponent
200203
close={() => setOpenSettings(false)}

src/frontend/src/locales/en.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
"stepper.nextStep": "Next Step",
5959
"stepper.back": "Back",
6060
"stepper.needHelp": "Need Help?",
61+
"flow.flowSettings": "Flow settings",
6162
"flow.menu.editDetails": "Edit details",
6263
"flow.menu.export": "Export",
6364
"flow.menu.duplicate": "Duplicate",

src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoryDetails.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export function MemoryDetails({
106106
/>
107107
</button>
108108
</PopoverTrigger>
109-
<PopoverContent align="start" className="w-72 p-4">
109+
<PopoverContent align="start" className="w-72 p-4" aria-label={t("memory.configLabel")}>
110110
<div className="flex flex-col gap-3 text-xs">
111111
<div className="flex flex-col gap-0.5">
112112
<span className="font-medium text-muted-foreground">

src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoryDetailsHeader.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ export function MemoryDetailsHeader({
120120
<Button
121121
variant="outline"
122122
size="sm"
123-
aria-label={t("memory.sessionFilter")}
124123
className="w-[180px] justify-between rounded-[10px] px-3"
124+
data-testid="session-filter-button"
125125
>
126126
<span className="truncate">
127127
{sessionLabel.length > 20

src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryDetails.test.tsx

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,16 @@ jest.mock("@/components/ui/popover", () => ({
4848
{children}
4949
</div>
5050
),
51-
PopoverContent: ({ children }: { children: React.ReactNode }) => (
52-
<div data-testid="popover-content">{children}</div>
51+
PopoverContent: ({
52+
children,
53+
"aria-label": ariaLabel,
54+
}: {
55+
children: React.ReactNode;
56+
"aria-label"?: string;
57+
}) => (
58+
<div data-testid="popover-content" aria-label={ariaLabel}>
59+
{children}
60+
</div>
5361
),
5462
}));
5563

@@ -264,6 +272,12 @@ describe("MemoryDetails — Config popover", () => {
264272
const chevron = screen.getByTestId("icon-ChevronDown");
265273
expect(chevron.className).not.toContain("rotate-180");
266274
});
275+
276+
it("popover content has aria-label 'Config' so screen readers can name the panel", () => {
277+
render(<MemoryDetails {...baseProps} />);
278+
const content = screen.getByTestId("popover-content");
279+
expect(content).toHaveAttribute("aria-label", "Config");
280+
});
267281
});
268282

269283
describe("MemoryDetails — stat cards session filtering", () => {

src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryDetailsHeader.test.tsx

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,7 @@ describe("MemoryDetailsHeader", () => {
190190
const props = makeProps({ sessions: ["session-1", "session-2"] });
191191
render(<MemoryDetailsHeader {...props} />);
192192

193-
const sessionTrigger = screen.getByRole("button", {
194-
name: "Session filter",
195-
});
193+
const sessionTrigger = screen.getByTestId("session-filter-button");
196194
expect(sessionTrigger).toBeInTheDocument();
197195
expect(sessionTrigger).not.toBeDisabled();
198196
});
@@ -203,9 +201,7 @@ describe("MemoryDetailsHeader", () => {
203201
hasNextSessionsPage: false,
204202
});
205203
render(<MemoryDetailsHeader {...props} />);
206-
expect(
207-
screen.getByRole("button", { name: "Session filter" }),
208-
).not.toBeDisabled();
204+
expect(screen.getByTestId("session-filter-button")).not.toBeDisabled();
209205
});
210206

211207
it("renders the reload button with correct aria-label", () => {
@@ -468,7 +464,7 @@ describe("MemoryDetailsHeader", () => {
468464
render(<MemoryDetailsHeader {...props} />);
469465

470466
expect(
471-
screen.getByRole("button", { name: "Session filter" }),
467+
screen.getByTestId("session-filter-button"),
472468
).toHaveTextContent("All Sessions");
473469
});
474470

@@ -477,7 +473,7 @@ describe("MemoryDetailsHeader", () => {
477473
render(<MemoryDetailsHeader {...props} />);
478474

479475
expect(
480-
screen.getByRole("button", { name: "Session filter" }),
476+
screen.getByTestId("session-filter-button"),
481477
).toHaveTextContent("All Sessions");
482478
});
483479

@@ -486,7 +482,7 @@ describe("MemoryDetailsHeader", () => {
486482
render(<MemoryDetailsHeader {...props} />);
487483

488484
expect(
489-
screen.getByRole("button", { name: "Session filter" }),
485+
screen.getByTestId("session-filter-button"),
490486
).toHaveTextContent("session-1");
491487
});
492488

@@ -557,7 +553,7 @@ describe("MemoryDetailsHeader", () => {
557553
});
558554
render(<MemoryDetailsHeader {...props} />);
559555

560-
const btn = screen.getByRole("button", { name: "Session filter" });
556+
const btn = screen.getByTestId("session-filter-button");
561557
expect(btn).toHaveTextContent(`${longId.slice(0, 20)}...`);
562558
});
563559
});

src/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarSegmentedNav.test.tsx

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,46 @@ describe("SidebarSegmentedNav", () => {
355355
expect(mockUseSidebar.setActiveSection).toHaveBeenCalledWith("mcp");
356356
});
357357

358+
it("renders a separator element before the memories nav item", () => {
359+
const { container } = render(<SidebarSegmentedNav />);
360+
// The separator is a <li role="separator" aria-hidden="true"> injected before memories
361+
const separator = container.querySelector('li[role="separator"]');
362+
expect(separator).toBeInTheDocument();
363+
expect(separator).toHaveAttribute("aria-hidden", "true");
364+
});
365+
366+
it("separator renders only once", () => {
367+
const { container } = render(<SidebarSegmentedNav />);
368+
const separators = container.querySelectorAll('li[role="separator"]');
369+
expect(separators).toHaveLength(1);
370+
});
371+
372+
it("separator appears between versions and memories in the DOM order", () => {
373+
const { container } = render(<SidebarSegmentedNav />);
374+
const menuItems = container.querySelectorAll(
375+
'[data-testid="sidebar-menu-item"], li[role="separator"]',
376+
);
377+
const nodes = Array.from(menuItems);
378+
const separatorIndex = nodes.findIndex(
379+
(n) => n.getAttribute("role") === "separator",
380+
);
381+
const memoriesButton = container.querySelector('[data-testid="sidebar-nav-memories"]');
382+
const memoriesItem = memoriesButton?.closest('[data-testid="sidebar-menu-item"]');
383+
const memoriesIndex = nodes.indexOf(memoriesItem as Element);
384+
// Separator should appear immediately before memories
385+
expect(separatorIndex).toBe(memoriesIndex - 1);
386+
});
387+
388+
it("sidebar-menu has no direct div children between it and menu items", () => {
389+
const { container } = render(<SidebarSegmentedNav />);
390+
const sidebarMenu = container.querySelector('[data-testid="sidebar-menu"]');
391+
const directDivChildren = Array.from(sidebarMenu?.children ?? []).filter(
392+
(child) => child.tagName === "DIV" && !child.hasAttribute("data-testid"),
393+
);
394+
// No anonymous wrapper divs — each direct child is either a menu-item or separator
395+
expect(directDivChildren).toHaveLength(0);
396+
});
397+
358398
it("exports NAV_ITEMS correctly", () => {
359399
expect(NAV_ITEMS).toHaveLength(6);
360400
expect(NAV_ITEMS[0]).toEqual({

src/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/sidebarSegmentedNav.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1+
import { Fragment } from "react";
12
import { useTranslation } from "react-i18next";
23
import ForwardedIconComponent from "@/components/common/genericIconComponent";
34
import ShadTooltip from "@/components/common/shadTooltipComponent";
4-
import { Separator } from "@/components/ui/separator";
55
import {
66
SidebarMenu,
77
SidebarMenuButton,
@@ -30,9 +30,13 @@ const SidebarSegmentedNav = () => {
3030
<div className="flex h-full flex-col border-r border-border bg-background">
3131
<SidebarMenu className="gap-2 py-1">
3232
{NAV_ITEMS.map((item) => (
33-
<div key={item.id}>
33+
<Fragment key={item.id}>
3434
{item.id === "memories" && (
35-
<Separator className="mx-auto my-1 w-5" />
35+
<li
36+
role="separator"
37+
aria-hidden="true"
38+
className="mx-auto my-1 w-5 border-t border-border"
39+
/>
3640
)}
3741
<SidebarMenuItem className="px-1 pt-1">
3842
<ShadTooltip content={t(item.tooltip)} side="right">
@@ -75,7 +79,7 @@ const SidebarSegmentedNav = () => {
7579
</SidebarMenuButton>
7680
</ShadTooltip>
7781
</SidebarMenuItem>
78-
</div>
82+
</Fragment>
7983
))}
8084
</SidebarMenu>
8185
</div>

0 commit comments

Comments
 (0)