Skip to content

Commit 62760b2

Browse files
viktoravelinoviktoravelino
authored
fix(a11y): name command palette, dialogs, and crash screen (LE-2043) (#14313)
* fix(a11y): name command palette, dialogs, and crash screen (LE-2043) Pre-existing gaps documented by the round-2 FINDING tests. cmdk hardcodes aria-labelledby and role="separator" after spreading caller props, so the first two needed the primitive reworked rather than a prop. - CommandInput drops cmdk's aria-labelledby when given aria-label, so the caller's name wins instead of resolving to an empty label element - CommandSeparator renders as presentational; separator is not an allowed child of CommandList's listbox - CommandDialog requires a label, naming the dialog and the search input - popoverObject names its Command, the last unnamed combobox call site - EditFlowSettings hides the lock switch when no setLocked is passed, so shareModal stops shipping a focusable inert control - BaseModal ariaLabel now applies to the dialog paths; templatesModal uses it and announces as "Templates" instead of "Dialog" - Crash screen: h1 title, link-wrapped report button, role="alert" Findings 3 and 8 are covered by #14250 and #14110 and left to those PRs. * fix(a11y): name modals with a hidden title, label dropdown search Review follow-ups. BaseModal suppressed the DialogTitle when ariaLabel was set but rendered nothing in its place, so Radix logged "DialogContent requires a DialogTitle" on every render of templatesModal and of the canvas playground modal, which already passed ariaLabel with no Header. Render a VisuallyHidden DialogTitle from ariaLabel instead; full-screen keeps the attribute since it is a plain div, not a Radix dialog. dropdownComponent's search box is a bare input that had only a placeholder, so the most common dropdown in the product still shipped an unnamed search field.
1 parent 3ac86f3 commit 62760b2

12 files changed

Lines changed: 335 additions & 134 deletions

File tree

‎src/frontend/src/components/common/crashErrorComponent/__tests__/crashErrorComponent.a11y.test.tsx‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { render, screen } from "@testing-library/react";
2+
import userEvent from "@testing-library/user-event";
3+
import { axe } from "@/utils/a11y-test";
4+
import CrashErrorComponent from "../index";
5+
6+
const renderCrashScreen = (resetErrorBoundary = jest.fn()) =>
7+
render(
8+
<CrashErrorComponent
9+
error={{ message: "boom", stack: "at boom" }}
10+
resetErrorBoundary={resetErrorBoundary}
11+
/>,
12+
);
13+
14+
describe("CrashErrorComponent accessibility", () => {
15+
it("should_have_no_axe_violations", async () => {
16+
const { container } = renderCrashScreen();
17+
18+
expect(await axe(container)).toHaveNoViolations();
19+
});
20+
21+
// The crash screen replaces the whole app, so it owns the page's heading
22+
// structure. Rendering the title as a <p> left the page with zero headings
23+
// (WCAG 1.3.1 / 2.4.6).
24+
it("should_expose_the_title_as_the_page_heading", () => {
25+
renderCrashScreen();
26+
27+
expect(
28+
screen.getByRole("heading", { level: 1, name: /unexpected error/i }),
29+
).toBeInTheDocument();
30+
});
31+
32+
// Nothing announced the failure to a screen reader: the boundary swaps the
33+
// tree without moving focus, so the region has to assert itself (WCAG 4.1.3).
34+
it("should_announce_the_failure_through_an_alert_region", () => {
35+
renderCrashScreen();
36+
37+
expect(screen.getByRole("alert")).toContainElement(
38+
screen.getByRole("heading", { level: 1 }),
39+
);
40+
});
41+
42+
// The report action used to be a <button> nested inside an <a href>, which
43+
// is a nested-interactive violation and gives the anchor no reachable name
44+
// of its own (WCAG 4.1.2).
45+
it("should_render_the_report_action_as_a_single_link", () => {
46+
renderCrashScreen();
47+
48+
const report = screen.getByRole("link", { name: /report on github/i });
49+
expect(report).toHaveAttribute(
50+
"href",
51+
"https://github.com/langflow-ai/langflow/issues/new",
52+
);
53+
expect(report.querySelector("button")).toBeNull();
54+
expect(
55+
screen.queryByRole("button", { name: /report on github/i }),
56+
).not.toBeInTheDocument();
57+
});
58+
59+
it("should_reset_the_error_boundary_from_the_restart_button", async () => {
60+
const user = userEvent.setup();
61+
const resetErrorBoundary = jest.fn();
62+
renderCrashScreen(resetErrorBoundary);
63+
64+
await user.click(screen.getByRole("button", { name: /restart/i }));
65+
66+
expect(resetErrorBoundary).toHaveBeenCalled();
67+
});
68+
});

‎src/frontend/src/components/common/crashErrorComponent/index.tsx‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,19 @@ export default function CrashErrorComponent({
1313
<div className="z-50 flex h-screen w-screen items-center justify-center bg-foreground bg-opacity-50">
1414
<div className="flex h-screen w-screen flex-col bg-background text-start shadow-lg">
1515
<main className="m-auto grid w-1/2 justify-center gap-5 text-center">
16-
<Card className="p-8">
16+
<Card className="p-8" role="alert">
1717
<CardHeader>
1818
<div className="m-auto">
19-
<XCircle strokeWidth={1.5} className="h-16 w-16" />
19+
<XCircle
20+
strokeWidth={1.5}
21+
className="h-16 w-16"
22+
aria-hidden="true"
23+
/>
2024
</div>
2125
<div>
22-
<p className="mb-4 text-xl text-foreground">
26+
<h1 className="mb-4 text-xl text-foreground">
2327
{t("crash.title")}
24-
</p>
28+
</h1>
2529
</div>
2630
</CardHeader>
2731

@@ -50,15 +54,15 @@ export default function CrashErrorComponent({
5054
{t("crash.restartButton")}
5155
</Button>
5256

53-
<a
54-
href="https://github.com/langflow-ai/langflow/issues/new"
55-
target="_blank"
56-
rel="noopener noreferrer"
57-
>
58-
<Button className="ml-3" ignoreTitleCase variant={"outline"}>
57+
<Button className="ml-3" variant="outline" asChild>
58+
<a
59+
href="https://github.com/langflow-ai/langflow/issues/new"
60+
target="_blank"
61+
rel="noopener noreferrer"
62+
>
5963
{t("crash.reportButton")}
60-
</Button>
61-
</a>
64+
</a>
65+
</Button>
6266
</div>
6367
</CardFooter>
6468
</Card>

‎src/frontend/src/components/core/dropdownComponent/__tests__/dropdownValueReset.test.tsx‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { render } from "@testing-library/react";
1+
import { render, screen } from "@testing-library/react";
22
import type { ReactNode } from "react";
33
import type { APIClassType } from "@/types/api";
44
import Dropdown from "../index";
@@ -190,3 +190,26 @@ describe("Dropdown value reset bug", () => {
190190
expect(mockOnSelect).not.toHaveBeenCalledWith("", undefined, true);
191191
});
192192
});
193+
194+
describe("Dropdown accessibility", () => {
195+
// The search box is a bare <input> that only carried a placeholder, which is
196+
// not an accessible name (WCAG 4.1.2 / 3.3.2).
197+
it("should_name_the_search_input", () => {
198+
render(
199+
<Dropdown
200+
value="tool_a"
201+
options={["tool_a", "tool_b"]}
202+
onSelect={jest.fn()}
203+
name="tool"
204+
nodeId="test-node"
205+
nodeClass={mockNodeClass}
206+
handleNodeClass={jest.fn()}
207+
id="test-dropdown"
208+
/>,
209+
);
210+
211+
expect(screen.getByTestId("dropdown_search_input")).toHaveAccessibleName(
212+
"Search options...",
213+
);
214+
});
215+
});

‎src/frontend/src/components/core/dropdownComponent/components/DropdownSearchInput.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export function DropdownSearchInput({
2020
onChange={onSearch}
2121
onKeyDown={onKeyDown}
2222
placeholder={t("input.searchOptions")}
23+
aria-label={t("input.searchOptions")}
2324
className="flex h-9 w-full rounded-md bg-transparent py-3 text-[13px] outline-none placeholder:text-muted-foreground disabled:cursor-not-allowed disabled:opacity-50"
2425
autoComplete="off"
2526
data-testid="dropdown_search_input"

‎src/frontend/src/components/core/editFlowSettingsComponent/index.tsx‎

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -192,34 +192,38 @@ export const EditFlowSettings: React.FC<
192192
<Form.Message match="valueMissing" className="field-invalid">
193193
{t("flow.pleaseEnterDescription")}
194194
</Form.Message>
195-
<div className="mt-3">
196-
<div className="flex items-center gap-2">
197-
<div>
198-
<div className="flex items-center gap-2">
199-
<Form.Label className="text-mmd font-medium">
200-
{t("flow.lockFlow")}
201-
</Form.Label>
195+
{/* Callers that only display the flow (no setLocked) would otherwise
196+
render a focusable switch that cannot change anything. */}
197+
{setLocked && (
198+
<div className="mt-3">
199+
<div className="flex items-center gap-2">
200+
<div>
201+
<div className="flex items-center gap-2">
202+
<Form.Label className="text-mmd font-medium">
203+
{t("flow.lockFlow")}
204+
</Form.Label>
202205

203-
<ForwardedIconComponent
204-
name={locked ? "Lock" : "Unlock"}
205-
className="text-muted-foreground !w-5 !h-5"
206-
/>
206+
<ForwardedIconComponent
207+
name={locked ? "Lock" : "Unlock"}
208+
className="text-muted-foreground !w-5 !h-5"
209+
/>
210+
</div>
211+
212+
<p className="text-xs text-muted-foreground/70 mt-1 font-normal">
213+
{t("flow.lockFlowDescription")}
214+
</p>
207215
</div>
208216

209-
<p className="text-xs text-muted-foreground/70 mt-1 font-normal">
210-
{t("flow.lockFlowDescription")}
211-
</p>
217+
<Switch
218+
checked={!!locked}
219+
onCheckedChange={(v) => setLocked(v)}
220+
disabled={readOnly}
221+
className="data-[state=checked]:bg-primary ml-auto"
222+
data-testid="lock-flow-switch"
223+
/>
212224
</div>
213-
214-
<Switch
215-
checked={!!locked}
216-
onCheckedChange={(v) => setLocked?.(v)}
217-
disabled={readOnly}
218-
className="data-[state=checked]:bg-primary ml-auto"
219-
data-testid="lock-flow-switch"
220-
/>
221225
</div>
222-
</div>
226+
)}
223227
</Form.Field>
224228
</>
225229
);

‎src/frontend/src/components/core/parameterRenderComponent/components/inputComponent/components/popoverObject/index.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { PopoverAnchor } from "@radix-ui/react-popover";
22
import { useEffect } from "react";
3+
import { useTranslation } from "react-i18next";
34
import ForwardedIconComponent from "@/components/common/genericIconComponent";
45
import {
56
Command,
@@ -45,6 +46,7 @@ const CustomInputPopoverObject = ({
4546
showOptions,
4647
inspectionPanel,
4748
}) => {
49+
const { t } = useTranslation();
4850
const PopoverContentInput =
4951
editNode || inspectionPanel ? PopoverContent : PopoverContentWithoutPortal;
5052

@@ -124,6 +126,7 @@ const CustomInputPopoverObject = ({
124126
align="center"
125127
>
126128
<Command
129+
label={optionsPlaceholder || t("input.searchOptions")}
127130
filter={(value, search) => {
128131
if (
129132
value.toLowerCase().includes(search.toLowerCase()) ||

0 commit comments

Comments
 (0)