Fixes after Review (CMEM-8156)

CHANGELOG.md

@@ -26,16 +26,26 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
     - if none of the elements referenced by `aria-labelledby` exists then a given `aria-label` is used as fallback, without `aria-label` the `role` and `aria-modal` attributes are removed
 - `<SimpleDialog />`, `<AlertDialog />`
     - the intent state is also used as `aria-label` fallback if none of the elements referenced by `aria-labelledby` exists
+- `<Select />`
+    - The clear action shows `onClearanceText` as native `title` instead of a `Tooltip`.
+    - If not filterable, the focusable combobox renders its default target without a button, and hides focusable elements of custom `children` from the Tab order and screen readers.
 
 ### Fixed
 
 - `<FieldItem />`
-    - Clicking a label focuses its control when a native label association is not available, including Select comboboxes and CodeEditor.
+    - Clicking a label focuses its control when a native label association is not available, e.g. for CodeEditor.
+    - An explicit `labelProps.htmlFor` that refers to an input element of the field item selects the connected input element.
+        - Helper text and message are connected to this input element via `aria-describedby`, not always to the first one.
+    - A label ID added to `aria-labelledby` by the field item is removed again if the label or the connected input element changes, also if the label ID is set by the application.
 - `<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.
     - Keep the clear action reachable with Tab and Shift+Tab and support clearing with Enter or Space without opening the options.
+    - Expose the clear action as a button named by `onClearanceText`, and as disabled if the Select is disabled.
+        - If `onClearanceText` is empty, the clear action stays keyboard accessible but is hidden from screen readers.
+    - Prevent any interaction with the clear action of a disabled Select.
+    - Without `fill`, the wrapper element used for `wrapperProps` and test IDs is displayed inline, like the Select itself.
 
 ## [26.2.1] - 2026-11-02
 

src/components/Form/FieldItem.tsx

@@ -72,8 +72,9 @@ export const FieldItem = ({
     ...otherProps
 }: FieldItemProps) => {
     const fieldItemRef = React.useRef<HTMLDivElement>(null);
-    const hiddenSelectButtonRef = React.useRef<HTMLElement | null>(null);
     const connectedInputRef = React.useRef<HTMLElement | undefined>(undefined);
+    /** label ID that was added to `aria-labelledby` by the field item itself, it may be set by the application */
+    const labelledByRef = React.useRef<{ element: HTMLElement; id: string } | undefined>(undefined);
     /** 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, "");
 
@@ -103,23 +104,13 @@ export const FieldItem = ({
                 connectableInputSelectors.map((selector) => `.${eccgui}-fielditem__inputfields ${selector}`).join(", "),
             ),
         );
-        const inputElement = inputElements[0];
+        // an explicit `for` of the label selects the connected input element, otherwise the first one is used
+        const labelTarget = labelElement instanceof HTMLLabelElement ? labelElement.getAttribute("for") : null;
+        const inputElement =
+            inputElements.find((element) => labelTarget && element.id === labelTarget) ?? inputElements[0];
         const helpElement = ownPart(fieldItem.querySelectorAll<HTMLElement>(`.${eccgui}-fielditem__helpertext`));
         const messageElement = ownPart(fieldItem.querySelectorAll<HTMLElement>(`.${eccgui}-fielditem__message`));
 
-        // Only expose the focusable combobox to assistive technology.
-        const selectButton = inputElement?.matches(`.${eccgui}-select[role="combobox"]`)
-            ? inputElement.querySelector<HTMLElement>("button")
-            : null;
-        if (hiddenSelectButtonRef.current && hiddenSelectButtonRef.current !== selectButton) {
-            hiddenSelectButtonRef.current.removeAttribute("aria-hidden");
-            hiddenSelectButtonRef.current = null;
-        }
-        if (selectButton && selectButton.getAttribute("aria-hidden") !== "true") {
-            selectButton.setAttribute("aria-hidden", "true");
-            hiddenSelectButtonRef.current = selectButton;
-        }
-
         const setMissingId = (element: HTMLElement | undefined, id: string) => {
             if (element && !element.id) {
                 element.id = id;
@@ -169,17 +160,27 @@ export const FieldItem = ({
         }
         connectedInputRef.current = inputElement;
 
+        // remove the label ID added before if the label is not connected to that input element via `aria-labelledby` anymore
+        const connectsNatively =
+            labelElement instanceof HTMLLabelElement &&
+            !!inputElement &&
+            labelableElements.includes(inputElement.tagName);
+        const labelledBy = labelledByRef.current;
+        if (
+            labelledBy &&
+            (labelledBy.element !== inputElement || labelledBy.id !== labelElement?.id || connectsNatively)
+        ) {
+            updateReferences(labelledBy.element, "aria-labelledby", [[undefined, labelledBy.id]]);
+            labelledByRef.current = undefined;
+        }
+
         if (!inputElement) {
             return;
         }
 
-        if (labelElement instanceof HTMLLabelElement && labelableElements.includes(inputElement.tagName)) {
-            // an already set `for` is only kept if it refers to a labelable input element of this field item
-            const labelTarget = labelElement.getAttribute("for");
-            const refersToOwnInput = inputElements.some(
-                (element) => element.id === labelTarget && labelableElements.includes(element.tagName),
-            );
-            if (!refersToOwnInput) {
+        if (connectsNatively) {
+            // a `for` that does not refer to an input element of this field item is replaced
+            if (labelTarget !== inputElement.id) {
                 labelElement.setAttribute("for", inputElement.id);
             }
         } else if (labelElement) {
@@ -190,6 +191,7 @@ export const FieldItem = ({
             }
             if (!inputElement.getAttribute("aria-labelledby")) {
                 inputElement.setAttribute("aria-labelledby", labelElement.id);
+                labelledByRef.current = { element: inputElement, id: labelElement.id };
             }
         } else {
             updateReferences(inputElement, "aria-labelledby", [[undefined, `label_${fieldItemId}`]]);

src/components/Form/tests/FieldItem.test.tsx

@@ -1,7 +1,7 @@
 import React from "react";
+import { createPortal } from "react-dom";
 import { render, screen, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { createPortal } from "react-dom";
 
 import "@testing-library/jest-dom";
 
@@ -73,8 +73,9 @@ describe("FieldItem", () => {
             expect(label).toHaveAttribute("for", input.id);
         });
         it("should keep a `for` that refers to another input element of the field item", () => {
-            const { label } = renderFieldItem({
+            const { label, fieldItem } = renderFieldItem({
                 labelProps: { text: "Label text", htmlFor: "otherinput" },
+                helperText: "Help text",
                 children: (
                     <>
                         <input type="text" id="custominput" />
@@ -83,6 +84,45 @@ describe("FieldItem", () => {
                 ),
             });
             expect(label).toHaveAttribute("for", "otherinput");
+            // the explicitly labelled input element is the connected one, also for the descriptions
+            expect(fieldItem.querySelector("#otherinput")).toHaveAccessibleDescription("Help text");
+            expect(fieldItem.querySelector("#custominput")).not.toHaveAttribute("aria-describedby");
+        });
+        it.each([
+            [
+                "a non-filterable Select",
+                <div className={`${eccgui}-select`} role="combobox" tabIndex={0}>
+                    <span>Value</span>
+                </div>,
+            ],
+            [
+                "a code editor",
+                <div className={`${eccgui}-codeeditor`}>
+                    <div className="cm-content" contentEditable />
+                </div>,
+            ],
+        ])("should keep a `for` that refers to an input element after %s", async (_, firstElement) => {
+            const user = userEvent.setup();
+            const { label, fieldItem } = renderFieldItem({
+                labelProps: { text: "Label text", htmlFor: "otherinput" },
+                helperText: "Help text",
+                children: (
+                    <>
+                        {firstElement}
+                        <input type="text" id="otherinput" />
+                    </>
+                ),
+            });
+            const otherInput = fieldItem.querySelector("#otherinput");
+
+            expect(label).toHaveAttribute("for", "otherinput");
+            expect(otherInput).toHaveAccessibleName("Label text");
+            expect(otherInput).toHaveAccessibleDescription("Help text");
+            expect(fieldItem.querySelector("[aria-labelledby]")).not.toBeInTheDocument();
+            expect(fieldItem.querySelector("[aria-describedby]")).toBe(otherInput);
+
+            await user.click(screen.getByText("Label text"));
+            expect(otherInput).toHaveFocus();
         });
         it("should replace a `for` that refers to an input element outside of the field item", () => {
             const { container } = render(
@@ -187,7 +227,7 @@ describe("FieldItem", () => {
                 expect(label).toHaveAttribute("for", input.id);
             });
         });
-        it("exposes the labelled Select combobox without a duplicate button", () => {
+        it("connects the Select combobox instead of its inner button", () => {
             const { fieldItem, label } = renderFieldItem({
                 labelProps: { text: "Filter by language" },
                 helperText: "Only matching values are shown.",
@@ -205,7 +245,6 @@ describe("FieldItem", () => {
             expect(combobox).toHaveAccessibleDescription("Choose a language. Only matching values are shown.");
             expect(combobox).toHaveAttribute("id", expect.stringMatching(/^input_/));
             expect(label).not.toHaveAttribute("for");
-            expect(button).toHaveAttribute("aria-hidden", "true");
             expect(button).not.toHaveAttribute("id");
             expect(button).not.toHaveAttribute("aria-describedby");
         });
@@ -230,7 +269,6 @@ describe("FieldItem", () => {
             expect(label).not.toHaveAttribute("for");
             expect(hiddenInput).not.toHaveAttribute("id");
             expect(hiddenInput).not.toHaveAttribute("aria-describedby");
-            expect(hiddenInput).not.toHaveAttribute("aria-hidden");
         });
         it("keeps a custom accessible name on the Select combobox", () => {
             const { fieldItem } = renderFieldItem({
@@ -247,7 +285,7 @@ describe("FieldItem", () => {
 
             expect(fieldItem.querySelector('[role="combobox"]')).toHaveAccessibleName("Custom type");
         });
-        it("restores the button when a Select becomes filterable", () => {
+        it("reconnects the label when a Select becomes filterable", () => {
             const { container, rerender } = render(
                 <FieldItem labelProps={{ text: "Type" }}>
                     <div className={`${eccgui}-select`} role="combobox">
@@ -258,7 +296,7 @@ describe("FieldItem", () => {
             const button = container.querySelector("button");
             const label = container.querySelector("label");
             const selectTarget = container.querySelector('[role="combobox"]');
-            expect(button).toHaveAttribute("aria-hidden", "true");
+            expect(selectTarget).toHaveAccessibleName("Type");
 
             rerender(
                 <FieldItem labelProps={{ text: "Type" }}>
@@ -267,7 +305,6 @@ describe("FieldItem", () => {
                     </div>
                 </FieldItem>,
             );
-            expect(button).not.toHaveAttribute("aria-hidden");
             expect(label).toHaveAttribute("for", button?.id);
             expect(selectTarget).not.toHaveAttribute("id");
             expect(selectTarget).not.toHaveAttribute("aria-labelledby");
@@ -281,7 +318,6 @@ describe("FieldItem", () => {
             );
             expect(label).not.toHaveAttribute("for");
             expect(container.querySelector('[role="combobox"]')).toHaveAccessibleName("Type");
-            expect(button).toHaveAttribute("aria-hidden", "true");
             expect(button).not.toHaveAttribute("id");
         });
         it("should remove the reference to a removed label from `aria-labelledby`", () => {
@@ -348,6 +384,59 @@ describe("FieldItem", () => {
             expect(input).toHaveAttribute("aria-labelledby", "customlabel");
             expect(input).toHaveAttribute("aria-describedby", "customhelp");
         });
+        it("should remove a label ID of the application from `aria-labelledby` if another input element is connected", () => {
+            const fieldItem = (htmlFor?: string) => (
+                <FieldItem labelProps={{ text: "Label text", id: "customlabel", htmlFor }}>
+                    <div className={`${eccgui}-codeeditor`}>
+                        <div className="cm-content" contentEditable id="editor" />
+                    </div>
+                    <input type="text" id="otherinput" />
+                </FieldItem>
+            );
+            const { container, rerender } = render(fieldItem());
+            const editor = container.querySelector("#editor");
+            const otherInput = container.querySelector("#otherinput");
+            expect(editor).toHaveAttribute("aria-labelledby", "customlabel");
+
+            rerender(fieldItem("otherinput"));
+            expect(editor).not.toHaveAttribute("aria-labelledby");
+            expect(container.querySelector("label")).toHaveAttribute("for", "otherinput");
+            expect(otherInput).toHaveAccessibleName("Label text");
+
+            rerender(fieldItem());
+            expect(editor).toHaveAttribute("aria-labelledby", "customlabel");
+            expect(container.querySelector("label")).not.toHaveAttribute("for");
+        });
+        it("should remove a label ID of the application from `aria-labelledby` if the label is removed", () => {
+            const editor = (
+                <div className={`${eccgui}-codeeditor`}>
+                    <div className="cm-content" contentEditable id="editor" />
+                </div>
+            );
+            const { container, rerender } = render(
+                <FieldItem labelProps={{ text: "Label text", id: "customlabel" }}>{editor}</FieldItem>,
+            );
+            expect(container.querySelector("#editor")).toHaveAttribute("aria-labelledby", "customlabel");
+
+            rerender(<FieldItem>{editor}</FieldItem>);
+            expect(container.querySelector("#editor")).not.toHaveAttribute("aria-labelledby");
+        });
+        it("should keep an `aria-labelledby` of the application if another input element is connected", () => {
+            const fieldItem = (htmlFor?: string) => (
+                <FieldItem labelProps={{ text: "Label text", id: "customlabel", htmlFor }}>
+                    <div className={`${eccgui}-codeeditor`}>
+                        <div className="cm-content" contentEditable id="editor" aria-labelledby="otherlabel" />
+                    </div>
+                    <input type="text" id="otherinput" />
+                </FieldItem>
+            );
+            const { container, rerender } = render(fieldItem());
+            const editor = container.querySelector("#editor");
+            expect(editor).toHaveAttribute("aria-labelledby", "otherlabel");
+
+            rerender(fieldItem("otherinput"));
+            expect(editor).toHaveAttribute("aria-labelledby", "otherlabel");
+        });
         it("should connect a code editor input element via `aria-labelledby`", () => {
             const { container } = render(
                 <FieldItem labelProps={{ text: "Label text" }} helperText="Helper text">

src/components/Select/Select.stories.tsx

@@ -4,7 +4,7 @@ import { Meta, StoryFn } from "@storybook/react";
 import { fn } from "storybook/test";
 
 import { helpersArgTypes } from "../../../.storybook/helpers";
-import { Button, Depiction, MenuItem, Select } from "../../index";
+import { Button, Depiction, FieldItem, MenuItem, Select, Tag } from "../../index";
 import { FullExample as DepictionExample } from "../Depiction/stories/Depiction.stories";
 
 export default {
@@ -20,19 +20,36 @@ export default {
     },
 } as Meta<typeof Select>;
 
-const Template: StoryFn<typeof Select> = (args) => <Select {...args} />;
+const Template: StoryFn<typeof Select> = (args) => (
+    <FieldItem labelProps={{ text: "Select example", hidden: true }}>
+        <Select {...args} />
+    </FieldItem>
+);
 
 export const Default = Template.bind({});
 Default.args = {
     items: loremIpsum({ p: 1, avgSentencesPerParagraph: 5, random: false })
         .toString()
         .split(".")
+        .filter((item) => item.trim().length > 0)
         .map((item) => {
             return { label: item };
         }),
-    itemRenderer: (item, props) => {
-        return <MenuItem text={item.label} title={item.label} {...props} />;
-    },
+    itemRenderer: (item, { handleClick, handleFocus, id, modifiers }) => (
+        <MenuItem
+            key={item.label}
+            id={id}
+            text={item.label}
+            title={item.label}
+            onClick={handleClick}
+            onFocus={handleFocus}
+            active={modifiers.active}
+            aria-selected={modifiers.active}
+            role="option"
+            roleStructure="none"
+            tabIndex={-1}
+        />
+    ),
     fill: true,
 };
 
@@ -57,6 +74,38 @@ ControlledTarget.args = {
     onActiveItemChange: fn(),
 };
 
+/**
+ * A controlled select target can also be used for a non-filterable select.
+ * Then the select itself is the focusable combobox, it gets the label and handles the keyboard navigation.
+ *
+ * **The controlled target must not be or contain any focusable element in this case**,
+ * e.g. use a `Tag` without `onClick` or a text instead of a `Button`.
+ * Focusable elements are removed from the Tab order and hidden from screen readers by the select,
+ * so they would not work as expected and their text is not announced as the selected value.
+ *
+ * If you need a focusable element like a `Button` anyway, then add the selected value in a hidden `span`
+ * that is still accessible to screen readers:
+ *
+ * ```tsx
+ * <Select filterable={false} {...otherSelectProps}>
+ *     <>
+ *         <Button text={selectedValue} />
+ *         <ApplicationViewability hide="screen">
+ *             <span>{selectedValue}</span>
+ *         </ApplicationViewability>
+ *     </>
+ * </Select>
+ * ```
+ */
+export const ControlledTargetNotFilterable = Template.bind({});
+ControlledTargetNotFilterable.args = {
+    ...Default.args,
+    fill: false,
+    filterable: false,
+    children: <Tag>Controlled select target</Tag>,
+    onActiveItemChange: fn(),
+};
+
 /**
  * If the selection is not mandatory it is suggested to add an option letting the user reset the select value.
  * `onClearanceHandler` follows the pattern to add a button on the right side of the element.