Select: Fix accesibility (label combobox) and restore keyboard navigation

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
+- `<Select />`
+    - Connect FieldItem labels, helper text, and messages to the non-filterable Select combobox.
 - new `utils` methods:
     - `truncateMarkdownDisplay`: helper function to iterate over `Markdown` renderings to improve the experienced `cutOff` value
 - new icons:
@@ -96,6 +98,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
     - the webpack configuration excludes `dist/` from module resolution now, so the sources are always used even if a story references the package root
 - Added explicitly `assert` dependency
     - the linter of `<CodeEditor />` uses `jshint`, which imports `console-browserify`, and this package requires the node core modules `assert` and `util` without declaring them; bundlers based on webpack 5 do not provide shims for node core modules anymore, so the polyfill (and `util` together with it) is part of the delivery now
+- `<Select />`
+    - 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
 

src/components/Form/FieldItem.tsx

@@ -71,6 +71,7 @@ export const FieldItem = ({
     ...otherProps
 }: FieldItemProps) => {
     const fieldItemRef = React.useRef<HTMLDivElement>(null);
+    const hiddenSelectButtonRef = React.useRef<HTMLElement | null>(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<HTMLElement>(
+                `.${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<HTMLElement>(`.${eccgui}-fielditem__helpertext`));
         const messageElement = ownPart(fieldItem.querySelectorAll<HTMLElement>(`.${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(() => {

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: (
+                    <div className={`${eccgui}-select`} role="combobox" aria-expanded="false">
+                        <button type="button">en</button>
+                    </div>
+                ),
+            });
+            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(
+                <FieldItem labelProps={{ text: "Type" }}>
+                    <div className={`${eccgui}-select`} role="combobox">
+                        <button type="button">Value</button>
+                    </div>
+                </FieldItem>,
+            );
+            const button = container.querySelector("button");
+            expect(button).toHaveAttribute("aria-hidden", "true");
+
+            rerender(
+                <FieldItem labelProps={{ text: "Type" }}>
+                    <div className={`${eccgui}-select`}>
+                        <button type="button">Value</button>
+                    </div>
+                </FieldItem>,
+            );
+            expect(button).not.toHaveAttribute("aria-hidden");
+        });
         it("should remove the reference to a removed label from `aria-labelledby`", () => {
             const { container, rerender } = render(
                 <FieldItem disabled labelProps={{ text: "Label text" }} children={<input type="text" />} />,

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(
+        <Select
+            items={["en"]}
+            itemRenderer={(item, { handleClick, id }) => (
+                <MenuItem
+                    key={item}
+                    text={item}
+                    id={id}
+                    role="option"
+                    roleStructure="none"
+                    tabIndex={-1}
+                    onClick={handleClick}
+                />
+            )}
+            onItemSelect={jest.fn()}
+        >
+            <button type="button">Choose language</button>
+        </Select>,
+    );
+
+    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(
+            <>
+                <button type="button">Before</button>
+                <FieldItem labelProps={{ text: "Type" }}>
+                    <Select
+                        filterable={false}
+                        items={["Value", "Object"]}
+                        itemRenderer={(item, { handleClick, handleFocus, id, modifiers }) => (
+                            <MenuItem
+                                key={item}
+                                text={item}
+                                id={id}
+                                onClick={handleClick}
+                                onFocus={handleFocus}
+                                active={modifiers.active}
+                                aria-selected={modifiers.active}
+                                role="option"
+                                roleStructure="none"
+                                tabIndex={-1}
+                            />
+                        )}
+                        onItemSelect={onItemSelect}
+                        text="Value"
+                        contextOverlayProps={{ transitionDuration: 0 }}
+                    />
+                </FieldItem>
+                <button type="button">After</button>
+            </>,
+        );
+
+        await user.tab();
+        await user.tab();
+        const combobox = screen.getByRole("combobox");
+        expect(combobox).toHaveFocus();
+        expect(combobox).toHaveAccessibleName("Type");
+        expect(screen.queryByRole("button", { name: "Type" })).not.toBeInTheDocument();
+
+        await user.keyboard("{Enter}");
+        expect(combobox).toHaveAttribute("aria-expanded", "true");
+        expect(combobox).toHaveFocus();
+        expect(document.getElementById(combobox.getAttribute("aria-activedescendant")!)).toEqual(
+            screen.getByRole("option", { name: "Value" }),
+        );
+
+        await user.keyboard("{ArrowDown}");
+        expect(combobox).toHaveFocus();
+        expect(combobox).toHaveAttribute("aria-activedescendant", expect.any(String));
+        expect(document.getElementById(combobox.getAttribute("aria-activedescendant")!)).toEqual(
+            screen.getByRole("option", { name: "Object" }),
+        );
+
+        await user.keyboard("{Escape}");
+        expect(combobox).toHaveFocus();
+        expect(onItemSelect).not.toHaveBeenCalled();
+        await user.tab();
+        await waitFor(() => expect(screen.getByRole("button", { name: "After" })).toHaveFocus());
+        await waitFor(() => expect(combobox).toHaveAttribute("aria-expanded", "false"));
+        await waitFor(() => expect(screen.queryByRole("listbox")).not.toBeInTheDocument());
+        expect(combobox).not.toHaveAttribute("aria-activedescendant");
+    });
+
+    it("selects the active option with Enter and keeps a custom button out of the Tab order", async () => {
+        const user = userEvent.setup();
+        const onItemSelect = jest.fn();
+        render(
+            <>
+                <Select
+                    filterable={false}
+                    items={["Value", "Object"]}
+                    itemRenderer={(item, { handleClick, id, modifiers }) => (
+                        <MenuItem
+                            key={item}
+                            text={item}
+                            id={id}
+                            onClick={handleClick}
+                            active={modifiers.active}
+                            aria-selected={modifiers.active}
+                            role="option"
+                            roleStructure="none"
+                            tabIndex={-1}
+                        />
+                    )}
+                    onItemSelect={onItemSelect}
+                    contextOverlayProps={{ transitionDuration: 0 }}
+                >
+                    <button type="button">Value</button>
+                </Select>
+                <button type="button">After</button>
+            </>,
+        );
+
+        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(
+            <>
+                <Select
+                    filterable={false}
+                    items={["Value", "Object"]}
+                    itemRenderer={(item, { handleClick, id, modifiers }) => (
+                        <MenuItem
+                            key={item}
+                            text={item}
+                            id={id}
+                            onClick={handleClick}
+                            active={modifiers.active}
+                            aria-selected={modifiers.active}
+                            role="option"
+                            roleStructure="none"
+                            tabIndex={-1}
+                        />
+                    )}
+                    onItemSelect={jest.fn()}
+                    text="Value"
+                    contextOverlayProps={{ transitionDuration: 0 }}
+                />
+                <button type="button">After</button>
+            </>,
+        );
+
+        const combobox = screen.getByRole("combobox");
+        await user.tab();
+        await user.keyboard("{Enter}");
+        expect(combobox).toHaveAttribute("aria-expanded", "true");
+        await user.tab();
+        expect(screen.getByRole("button", { name: "After" })).toHaveFocus();
+        await waitFor(() => expect(combobox).toHaveAttribute("aria-expanded", "false"));
+    });
+});