Skip to content

Commit 1c7a233

Browse files
Leos-Khaiclaude
andcommitted
fix(web): keep composer menu option ids unique and autocomplete honest
Advertise list autocomplete only when the editor is wired to a trigger menu, so the Settings font preview stops claiming one. Escape whitespace in option ids reversibly so paths that differ only by a space no longer share a DOM id, and cover the helper with a unit test. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
1 parent eb14878 commit 1c7a233

4 files changed

Lines changed: 34 additions & 14 deletions

File tree

‎apps/web/src/components/ComposerPromptEditor.tsx‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -893,9 +893,11 @@ interface ComposerPromptEditorProps {
893893
onPaste: React.ClipboardEventHandler<HTMLElement>;
894894
editorRef: React.RefObject<ComposerPromptEditorHandle | null>;
895895
/**
896-
* Listbox of the open trigger menu and its highlighted option. Wiring them
897-
* into aria-controls / aria-activedescendant lets screen readers follow the
898-
* menu while focus never leaves the editor.
896+
* Trigger-menu wiring for screen readers. `menuListboxId` is the listbox the
897+
* menus render into and, when present, advertises list autocomplete.
898+
* `menuActiveOptionId` is the highlighted option while that listbox is
899+
* rendered; it drives aria-activedescendant so the reader follows the menu
900+
* without focus ever leaving the editor.
899901
*/
900902
menuListboxId?: string | undefined;
901903
menuActiveOptionId?: string | undefined;
@@ -1994,8 +1996,8 @@ function ComposerPromptEditorInner({
19941996
)}
19951997
data-testid="composer-editor"
19961998
aria-placeholder={placeholder}
1997-
ariaAutoComplete="list"
1998-
ariaControls={menuListboxId}
1999+
ariaAutoComplete={menuListboxId ? "list" : undefined}
2000+
ariaControls={menuActiveOptionId ? menuListboxId : undefined}
19992001
ariaActiveDescendant={menuActiveOptionId}
20002002
placeholder={<span />}
20012003
onKeyDown={(event) => {

‎apps/web/src/components/chat/ChatComposer.tsx‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2467,13 +2467,14 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps)
24672467

24682468
const isComposerApprovalState = activePendingApproval !== null;
24692469

2470-
// The editor only references the listbox while it is actually rendered, so
2471-
// aria-controls and aria-activedescendant never point at missing nodes.
2470+
// The editor only references an option while the listbox is actually
2471+
// rendered, so aria-activedescendant never points at a missing node.
24722472
const composerMenuListboxId = useId();
2473-
const composerMenuListboxRendered =
2474-
composerMenuOpen && !isComposerApprovalState && composerMenuItems.length > 0;
24752473
const composerMenuActiveOptionDomId =
2476-
composerMenuListboxRendered && activeComposerMenuItem
2474+
composerMenuOpen &&
2475+
!isComposerApprovalState &&
2476+
composerMenuItems.length > 0 &&
2477+
activeComposerMenuItem
24772478
? composerCommandOptionDomId(composerMenuListboxId, activeComposerMenuItem.id)
24782479
: undefined;
24792480

@@ -6655,7 +6656,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps)
66556656
onChange={onPromptChange}
66566657
onVisibleSelectionChange={expandComposerForEditorChange}
66576658
onCommandKeyDown={onComposerCommandKey}
6658-
menuListboxId={composerMenuListboxRendered ? composerMenuListboxId : undefined}
6659+
menuListboxId={composerMenuListboxId}
66596660
menuActiveOptionId={composerMenuActiveOptionDomId}
66606661
onPageScrollKeyDown={onPageScrollKeyDown}
66616662
onPageScrollKeyUp={onPageScrollKeyUp}

‎apps/web/src/components/chat/ComposerCommandMenu.test.tsx‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,19 @@ import { renderToStaticMarkup } from "react-dom/server";
22
import { ProviderDriverKind } from "@t3tools/contracts";
33
import { describe, expect, it } from "vite-plus/test";
44

5-
import { ComposerCommandMenu } from "./ComposerCommandMenu";
5+
import { ComposerCommandMenu, composerCommandOptionDomId } from "./ComposerCommandMenu";
6+
7+
describe("composerCommandOptionDomId", () => {
8+
it("produces a whitespace-free id that stays distinct across near-identical paths", () => {
9+
const spaced = composerCommandOptionDomId("menu", "path:file:docs/my file.md");
10+
const underscored = composerCommandOptionDomId("menu", "path:file:docs/my_file.md");
11+
const escaped = composerCommandOptionDomId("menu", "path:file:docs/my%0020file.md");
12+
13+
expect(spaced).not.toMatch(/\s/);
14+
expect(new Set([spaced, underscored, escaped]).size).toBe(3);
15+
expect(composerCommandOptionDomId("menu", "slash:model")).toBe("menu-slash:model");
16+
});
17+
});
618

719
describe("ComposerCommandMenu", () => {
820
it("renders slash commands with their descriptions", () => {

‎apps/web/src/components/chat/ComposerCommandMenu.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,15 @@ export type ComposerCommandItem =
7070

7171
/**
7272
* DOM id of a menu option, so the editor's aria-activedescendant can point at
73-
* it. Item ids embed paths and may contain whitespace, which an IDREF cannot.
73+
* it. Item ids embed paths and may contain whitespace, which an IDREF cannot;
74+
* the fixed-width escape keeps ids that differ only there distinct.
7475
*/
7576
export function composerCommandOptionDomId(listboxId: string, itemId: string): string {
76-
return `${listboxId}-${itemId.replace(/\s/g, "_")}`;
77+
const escaped = itemId.replace(
78+
/[\s%]/g,
79+
(char) => `%${char.charCodeAt(0).toString(16).padStart(4, "0")}`,
80+
);
81+
return `${listboxId}-${escaped}`;
7782
}
7883

7984
const LISTBOX_LABEL_BY_TRIGGER: Record<ComposerTriggerKind, string> = {

0 commit comments

Comments
 (0)