Select component a11y issues (CMEM-8157)

CHANGELOG.md

@@ -25,6 +25,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
             - an already set `for` is only kept if it refers to the ID of the input element of the field item
         - helper text and message are referred by the input element via `aria-describedby`
         - `input`, `textarea`, `select`, the toggle button of `<Select />` and the editable area of `<CodeEditor />` are supported as input element
+            - the toggle button of `<Select />` is named by the label and its own content via `aria-labelledby`, so the selected value stays part of the accessible name
+            - the `combobox` target wrapper of not filterable `<Select />` elements gets the same name
             - input elements that cannot be referenced by `for`, e.g. the editable area of the code editor, are connected via `aria-labelledby`
             - parts that are created after the field item was mounted, e.g. by the code editor, are connected as soon as they exist
         - already set `id` values and connections are never overwritten

src/common/utils/truncateMarkdownDisplay.ts

@@ -4,11 +4,11 @@ import { MarkdownProps } from "../../cmem/markdown/Markdown";
 
 import { reduceToText, ReduceToTextFuncType } from "./reduceToText";
 
-interface MarkdownWithCutOffProps extends Omit<MarkdownProps, "cutOff"> {
+export interface MarkdownWithCutOffProps extends Omit<MarkdownProps, "cutOff"> {
     cutOff: NonNullable<MarkdownProps["cutOff"]>;
 }
 
-interface TruncateMarkdownDisplayType {
+export interface TruncateMarkdownDisplayType {
     (
         /**
          *  Markdown element with mandatory `cutOff` property.

src/components/Form/FieldItem.tsx

@@ -153,6 +153,33 @@ export const FieldItem = ({
             updateReferences("aria-labelledby", [[undefined, `label_${fieldItemId}`]]);
         }
 
+        /**
+         * The toggle button of a `Select` displays the selected value as its content.
+         * A label connected via `for` or `aria-labelledby` would replace that value in the accessible name,
+         * so the button refers to the label and to itself.
+         * BlueprintJS sets `role="combobox"` on the target wrapper of not filterable selects, it gets the same name.
+         * Names set by the using application via `aria-label` or foreign `aria-labelledby` IDs stay untouched.
+         */
+        const selectTarget = inputElement.matches(`.${eccgui}-select button`)
+            ? inputElement.closest<HTMLElement>(`.${eccgui}-select`)
+            : null;
+        if (selectTarget) {
+            const ownLabelIds = [labelElement?.id, `label_${fieldItemId}`, inputElement.id];
+            const nameSelectElement = (element: HTMLElement, isNamed: boolean) => {
+                const references = (element.getAttribute("aria-labelledby") ?? "").split(" ").filter(Boolean);
+                if (element.hasAttribute("aria-label") || references.some((id) => !ownLabelIds.includes(id))) {
+                    return;
+                }
+                if (labelElement && isNamed) {
+                    element.setAttribute("aria-labelledby", `${labelElement.id} ${inputElement.id}`);
+                } else {
+                    element.removeAttribute("aria-labelledby");
+                }
+            };
+            nameSelectElement(inputElement, true);
+            nameSelectElement(selectTarget, selectTarget.getAttribute("role") === "combobox");
+        }
+
         updateReferences("aria-describedby", [
             [messageElement, `message_${fieldItemId}`],
             [helpElement, `help_${fieldItemId}`],

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

@@ -5,6 +5,9 @@ import "@testing-library/jest-dom";
 
 import { CLASSPREFIX as eccgui } from "../../../configuration/constants";
 import { CodeEditor } from "../../../extensions/codemirror/CodeMirror";
+import Button from "../../Button/Button";
+import MenuItem from "../../Menu/MenuItem";
+import Select from "../../Select/Select";
 import FieldItem from "../FieldItem";
 
 const renderFieldItem = (props: React.ComponentProps<typeof FieldItem>) => {
@@ -316,3 +319,84 @@ describe("FieldItem with CodeEditor", () => {
         expect(input).toHaveAttribute("aria-describedby", message.id);
     });
 });
+
+describe("FieldItem with Select", () => {
+    const renderSelect = (
+        fieldItemProps: Partial<React.ComponentProps<typeof FieldItem>>,
+        selectProps: Partial<React.ComponentProps<typeof Select<string>>> = {},
+    ) => {
+        const view = render(
+            <FieldItem labelProps={{ text: "Label text" }} {...fieldItemProps}>
+                <Select<string>
+                    items={["first", "second"]}
+                    itemRenderer={(item, { handleClick }) => <MenuItem key={item} text={item} onClick={handleClick} />}
+                    onItemSelect={() => {}}
+                    text="first"
+                    {...selectProps}
+                />
+            </FieldItem>,
+        );
+        const { container } = view;
+        return {
+            ...view,
+            label: container.getElementsByClassName(`${eccgui}-fielditem__label`)[0] as HTMLElement | undefined,
+            target: container.getElementsByClassName(`${eccgui}-select`)[0] as HTMLElement,
+            button: container.querySelector(`.${eccgui}-select button`) as HTMLElement,
+        };
+    };
+
+    it("should name the toggle button by the label and its own content that displays the value", () => {
+        const { label, button } = renderSelect({});
+        expect(label).toHaveAttribute("for", button.id);
+        expect(button).toHaveAttribute("aria-labelledby", `${label!.id} ${button.id}`);
+    });
+    it("should also name the combobox wrapper of not filterable selects", () => {
+        const { label, target, button } = renderSelect({}, { filterable: false });
+        expect(target).toHaveAttribute("role", "combobox");
+        expect(target).toHaveAttribute("aria-labelledby", `${label!.id} ${button.id}`);
+    });
+    it("should not name the wrapper of filterable selects because it is no combobox", () => {
+        const { target } = renderSelect({});
+        expect(target).not.toHaveAttribute("role");
+        expect(target).not.toHaveAttribute("aria-labelledby");
+    });
+    it("should keep the value in the name of disabled field items", () => {
+        const { label, button } = renderSelect({ disabled: true }, { disabled: true });
+        expect(label!.tagName).toBe("SPAN");
+        expect(button).toHaveAttribute("aria-labelledby", `${label!.id} ${button.id}`);
+    });
+    it("should not change names that are set by the using application", () => {
+        const { button } = renderSelect({}, { children: <Button aria-label="Custom name" text="first" /> });
+        expect(button).toHaveAttribute("aria-label", "Custom name");
+        expect(button).not.toHaveAttribute("aria-labelledby");
+
+        const { button: otherButton } = renderSelect(
+            {},
+            { children: <Button aria-labelledby="externallabel" text="first" /> },
+        );
+        expect(otherButton).toHaveAttribute("aria-labelledby", "externallabel");
+    });
+    it("should remove the names created by the field item if the label is removed", () => {
+        const { target, button, rerender } = renderSelect({}, { filterable: false });
+        expect(button).toHaveAttribute("aria-labelledby");
+
+        rerender(
+            <FieldItem>
+                <Select<string>
+                    items={["first", "second"]}
+                    itemRenderer={(item, { handleClick }) => <MenuItem key={item} text={item} onClick={handleClick} />}
+                    onItemSelect={() => {}}
+                    text="first"
+                    filterable={false}
+                />
+            </FieldItem>,
+        );
+        expect(button).not.toHaveAttribute("aria-labelledby");
+        expect(target).not.toHaveAttribute("aria-labelledby");
+    });
+    it("should not be connected if `preventAriaAttribution` is set", () => {
+        const { button, target } = renderSelect({ preventAriaAttribution: true }, { filterable: false });
+        expect(button).not.toHaveAttribute("aria-labelledby");
+        expect(target).not.toHaveAttribute("aria-labelledby");
+    });
+});