From 1d57682dab7f6272b8abfd874c5ff7ecdf2feeca Mon Sep 17 00:00:00 2001 From: Andrew Aquino Date: Wed, 14 Oct 2026 15:58:33 -0700 Subject: [PATCH] fix: give buttons accessible names (#30796) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the axe `button-name` (WCAG 4.1.2) findings from the Deque audit: - fixes DEQ-431 - fixes DEQ-456 - fixes DEQ-101 (duplicate of DEQ-456) - fixes DEQ-138 affected buttons: - **Workspace sharing role selects** (DEQ-431, DEQ-456, DEQ-101): Radix Select triggers render `role="combobox"`, and ARIA doesn't let a combobox take its name from its content. That's why the visible "Use"/"Admin" text didn't count. Each member row's select is now named `Role for `. The add-member form's select is named `Role`. - **Agent logs "Copy selected logs" button** (DEQ-138): the icon-only button had a name only in its tooltip. It now has `sr-only` text, matching the download button next to it. Adds Vitest coverage in `WorkspaceSharingForm.test.tsx`. The test finds each select by its accessible name and checks that changing the role calls the update callback. It fails without the fix.
Implementation notes and review decisions **Mapping axe selectors to components** - DEQ-431: `form > div > button:nth-child(2)` is `RoleSelectField` in `AddWorkspaceUserOrGroup`. - DEQ-456 / DEQ-101: `table > tbody > tr > td:nth-child(2) > button` is `RoleSelect` in `WorkspaceSharingForm`. - DEQ-138: `section:nth-child(3)` (agent card logs section) `> … > div:nth-child(2) > button:nth-child(1)` is the copy button in the `AgentRow` logs toolbar. `data-state="closed"` with no `aria-expanded` means it's a Tooltip trigger. **Justified findings from the frontend review (`frontend-deep-review` and `frontend-review`)** - FE1/TS5, no Vitest for the copy button: rendering `AgentRow` with streamed logs needs the proxy context and websocket log fixtures. The change only adds an `sr-only` label. I removed a story `play` assertion that I'd first added, because FE1 bans assertions in `play` functions. - FE6, label-in-name (WCAG 2.5.3): the combobox's visible text is its *value* ("Use"/"Admin"), not its label. The visible label is the "Role" column header, and the accessible names include it. Screen readers still announce the value from the trigger's content. - Not changed: the copy button's tooltip switches to "Copied!" but screen readers hear no announcement. That was already the case before this PR, and it's a different WCAG criterion (4.1.3), so it's out of scope here. **Checks run:** `pnpm check`, `pnpm lint:types`, `pnpm lint`, the unit tests, the `AgentRow` / `WorkspaceSharingPage` / `WorkspaceActions` story tests, and `make pre-commit`.
--- _Generated by Coder Agents on behalf of @aqandrew._ --- site/src/modules/resources/AgentRow.tsx | 6 +- .../WorkspaceSharingForm.test.tsx | 89 +++++++++++++++++++ .../WorkspaceSharingForm.tsx | 11 ++- 3 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 site/src/modules/workspaces/WorkspaceSharingForm/WorkspaceSharingForm.test.tsx diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index f28c9a1670..af4968215b 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -157,6 +157,7 @@ export const AgentRow: React.FC = ({ agent.display_apps.includes("vscode") || agent.display_apps.includes("vscode_insiders"); const showVSCode = hasVSCodeApp && !browser_only; + const copyLogsLabel = "Copy selected logs"; const hasStartupFeatures = Boolean(agent.logs_length); const runningScriptsCount = agent.scripts.filter( @@ -720,12 +721,11 @@ export const AgentRow: React.FC = ({ ) : ( )} + {copyLogsLabel} - {showCopiedSuccess - ? "Copied!" - : "Copy selected logs"} + {showCopiedSuccess ? "Copied!" : copyLogsLabel} diff --git a/site/src/modules/workspaces/WorkspaceSharingForm/WorkspaceSharingForm.test.tsx b/site/src/modules/workspaces/WorkspaceSharingForm/WorkspaceSharingForm.test.tsx new file mode 100644 index 0000000000..31da80e2bf --- /dev/null +++ b/site/src/modules/workspaces/WorkspaceSharingForm/WorkspaceSharingForm.test.tsx @@ -0,0 +1,89 @@ +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { API } from "#/api/api"; +import type { WorkspaceACL } from "#/api/typesGenerated"; +import { + MockGroup, + MockUserMember, + MockWorkspace, +} from "#/testHelpers/entities"; +import { render } from "#/testHelpers/renderHelpers"; +import { RoleSelectField, WorkspaceSharingForm } from "./WorkspaceSharingForm"; + +const mockWorkspaceACL: WorkspaceACL = { + users: [ + { + id: MockUserMember.id, + username: MockUserMember.username, + name: MockUserMember.name, + avatar_url: MockUserMember.avatar_url, + role: "use", + }, + ], + group: [{ ...MockGroup, role: "use" }], +}; + +describe("WorkspaceSharingForm", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("names each member's role select after the member", async () => { + vi.spyOn(API, "getWorkspaceSharingSettings").mockResolvedValue({ + sharing_disabled: false, + sharing_globally_disabled: false, + shareable_workspace_owners: "everyone", + }); + const onUpdateUser = vi.fn(); + const onUpdateGroup = vi.fn(); + + render( + , + ); + + const user = userEvent.setup(); + await user.click( + await screen.findByRole("combobox", { + name: `Role for ${MockUserMember.username}`, + }), + ); + await user.click(await screen.findByRole("option", { name: /Admin/ })); + expect(onUpdateUser).toHaveBeenCalledWith( + mockWorkspaceACL.users[0], + "admin", + ); + + await user.click( + screen.getByRole("combobox", { + name: `Role for ${MockGroup.display_name || MockGroup.name}`, + }), + ); + await user.click(await screen.findByRole("option", { name: /Admin/ })); + expect(onUpdateGroup).toHaveBeenCalledWith( + mockWorkspaceACL.group[0], + "admin", + ); + }); +}); + +describe("RoleSelectField", () => { + it("has an accessible name", async () => { + const onChange = vi.fn(); + render(); + + const user = userEvent.setup(); + await user.click(await screen.findByRole("combobox", { name: "Role" })); + await user.click(await screen.findByRole("option", { name: "Admin" })); + expect(onChange).toHaveBeenCalledWith("admin"); + }); +}); diff --git a/site/src/modules/workspaces/WorkspaceSharingForm/WorkspaceSharingForm.tsx b/site/src/modules/workspaces/WorkspaceSharingForm/WorkspaceSharingForm.tsx index c2aea94afc..dd42f06546 100644 --- a/site/src/modules/workspaces/WorkspaceSharingForm/WorkspaceSharingForm.tsx +++ b/site/src/modules/workspaces/WorkspaceSharingForm/WorkspaceSharingForm.tsx @@ -41,12 +41,14 @@ import { getGroupSubtitle } from "#/modules/groups"; type RoleSelectProps = { value: WorkspaceRole; + memberOrGroupName: string; disabled?: boolean; onValueChange: (value: WorkspaceRole) => void; }; const RoleSelect: React.FC = ({ value, + memberOrGroupName, disabled, onValueChange, }) => { @@ -58,7 +60,10 @@ const RoleSelect: React.FC = ({ return (