mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
fix: give buttons accessible names (#30796)
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 <member>`. 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.
<details>
<summary>Implementation notes and review decisions</summary>
**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`.
</details>
---
_Generated by Coder Agents on behalf of @aqandrew._
This commit is contained in:
@@ -157,6 +157,7 @@ export const AgentRow: React.FC<AgentRowProps> = ({
|
||||
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<AgentRowProps> = ({
|
||||
) : (
|
||||
<CopyIcon />
|
||||
)}
|
||||
<span className="sr-only">{copyLogsLabel}</span>
|
||||
</Button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent>
|
||||
{showCopiedSuccess
|
||||
? "Copied!"
|
||||
: "Copy selected logs"}
|
||||
{showCopiedSuccess ? "Copied!" : copyLogsLabel}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
</TooltipProvider>
|
||||
|
||||
@@ -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(
|
||||
<WorkspaceSharingForm
|
||||
organizationId={MockWorkspace.organization_id}
|
||||
workspaceACL={mockWorkspaceACL}
|
||||
canShareWorkspace
|
||||
error={undefined}
|
||||
onUpdateUser={onUpdateUser}
|
||||
updatingUserId={undefined}
|
||||
onRemoveUser={vi.fn()}
|
||||
onUpdateGroup={onUpdateGroup}
|
||||
onRemoveGroup={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
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(<RoleSelectField value="use" onChange={onChange} />);
|
||||
|
||||
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");
|
||||
});
|
||||
});
|
||||
@@ -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<RoleSelectProps> = ({
|
||||
value,
|
||||
memberOrGroupName,
|
||||
disabled,
|
||||
onValueChange,
|
||||
}) => {
|
||||
@@ -58,7 +60,10 @@ const RoleSelect: React.FC<RoleSelectProps> = ({
|
||||
|
||||
return (
|
||||
<Select value={value} onValueChange={onValueChange} disabled={disabled}>
|
||||
<SelectTrigger className="w-40 h-auto">
|
||||
<SelectTrigger
|
||||
className="w-40 h-auto"
|
||||
aria-label={`Role for ${memberOrGroupName}`}
|
||||
>
|
||||
<SelectValue>
|
||||
<span className="bg-surface-secondary rounded-md px-3 py-0.5 inline-block">
|
||||
{roleLabels[value]}
|
||||
@@ -128,7 +133,7 @@ export const RoleSelectField: React.FC<RoleSelectFieldProps> = ({
|
||||
onValueChange={(val: WorkspaceRole) => onChange(val)}
|
||||
disabled={disabled}
|
||||
>
|
||||
<SelectTrigger className="w-40">
|
||||
<SelectTrigger className="w-40" aria-label="Role">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
@@ -259,6 +264,7 @@ export const WorkspaceSharingForm: React.FC<WorkspaceSharingFormProps> = ({
|
||||
{canShareWorkspace ? (
|
||||
<RoleSelect
|
||||
value={group.role}
|
||||
memberOrGroupName={group.display_name || group.name}
|
||||
disabled={updatingGroupId === group.id}
|
||||
onValueChange={(value) => onUpdateGroup(group, value)}
|
||||
/>
|
||||
@@ -307,6 +313,7 @@ export const WorkspaceSharingForm: React.FC<WorkspaceSharingFormProps> = ({
|
||||
{canShareWorkspace ? (
|
||||
<RoleSelect
|
||||
value={user.role}
|
||||
memberOrGroupName={user.username}
|
||||
disabled={updatingUserId === user.id}
|
||||
onValueChange={(value) => onUpdateUser(user, value)}
|
||||
/>
|
||||
|
||||
Reference in New Issue
Block a user