From dcae1cfdde85155acc7cde1de858a1438bf737ad Mon Sep 17 00:00:00 2001 From: Xubin Ren <52506698+Re-bin@users.noreply.github.com> Date: Tue, 4 Aug 2026 15:52:07 +0800 Subject: [PATCH] fix(webui): correct combobox navigation semantics --- .../src/components/settings/SettingsView.tsx | 34 ++++++----- webui/src/components/ui/combobox.tsx | 30 ++++------ webui/src/tests/combobox.test.tsx | 59 +++++++++++++++++-- 3 files changed, 86 insertions(+), 37 deletions(-) diff --git a/webui/src/components/settings/SettingsView.tsx b/webui/src/components/settings/SettingsView.tsx index bcf52954d..38c46d24e 100644 --- a/webui/src/components/settings/SettingsView.tsx +++ b/webui/src/components/settings/SettingsView.tsx @@ -8955,14 +8955,14 @@ function TimezonePicker({ /> -
- {filteredOptions.length ? ( - filteredOptions.map((option) => { + {filteredOptions.length ? ( +
+ {filteredOptions.map((option) => { const selected = option.name === value; return ( ); - }) - ) : ( -
- {tx("settings.timezone.empty", "No matching timezones.")} -
- )} -
+ })} +
+ ) : ( +
+ {tx("settings.timezone.empty", "No matching timezones.")} +
+ )} ); diff --git a/webui/src/components/ui/combobox.tsx b/webui/src/components/ui/combobox.tsx index 8f78aeebf..8ba60b804 100644 --- a/webui/src/components/ui/combobox.tsx +++ b/webui/src/components/ui/combobox.tsx @@ -25,7 +25,10 @@ export function useComboboxNavigation({ const [activeValue, setActiveValue] = React.useState(null); React.useEffect(() => { - if (!open) return; + if (!open) { + setActiveValue(null); + return; + } setActiveValue((current) => { if (current && values.includes(current)) return current; if (selectedValue && values.includes(selectedValue)) return selectedValue; @@ -54,23 +57,15 @@ export function useComboboxNavigation({ if (event.nativeEvent.isComposing) return; switch (event.key) { case "ArrowDown": - event.preventDefault(); - move(1); - break; - case "ArrowUp": - event.preventDefault(); - move(-1); - break; - case "Home": if (values.length) { event.preventDefault(); - setActiveValue(values[0]); + move(1); } break; - case "End": + case "ArrowUp": if (values.length) { event.preventDefault(); - setActiveValue(values[values.length - 1]); + move(-1); } break; case "Enter": @@ -86,12 +81,13 @@ export function useComboboxNavigation({ } }; + const expanded = open && values.length > 0; const inputProps = { role: "combobox" as const, "aria-autocomplete": "list" as const, - "aria-controls": listboxId, - "aria-expanded": open, - "aria-activedescendant": activeOptionId, + "aria-controls": expanded ? listboxId : undefined, + "aria-expanded": expanded, + "aria-activedescendant": expanded ? activeOptionId : undefined, onKeyDown: onInputKeyDown, }; @@ -105,7 +101,7 @@ export function useComboboxNavigation({ return { id: `${listboxId}-option-${index}`, role: "option" as const, - "aria-selected": value === selectedValue, + "aria-selected": value === activeValue, "data-highlighted": value === activeValue ? "" : undefined, tabIndex: -1, onPointerMove: () => setActiveValue(value), @@ -126,7 +122,7 @@ const ComboboxOption = React.forwardRef< className={cn( floatingItemClassName, floatingItemFocusClassName, - "w-full cursor-default text-left data-[highlighted]:bg-muted/85 data-[highlighted]:text-foreground aria-selected:bg-muted/80", + "w-full cursor-default text-left data-[highlighted]:bg-muted/85 data-[highlighted]:text-foreground", className, )} {...props} diff --git a/webui/src/tests/combobox.test.tsx b/webui/src/tests/combobox.test.tsx index 40a0266dd..44820a905 100644 --- a/webui/src/tests/combobox.test.tsx +++ b/webui/src/tests/combobox.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen } from "@testing-library/react"; +import { createEvent, fireEvent, render, screen } from "@testing-library/react"; import { useState } from "react"; import { describe, expect, it } from "vitest"; @@ -9,12 +9,12 @@ import { const OPTIONS = ["Alpha", "Beta", "Gamma"]; -function ComboboxHarness() { +function ComboboxHarness({ options = OPTIONS }: { options?: readonly string[] }) { const [open, setOpen] = useState(true); const [selected, setSelected] = useState("Beta"); const navigation = useComboboxNavigation({ open, - values: OPTIONS, + values: options, selectedValue: selected, onSelect: setSelected, onClose: () => setOpen(false), @@ -23,9 +23,12 @@ function ComboboxHarness() { return ( <> - {open ? ( + + {open && options.length ? (
- {OPTIONS.map((option) => ( + {options.map((option) => ( {option} @@ -49,6 +52,14 @@ describe("combobox navigation", () => { ); fireEvent.keyDown(input, { key: "ArrowDown" }); + expect(screen.getByRole("option", { name: "Beta" })).toHaveAttribute( + "aria-selected", + "false", + ); + expect(screen.getByRole("option", { name: "Gamma" })).toHaveAttribute( + "aria-selected", + "true", + ); expect(input).toHaveAttribute( "aria-activedescendant", screen.getByRole("option", { name: "Gamma" }).id, @@ -58,6 +69,44 @@ describe("combobox navigation", () => { expect(screen.getByRole("status", { name: "Selection" })).toHaveTextContent("Gamma"); }); + it("preserves native text editing keys", () => { + render(); + + const input = screen.getByRole("combobox", { name: "Options" }); + for (const key of ["Home", "End"]) { + const event = createEvent.keyDown(input, { key }); + fireEvent(input, event); + expect(event.defaultPrevented).toBe(false); + } + }); + + it("restores the selected option after closing without a selection", () => { + render(); + + const input = screen.getByRole("combobox", { name: "Options" }); + fireEvent.keyDown(input, { key: "ArrowDown" }); + fireEvent.keyDown(input, { key: "Escape" }); + fireEvent.click(screen.getByRole("button", { name: "Open options" })); + + const selectedOption = screen.getByRole("option", { name: "Beta" }); + expect(input).toHaveAttribute("aria-activedescendant", selectedOption.id); + fireEvent.keyDown(input, { key: "Enter" }); + expect(screen.getByRole("status", { name: "Selection" })).toHaveTextContent("Beta"); + }); + + it("collapses the combobox when no options are available", () => { + render(); + + const input = screen.getByRole("combobox", { name: "Options" }); + expect(input).toHaveAttribute("aria-expanded", "false"); + expect(input).not.toHaveAttribute("aria-controls"); + expect(input).not.toHaveAttribute("aria-activedescendant"); + + const event = createEvent.keyDown(input, { key: "ArrowDown" }); + fireEvent(input, event); + expect(event.defaultPrevented).toBe(false); + }); + it("closes the listbox on Escape", () => { render();