diff --git a/CHANGELOG.md b/CHANGELOG.md index 392f4abbe..93b9009c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p - for those alert intent states the content area gets an `id` and is referred by the dialog via `aria-describedby` - explicitly given values are never overwritten - if neither `title`, `aria-label` nor `aria-labelledby` is given for an alert, then the `intent` level is used as fallback for `aria-label`, so the alert dialog always has an accessible name +- `` + - Restore keyboard navigation for non-filterable Select. + - Restore predictable Enter, arrow key, Escape, and Tab behavior, with a visible focus indicator. + - Keep focus on the combobox and expose the active option to screen readers. ### Deprecated diff --git a/src/components/Form/FieldItem.tsx b/src/components/Form/FieldItem.tsx index 338b36158..2e21b255c 100644 --- a/src/components/Form/FieldItem.tsx +++ b/src/components/Form/FieldItem.tsx @@ -71,6 +71,7 @@ export const FieldItem = ({ ...otherProps }: FieldItemProps) => { const fieldItemRef = React.useRef(null); + const hiddenSelectButtonRef = React.useRef(null); /** unique ID of this field item, used as suffix for the IDs of its parts */ const fieldItemId = React.useId().replace(/[^a-zA-Z0-9_-]/g, ""); @@ -99,6 +100,31 @@ export const FieldItem = ({ connectableInputSelectors.map((selector) => `.${eccgui}-fielditem__inputfields ${selector}`).join(", "), ), ); + /** + * Blueprint renders a non-filterable Select as a trigger button inside a popover target with + * role="combobox". Our Select makes that outer target the keyboard focus stop, while the + * normal input selector above still finds the inner button. The inner button remains a + * pointer target, but exposing both elements as labelled controls can cause the same field + * to be announced twice. Hide the inner button from assistive technology and connect the + * label, helper text and message directly to the focusable combobox. + * + * Filterable Selects put the combobox role on their popup search input instead, and do not + * match this selector. Keeping the non-filterable case here lets consumers use FieldItem + * without repeating these wrapper-specific ARIA connections at every Select usage. + */ + const selectCombobox = ownPart( + fieldItem.querySelectorAll( + `.${eccgui}-fielditem__inputfields .${eccgui}-select[role="combobox"]`, + ), + ); + if (hiddenSelectButtonRef.current && (!selectCombobox || hiddenSelectButtonRef.current !== inputElement)) { + hiddenSelectButtonRef.current.removeAttribute("aria-hidden"); + hiddenSelectButtonRef.current = null; + } + if (selectCombobox && inputElement && inputElement.getAttribute("aria-hidden") !== "true") { + inputElement.setAttribute("aria-hidden", "true"); + hiddenSelectButtonRef.current = inputElement; + } const helpElement = ownPart(fieldItem.querySelectorAll(`.${eccgui}-fielditem__helpertext`)); const messageElement = ownPart(fieldItem.querySelectorAll(`.${eccgui}-fielditem__message`)); @@ -120,8 +146,12 @@ export const FieldItem = ({ * Update a list of ID references, only the IDs created by this field item are removed if their part is gone. * References set by the using application always stay untouched. */ - const updateReferences = (attribute: string, parts: [HTMLElement | undefined, string][]) => { - const references = (inputElement.getAttribute(attribute) ?? "").split(" ").filter(Boolean); + const updateReferences = ( + element: HTMLElement, + attribute: string, + parts: [HTMLElement | undefined, string][], + ) => { + const references = (element.getAttribute(attribute) ?? "").split(" ").filter(Boolean); parts.forEach(([element, ownId]) => { if (element) { if (!references.includes(element.id)) { @@ -132,13 +162,17 @@ export const FieldItem = ({ } }); if (references.length > 0) { - inputElement.setAttribute(attribute, references.join(" ")); + element.setAttribute(attribute, references.join(" ")); } else { - inputElement.removeAttribute(attribute); + element.removeAttribute(attribute); } }; - if (labelElement instanceof HTMLLabelElement && labelableElements.includes(inputElement.tagName)) { + if (selectCombobox) { + if (labelElement instanceof HTMLLabelElement && labelElement.htmlFor === inputElement.id) { + labelElement.removeAttribute("for"); + } + } else if (labelElement instanceof HTMLLabelElement && labelableElements.includes(inputElement.tagName)) { // an already set `for` is only kept if it refers to the ID of the input element of this field item if (labelElement.getAttribute("for") !== inputElement.id) { labelElement.setAttribute("for", inputElement.id); @@ -150,13 +184,23 @@ export const FieldItem = ({ inputElement.setAttribute("aria-labelledby", labelElement.id); } } else { - updateReferences("aria-labelledby", [[undefined, `label_${fieldItemId}`]]); + updateReferences(inputElement, "aria-labelledby", [[undefined, `label_${fieldItemId}`]]); } - updateReferences("aria-describedby", [ + const descriptions: [HTMLElement | undefined, string][] = [ [messageElement, `message_${fieldItemId}`], [helpElement, `help_${fieldItemId}`], - ]); + ]; + updateReferences(inputElement, "aria-describedby", descriptions); + + if (selectCombobox) { + if (labelElement) { + updateReferences(selectCombobox, "aria-labelledby", [[labelElement, `label_${fieldItemId}`]]); + } else { + updateReferences(selectCombobox, "aria-labelledby", [[undefined, `label_${fieldItemId}`]]); + } + updateReferences(selectCombobox, "aria-describedby", descriptions); + } }, [fieldItemId, preventAriaAttribution]); React.useEffect(() => { diff --git a/src/components/Form/tests/FieldItem.test.tsx b/src/components/Form/tests/FieldItem.test.tsx index 08239e299..ee1682aa1 100644 --- a/src/components/Form/tests/FieldItem.test.tsx +++ b/src/components/Form/tests/FieldItem.test.tsx @@ -162,6 +162,45 @@ describe("FieldItem", () => { expect(label).toHaveAttribute("for", input.id); }); }); + it("exposes the labelled Select combobox without a duplicate button", () => { + const { fieldItem, label } = renderFieldItem({ + labelProps: { text: "Filter by language" }, + helperText: "Only matching values are shown.", + messageText: "Choose a language.", + children: ( + + ), + }); + const combobox = fieldItem.querySelector('[role="combobox"]'); + const button = fieldItem.querySelector("button"); + + expect(combobox).toHaveAccessibleName("Filter by language"); + expect(combobox).toHaveAccessibleDescription("Choose a language. Only matching values are shown."); + expect(label).not.toHaveAttribute("for"); + expect(button).toHaveAttribute("aria-hidden", "true"); + }); + it("restores the button when a Select becomes filterable", () => { + const { container, rerender } = render( + +
+ +
+
, + ); + const button = container.querySelector("button"); + expect(button).toHaveAttribute("aria-hidden", "true"); + + rerender( + +
+ +
+
, + ); + expect(button).not.toHaveAttribute("aria-hidden"); + }); it("should remove the reference to a removed label from `aria-labelledby`", () => { const { container, rerender } = render( } />, diff --git a/src/components/Select/Select.test.tsx b/src/components/Select/Select.test.tsx new file mode 100644 index 000000000..ef1218c6a --- /dev/null +++ b/src/components/Select/Select.test.tsx @@ -0,0 +1,182 @@ +import React from "react"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import "@testing-library/jest-dom"; + +import FieldItem from "../Form/FieldItem"; +import MenuItem from "../Menu/MenuItem"; + +import Select from "./Select"; + +it("connects a filterable Select search input to its listbox", async () => { + const user = userEvent.setup(); + render( + , + ); + + await user.click(screen.getByRole("button", { name: "Choose language" })); + expect(screen.getByRole("combobox")).toHaveAttribute("aria-controls", screen.getByRole("listbox").id); +}); + +describe("non-filterable Select keyboard navigation", () => { + it("keeps focus on the combobox while navigating and returns to it after Escape", async () => { + const user = userEvent.setup(); + const onItemSelect = jest.fn(); + + render( + <> + + + ( + + )} + onItemSelect={onItemSelect} + contextOverlayProps={{ transitionDuration: 0 }} + > + + + + , + ); + + const combobox = screen.getByRole("combobox"); + await user.tab(); + expect(combobox).toHaveFocus(); + await user.keyboard("{Enter}{ArrowDown}{Enter}"); + expect(onItemSelect).toHaveBeenCalledWith("Object", expect.anything()); + await waitFor(() => expect(combobox).toHaveAttribute("aria-expanded", "false")); + await waitFor(() => expect(screen.queryByRole("listbox")).not.toBeInTheDocument()); + expect(combobox).toHaveFocus(); + await user.tab(); + expect(screen.getByRole("button", { name: "After" })).toHaveFocus(); + }); + + it("closes the list when Tab moves focus to the next control", async () => { + const user = userEvent.setup(); + render( + <> +