Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { describe, expect, it, vi } from "vitest";
import { render, screen } from "@testing-library/react";

/**
* A11Y-11 regression guard (shared picker panel half).
*
* SearchablePickerPanel backs five of the six chat pickers (expert, knowledge,
* skill, subagent, connector), which own no list markup themselves. Before this
* fix it had zero ARIA: the search box was named only by a placeholder, and
* filtering was completely silent.
*
* These panels are MULTI-SELECT TOGGLES inside a popover, not comboboxes. That
* is why the list is role="group" with aria-pressed on the items rather than
* role="listbox" with aria-selected: the rows contain real <button>s, and an
* option may not contain interactive descendants.
*/

vi.mock("react-i18next", () => ({
useTranslation: () => ({
// Resolve to the fallback, as the shared test setup does, so assertions
// read the literal string rather than a locale lookup.
t: (_key: string, fallback?: string, options?: { count?: number }) =>
fallback === undefined
? _key
: fallback.replace("{{count}}", String(options?.count ?? "")),
}),
}));

import SearchablePickerPanel from "./SearchablePickerPanel";

interface Row {
id: string;
name: string;
}

const rows: Row[] = [
{ id: "a", name: "Alpha" },
{ id: "b", name: "Beta" },
];

function setup(
overrides: Partial<Parameters<typeof SearchablePickerPanel<Row>>[0]> = {},
) {
return render(
<SearchablePickerPanel<Row>
items={rows}
filterFn={(item, query) =>
item.name.toLowerCase().includes(query.toLowerCase())
}
searchPlaceholder="Search things"
ariaLabel="Things"
emptyMessage="Nothing here"
renderItem={(item) => (
<button type="button" aria-pressed={item.id === "a"}>
{item.name}
</button>
)}
footerIcon={null}
footerLabel="Manage"
onFooterClick={vi.fn()}
{...overrides}
/>,
);
}

describe("searchable picker panel (A11Y-11)", () => {
it("gives the search field an accessible name, not just a placeholder", () => {
setup();
// The placeholder disappears on first keystroke and is not a name.
expect(screen.getByRole("searchbox")).toHaveAccessibleName("Things");
});

it("exposes the list as a labelled group", () => {
setup();
expect(screen.getByRole("group")).toHaveAccessibleName("Things");
});

it("announces the result count through a live region", () => {
// Filtering was previously silent: the list could shrink to nothing with no
// announcement at all.
setup();
const status = screen.getByRole("status");
expect(status).toHaveAttribute("aria-live", "polite");
expect(status).toHaveTextContent("2 results");
});

it("keeps the live region present even when the filter matches nothing", () => {
// An aria-live region must exist BEFORE its content changes, or the update
// is not announced. Rendering it only in the non-empty branch would break
// exactly the case that needs announcing most.
setup({ items: [] });
expect(screen.getByRole("status")).toHaveTextContent("0 results");
});

it("still renders the items and the empty state", () => {
setup();
expect(screen.getAllByRole("button")).toHaveLength(3); // 2 rows + footer
expect(screen.getByText("Alpha")).toBeInTheDocument();

setup({ items: [] });
expect(screen.getByText("Nothing here")).toBeInTheDocument();
});

it("carries aria-pressed through from renderItem", () => {
// The panel does not own toggle state - callers pass it in renderItem - so
// this pins that the panel does not strip it.
setup();
expect(screen.getByRole("button", { name: "Alpha" })).toHaveAttribute(
"aria-pressed",
"true",
);
expect(screen.getByRole("button", { name: "Beta" })).toHaveAttribute(
"aria-pressed",
"false",
);
});
});
33 changes: 32 additions & 1 deletion dashboard/src/components/ChatPicker/SearchablePickerPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { ReactNode } from "react";
import { useTranslation } from "react-i18next";
import { Search } from "lucide-react";
import { useFilteredList } from "../../hooks/useFilteredList";
import styles from "./picker.module.less";
Expand All @@ -22,6 +23,13 @@ interface SearchablePickerPanelProps<T> {
footerMuted?: boolean;
/** Optional row(s) rendered between the list and the primary footer. */
beforeFooter?: ReactNode;
/**
* Names the search field and the option group. These panels are multi-select
* toggles inside a popover, so their items are buttons with aria-pressed
* rather than listbox options — the label is what tells a screen reader what
* the set of toggles controls.
*/
ariaLabel: string;
}

export default function SearchablePickerPanel<T>({
Expand All @@ -38,7 +46,9 @@ export default function SearchablePickerPanel<T>({
onFooterClick,
footerMuted = false,
beforeFooter,
ariaLabel,
}: SearchablePickerPanelProps<T>) {
const { t } = useTranslation();
const { query, setQuery, filtered } = useFilteredList(items, filterFn);
const panelClass =
width === "compact"
Expand All @@ -53,14 +63,35 @@ export default function SearchablePickerPanel<T>({
<input
type="search"
className={styles.searchInput}
// The placeholder is not an accessible name: it disappears once the
// user types, and some AT ignores it entirely.
aria-label={ariaLabel}
placeholder={searchPlaceholder}
value={query}
onChange={(e) => setQuery(e.target.value)}
/>
<Search size={15} className={styles.searchIcon} aria-hidden />
</div>

<div className={styles.list} data-picker-list="">
{/* Filtering is visually obvious and was completely silent to assistive
tech: the list can shrink to empty with no announcement. */}
<div
className={styles.searchStatus}
role="status"
aria-live="polite"
data-picker-status=""
>
{t("chat.pickerResultCount", "{{count}} results", {
count: filtered.length,
})}
</div>

<div
className={styles.list}
data-picker-list=""
role="group"
aria-label={ariaLabel}
>
{filtered.length === 0 ? (
<div className={styles.empty}>{emptyMessage}</div>
) : (
Expand Down
14 changes: 14 additions & 0 deletions dashboard/src/components/ChatPicker/picker.module.less
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,20 @@
pointer-events: none;
}

/* Visually hidden: the result count is announced, not shown. Filtering already
has a visible signal (the list shrinks), so this must not alter layout. */
.searchStatus {
position: absolute;
width: 1px;
height: 1px;
margin: -1px;
padding: 0;
overflow: hidden;
clip: rect(0, 0, 0, 0);
white-space: nowrap;
border: 0;
}

.list {
max-height: 320px;
overflow-y: auto;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ describe("ChatInput prefill clear-on-send", () => {
/>,
);

const textarea = screen.getByRole("textbox") as HTMLTextAreaElement;
const textarea = screen.getByRole("combobox") as HTMLTextAreaElement;
const prefill = "请填写收件人:";

act(() => {
Expand Down Expand Up @@ -214,7 +214,7 @@ describe("ChatInput prefill clear-on-send", () => {
/>,
);

const textarea = screen.getByRole("textbox") as HTMLTextAreaElement;
const textarea = screen.getByRole("combobox") as HTMLTextAreaElement;
const prefill = "请填写收件人:";
parentPrefill = prefill;

Expand Down Expand Up @@ -260,7 +260,7 @@ describe("ChatInput prefill clear-on-send", () => {
};
const { rerender } = render(<ChatInput {...props} agentId="agent-1" />);

const textarea = screen.getByRole("textbox") as HTMLTextAreaElement;
const textarea = screen.getByRole("combobox") as HTMLTextAreaElement;
expect(textarea.value).toBe("previous expert prompt");

rerender(<ChatInput {...props} agentId="agent-2" />);
Expand All @@ -283,7 +283,7 @@ describe("ChatInput prefill clear-on-send", () => {
threadId="thread-1"
/>,
);
const textarea = screen.getByRole("textbox") as HTMLTextAreaElement;
const textarea = screen.getByRole("combobox") as HTMLTextAreaElement;
fireEvent.change(textarea, { target: { value: "second question" } });
fireEvent.click(screen.getByRole("button", { name: "send" }));
expect(onSend).toHaveBeenCalledWith("second question", undefined);
Expand Down
20 changes: 19 additions & 1 deletion dashboard/src/pages/Chat/components/ChatInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ import type { KnowledgeBase } from "../../../api/modules/knowledgeBases";
import type { SkillSpec } from "../../Agent/Skills/useSkills";
import type { ChatAgentOption } from "./ExpertAgentAvatar";
import type { AgentSubagentSummary } from "../../../api/modules/subagents";
import MentionPickerMenu from "./MentionPickerMenu";
import MentionPickerMenu, {
MENTION_LISTBOX_ID,
mentionOptionId,
} from "./MentionPickerMenu";
import ChatInputPreviewBar from "./ChatInputPreviewBar";
import ChatInputActionsRow from "./ChatInputActionsRow";
import ChatQueuedMessages from "./ChatQueuedMessages";
Expand Down Expand Up @@ -801,6 +804,21 @@ const ChatInput = forwardRef<ChatInputHandle, ChatInputProps>(
<textarea
ref={textareaRef}
className={styles.textarea}
// Combobox semantics: the textarea owns the popup, and
// aria-activedescendant names the option the arrow keys have
// highlighted, so the highlighted item is announced while focus
// stays in the input. Without this the menu was a visually-only
// affordance — 0 listbox/combobox roles existed in pages/Chat.
role="combobox"
// A combobox must have a name. The placeholder is not one: it
// disappears once the user types, and some AT ignores it.
aria-label={t("chatWelcome.inputPlaceholder", "Message Octop...")}
aria-expanded={mentionMenuOpen}
aria-controls={mentionMenuOpen ? MENTION_LISTBOX_ID : undefined}
aria-activedescendant={
mentionMenuOpen ? mentionOptionId(mentionMenuIndex) : undefined
}
aria-autocomplete="list"
value={text}
onChange={(e) => {
userHasEditedRef.current = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export default function ConnectorPickerPopover({
items={connectors}
filterFn={filterFn}
searchPlaceholder={t("connectors.chatPickerSearch")}
ariaLabel={t("connectors.chatPickerSearch")}
emptyMessage={t("connectors.chatEmptyHint")}
width="narrow"
footerIcon={<Settings2 size={15} aria-hidden />}
Expand Down
8 changes: 8 additions & 0 deletions dashboard/src/pages/Chat/components/ExpertPickerPopover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ export default function ExpertPickerPopover({
? t("chat.expertPickerHiddenSearch")
: t("chat.expertPickerSearch")
}
ariaLabel={
viewingHidden
? t("chat.expertPickerHiddenSearch")
: t("chat.expertPickerSearch")
}
emptyMessage={
viewingHidden
? t("chat.expertPickerHiddenEmpty")
Expand Down Expand Up @@ -151,6 +156,9 @@ export default function ExpertPickerPopover({
<button
type="button"
className={styles.expertPickerSelect}
// Selection was CSS-only, so which experts are active was
// invisible to assistive tech.
aria-pressed={active}
onClick={() => onSelect(agent)}
>
<ExpertAgentAvatar
Expand Down
25 changes: 25 additions & 0 deletions dashboard/src/pages/Chat/components/HitlPolicyPicker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,31 @@ describe("HitlPolicyPicker", () => {
expect(onChange).toHaveBeenCalledWith({ mode: "ask" });
});

it("announces which approval mode is in force (A11Y-11)", async () => {
// The active mode was carried by a CSS class alone, so a screen reader
// could not tell "ask" from "allow all".
render(
<HitlPolicyPicker policy={{ mode: "allow_all" }} onChange={vi.fn()} />,
);
fireEvent.click(screen.getByTestId("hitl-policy-picker"));

const allowAll = await screen.findByText("chat.hitl.policy.allowAll");
const allowAllBtn = allowAll.closest("button");
expect(allowAllBtn).toHaveAttribute("aria-pressed", "true");

const ask = await screen.findByText("chat.hitl.policy.ask");
expect(ask.closest("button")).toHaveAttribute("aria-pressed", "false");
});

it("groups the mode buttons under a label (A11Y-11)", async () => {
render(<HitlPolicyPicker policy={{ mode: "ask" }} onChange={vi.fn()} />);
fireEvent.click(screen.getByTestId("hitl-policy-picker"));
// find the group by its label to prove the relationship resolves
expect(
await screen.findByRole("group", { name: "chat.hitl.policy.picker" }),
).toBeInTheDocument();
});

it("can remove one allowed tool without closing the menu", async () => {
const onChange = vi.fn();
render(
Expand Down
10 changes: 9 additions & 1 deletion dashboard/src/pages/Chat/components/HitlPolicyPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,19 @@ export default function HitlPolicyPicker({
onOpenChange={setOpen}
overlayClassName={styles.modelPopover}
content={
<div className={styles.modeMenu}>
<div
className={styles.modeMenu}
role="group"
aria-label={t("chat.hitl.policy.picker")}
>
<button
type="button"
className={`${styles.modeMenuItem} ${
policy.mode === "ask" ? styles.modeMenuItemActive : ""
}`}
// The active policy was carried by a CSS class alone, so a screen
// reader could not tell which mode is in force.
aria-pressed={policy.mode === "ask"}
onClick={() => {
onChange({ mode: "ask" });
setOpen(false);
Expand All @@ -63,6 +70,7 @@ export default function HitlPolicyPicker({
className={`${styles.modeMenuItem} ${
policy.mode === "allow_all" ? styles.modeMenuItemActive : ""
}`}
aria-pressed={policy.mode === "allow_all"}
onClick={() => {
onChange({ mode: "allow_all" });
setOpen(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export default function KnowledgePickerPopover({
items={knowledgeBases}
filterFn={filterFn}
searchPlaceholder={t("chat.knowledgePickerSearch")}
ariaLabel={t("chat.knowledgePickerSearch")}
emptyMessage={t("chat.knowledgePickerEmpty")}
width="narrow"
footerIcon={
Expand Down
Loading