From 1caadb181e4edf6892b4f67a4b7fb09c6f937ff1 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Mon, 12 Oct 2026 12:12:56 +0200 Subject: [PATCH 01/19] Fix misused menu roles in selection popups and overlays - Use listbox and option roles for autocomplete suggestions. - Remove implicit menu semantics from generic ContextOverlay triggers. - Preserve menu popup semantics for ContextMenu and React Flow tools. - Add regression tests for popup semantics and selection behavior. --- CHANGELOG.md | 1 + .../AutoSuggestion/AutoSuggestion.tsx | 35 +++++++++- .../AutoSuggestion/AutoSuggestionList.tsx | 14 +++- .../tests/AutoSuggestion.test.tsx | 35 +++++++++- .../tests/AutoSuggestionList.test.tsx | 5 +- .../AutoCompleteField.test.tsx | 27 ++++++++ .../AutocompleteField/AutoCompleteField.tsx | 11 ++- .../autoCompleteFieldUtils.tsx | 9 ++- src/components/ContextOverlay/ContextMenu.tsx | 2 + .../ContextOverlay/ContextOverlay.tsx | 12 ++++ .../ContextOverlay/tests/ContextMenu.test.tsx | 3 + .../tests/ContextOverlay.test.tsx | 36 +++++++++- src/components/Menu/MenuItem.tsx | 69 +++++++++---------- src/components/Select/Select.stories.tsx | 2 +- .../codemirror/tests/CodeEditor.test.tsx | 8 +-- .../tests/EditorAppearanceConfigMenu.test.tsx | 12 ++-- .../toolbars/EditorAppearanceConfigMenu.tsx | 5 +- .../react-flow/handles/HandleTools.tsx | 3 +- src/extensions/react-flow/nodes/NodeTools.tsx | 3 +- .../react-flow/tests/ToolsPopupKind.test.tsx | 55 +++++++++++++++ 20 files changed, 286 insertions(+), 61 deletions(-) create mode 100644 src/components/AutocompleteField/AutoCompleteField.test.tsx create mode 100644 src/extensions/react-flow/tests/ToolsPopupKind.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 392f4abbe..370e45ecd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,7 @@ 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 `` 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 +- Fix misused menu roles in selection popups and overlays ### Deprecated diff --git a/src/components/AutoSuggestion/AutoSuggestion.tsx b/src/components/AutoSuggestion/AutoSuggestion.tsx index 89b76b4d7..dc1d2eb17 100644 --- a/src/components/AutoSuggestion/AutoSuggestion.tsx +++ b/src/components/AutoSuggestion/AutoSuggestion.tsx @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import React, { useEffect, useMemo, useState } from "react"; -import { Classes as BlueprintClassNames } from "@blueprintjs/core"; +import { Classes as BlueprintClassNames, PopupKind } from "@blueprintjs/core"; import { EditorView, Rect } from "@codemirror/view"; import { debounce } from "lodash"; @@ -218,6 +218,7 @@ export const CodeAutocompleteField = ({ CodeAutocompleteFieldSuggestionWithReplacementInfo | undefined >(undefined); const [cm, setCM] = React.useState(); + const suggestionListId = React.useId(); const currentCm = React.useRef(undefined); currentCm.current = cm; const isFocused = React.useRef(false); @@ -241,6 +242,36 @@ export const CodeAutocompleteField = ({ /** This is for the AutoSuggestionList component in order to re-render. */ const [focusedIndex, setFocusedIndex] = React.useState(0); + + React.useEffect(() => { + if (!cm) return; + const editor = cm.contentDOM; + editor.setAttribute("role", "combobox"); + editor.setAttribute("aria-autocomplete", "list"); + editor.setAttribute("aria-haspopup", "listbox"); + return () => { + editor.setAttribute("role", "textbox"); + editor.removeAttribute("aria-autocomplete"); + editor.removeAttribute("aria-haspopup"); + editor.removeAttribute("aria-controls"); + editor.removeAttribute("aria-expanded"); + editor.removeAttribute("aria-activedescendant"); + }; + }, [cm]); + + React.useEffect(() => { + if (!cm) return; + const editor = cm.contentDOM; + const suggestionsVisible = shouldShowDropdown && !suggestionsPending && suggestions.length > 0; + editor.setAttribute("aria-expanded", String(suggestionsVisible)); + if (suggestionsVisible) { + editor.setAttribute("aria-controls", suggestionListId); + editor.setAttribute("aria-activedescendant", `${suggestionListId}-option-${focusedIndex}`); + } else { + editor.removeAttribute("aria-controls"); + editor.removeAttribute("aria-activedescendant"); + } + }, [cm, shouldShowDropdown, suggestionsPending, suggestions.length, focusedIndex, suggestionListId]); const selectedTextRanges = React.useRef([]); const pathIsValid = validationResponse?.valid ?? true; @@ -709,6 +740,7 @@ export const CodeAutocompleteField = ({ } ${blueprintIntent ? BlueprintClassNames.intentClass(blueprintIntent as any) : ""}${inputIntentClass}`} > , "children"> { + listboxId?: string; // The options of the drop down options: Array; // Called when an item has been selected from the drop down @@ -76,6 +77,7 @@ const Item = React.forwardRef(ListItem); /** A drop-down-like list that can be used in combination with other components to show and select items. */ export const AutoSuggestionList = ({ + listboxId, isOpen, options, loading, @@ -86,6 +88,8 @@ export const AutoSuggestionList = ({ offsetValues, ...otherDivProps }: AutoSuggestionListProps) => { + const generatedListboxId = React.useId(); + const optionListId = listboxId ?? generatedListboxId; const [hoveredItem, setHoveredItem] = React.useState< CodeAutocompleteFieldSuggestionWithReplacementInfo | undefined >(undefined); @@ -141,10 +145,18 @@ export const AutoSuggestionList = ({ {loading ? ( Loader ) : ( - + {options.map((item, index) => ( { + if (element) { + element.id = `${optionListId}-option-${index}`; + element.setAttribute("aria-selected", String(currentlyFocusedIndex === index)); + } + }} active={currentlyFocusedIndex === index} onMouseDown={(e: any) => e.preventDefault()} onClick={() => onItemSelectionChange(item)} diff --git a/src/components/AutoSuggestion/tests/AutoSuggestion.test.tsx b/src/components/AutoSuggestion/tests/AutoSuggestion.test.tsx index 2a097fa8e..8577079ec 100644 --- a/src/components/AutoSuggestion/tests/AutoSuggestion.test.tsx +++ b/src/components/AutoSuggestion/tests/AutoSuggestion.test.tsx @@ -51,6 +51,39 @@ describe("CodeAutocompleteField", () => { expect(getByText(props.label!)).toBeTruthy(); }); + it("connects the focused editor to the active suggestion", async () => { + const { container } = render( + ({ + inputString, + cursorPosition, + replacementResults: [ + { + replacementInterval: { from: 0, length: 5 }, + extractedQuery: "", + replacements: [{ value: "first" }, { value: "second" }], + }, + ], + })} + />, + ); + const editor = screen.getByRole("combobox"); + act(() => editor.focus()); + const options = await screen.findAllByRole("option"); + expect(container.querySelector('[aria-haspopup="menu"]')).toBeNull(); + expect(editor).toHaveAttribute("aria-haspopup", "listbox"); + expect(editor).toHaveAttribute("aria-controls", options[0].parentElement?.id); + expect(editor).toHaveAttribute("aria-activedescendant", options[0].id); + fireEvent.keyDown(editor, { key: "ArrowDown", code: "ArrowDown", keyCode: 40 }); + await waitFor(() => expect(editor).toHaveAttribute("aria-activedescendant", options[1].id)); + fireEvent.keyDown(editor, { key: "Escape", code: "Escape", keyCode: 27 }); + await waitFor(() => expect(editor).toHaveAttribute("aria-expanded", "false")); + expect(editor).not.toHaveAttribute("aria-activedescendant"); + }); + it.each([false, true])("updates the container height (multiline: %s)", (multiline) => { const { container, rerender } = render(); const editorContainer = container.querySelector(".eccgui-codeeditor"); @@ -92,7 +125,7 @@ describe("CodeAutocompleteField", () => { })} />, ); - const editor = screen.getByRole("textbox"); + const editor = screen.getByRole("combobox"); act(() => editor.focus()); expect(await screen.findByText("completion")).toBeVisible(); const view = EditorView.findFromDOM(editor); diff --git a/src/components/AutoSuggestion/tests/AutoSuggestionList.test.tsx b/src/components/AutoSuggestion/tests/AutoSuggestionList.test.tsx index ee9d7e740..43967d967 100644 --- a/src/components/AutoSuggestion/tests/AutoSuggestionList.test.tsx +++ b/src/components/AutoSuggestion/tests/AutoSuggestionList.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { Classes } from "@blueprintjs/core"; -import { fireEvent, render, waitFor } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import "@testing-library/jest-dom"; @@ -65,6 +65,9 @@ describe("Dropdown list", () => { const dropdownItems = container.getElementsByTagName("li"); expect(container).not.toBeEmptyDOMElement(); expect(dropdownItems.length).toBe(2); + expect(screen.getByRole("listbox")).toBeInTheDocument(); + expect(screen.getAllByRole("option")).toHaveLength(2); + expect(screen.queryByRole("menu")).not.toBeInTheDocument(); }); it("should move horizontally for every cursor movement", async () => { diff --git a/src/components/AutocompleteField/AutoCompleteField.test.tsx b/src/components/AutocompleteField/AutoCompleteField.test.tsx new file mode 100644 index 000000000..709a8e781 --- /dev/null +++ b/src/components/AutocompleteField/AutoCompleteField.test.tsx @@ -0,0 +1,27 @@ +import React from "react"; +import { fireEvent, render, screen } from "@testing-library/react"; + +import "@testing-library/jest-dom"; + +import { SuggestField } from "./AutoCompleteField"; + +describe("SuggestField", () => { + it("renders suggestions as listbox options", async () => { + render( + + onSearch={() => ["Alpha"]} + itemRenderer={(item) => item} + itemValueRenderer={(item) => item} + itemValueSelector={(item) => item} + itemValueString={(item) => item} + noResultText="No results" + />, + ); + + fireEvent.focus(screen.getByRole("combobox")); + + const listbox = await screen.findByRole("listbox"); + expect(listbox.querySelectorAll('[role="option"]')).toHaveLength(1); + expect(listbox.querySelectorAll('[role="menuitem"]')).toHaveLength(0); + }); +}); diff --git a/src/components/AutocompleteField/AutoCompleteField.tsx b/src/components/AutocompleteField/AutoCompleteField.tsx index 770fc222e..3b9bac7fd 100644 --- a/src/components/AutocompleteField/AutoCompleteField.tsx +++ b/src/components/AutocompleteField/AutoCompleteField.tsx @@ -341,6 +341,7 @@ export function SuggestField(props: SuggestFieldProps(props: SuggestFieldProps ""} itemRenderer={requestError ? requestErrorRenderer : optionRenderer} itemsEqual={areEqualItems} - noResults={} + noResults={} onItemSelect={onSelectionChange} onQueryChange={(q) => setQuery(q)} resetOnQuery={false} @@ -495,8 +496,12 @@ export function SuggestField(props: SuggestFieldProps ( - - } /> + + } + /> ) : undefined diff --git a/src/components/AutocompleteField/autoCompleteFieldUtils.tsx b/src/components/AutocompleteField/autoCompleteFieldUtils.tsx index 24e9945e3..9b7d8ca7e 100644 --- a/src/components/AutocompleteField/autoCompleteFieldUtils.tsx +++ b/src/components/AutocompleteField/autoCompleteFieldUtils.tsx @@ -30,7 +30,14 @@ export const createNewItemRendererFactory = ( ); } return ( - + ); }; }; diff --git a/src/components/ContextOverlay/ContextMenu.tsx b/src/components/ContextOverlay/ContextMenu.tsx index 1515b0fd5..1966851d2 100644 --- a/src/components/ContextOverlay/ContextMenu.tsx +++ b/src/components/ContextOverlay/ContextMenu.tsx @@ -1,4 +1,5 @@ import React, { ReactElement } from "react"; +import { PopupKind } from "@blueprintjs/core"; import { CLASSPREFIX as eccgui } from "../../configuration/constants"; import { ValidIconName } from "../Icon/canonicalIconNames"; @@ -93,6 +94,7 @@ export const ContextMenu = ({ return ( { + /** + * Semantic type of the popup, matching its content role. + * When omitted, the target uses aria-haspopup="false" for generic attached content. + */ + popupKind?: BlueprintPopoverProps["popupKind"]; /** * `target` element to use as toggler for the overlay display. */ @@ -44,6 +49,7 @@ export const ContextOverlay = ({ usePlaceholder = false, paddingSize, content, + popupKind, ...otherPopoverProps }: ContextOverlayProps) => { const placeholderRef = React.useRef(null); @@ -159,6 +165,10 @@ export const ContextOverlay = ({ }, React.cloneElement(childTarget, { ...childTarget.props, + "aria-haspopup": + otherPopoverProps.interactionKind === InteractionKind.HOVER_TARGET_ONLY + ? undefined + : (popupKind ?? false), className: childTarget.props.className ?? "" + (otherPopoverProps.fill ? ` ${BlueprintClasses.FILL}` : ""), tabIndex: @@ -194,6 +204,8 @@ export const ContextOverlay = ({ ) : undefined } {...otherPopoverProps} + // Blueprint forwards this value directly to aria-haspopup, but its type excludes the valid false value. + popupKind={(popupKind ?? false) as BlueprintPopoverProps["popupKind"]} className={targetClassName} portalClassName={portalClassNameFinal.trim() ?? undefined} ref={refocus} diff --git a/src/components/ContextOverlay/tests/ContextMenu.test.tsx b/src/components/ContextOverlay/tests/ContextMenu.test.tsx index e2c909521..00ad29ad3 100644 --- a/src/components/ContextOverlay/tests/ContextMenu.test.tsx +++ b/src/components/ContextOverlay/tests/ContextMenu.test.tsx @@ -37,7 +37,10 @@ describe("ContextMenu", () => { it("if placeholder is used the menu should be displayed on click", async () => { const { container } = render(); checkForPlaceholderClass(container, 1); + expect(screen.getByRole("button")).toHaveAttribute("aria-haspopup", "menu"); fireEvent.click(container.getElementsByClassName(overlayWrapper)[0]); expect(await screen.findByText("First option")).toBeVisible(); + expect(screen.getByRole("button")).toHaveAttribute("aria-haspopup", "menu"); + expect(screen.getByRole("menu")).toBeVisible(); }); }); diff --git a/src/components/ContextOverlay/tests/ContextOverlay.test.tsx b/src/components/ContextOverlay/tests/ContextOverlay.test.tsx index 5c417dc33..2d349cbce 100644 --- a/src/components/ContextOverlay/tests/ContextOverlay.test.tsx +++ b/src/components/ContextOverlay/tests/ContextOverlay.test.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { PopoverInteractionKind } from "@blueprintjs/core"; +import { PopoverInteractionKind, PopupKind } from "@blueprintjs/core"; import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import "@testing-library/jest-dom"; @@ -17,6 +17,40 @@ const checkForPlaceholderClass = (container: HTMLElement, tobe: number) => { }; describe("ContextOverlay", () => { + it("does not announce generic attached content as a menu", () => { + render( + Attached information

}> + + , + ); + expect(screen.getByRole("button", { name: "Show information" })).toHaveAttribute("aria-haspopup", "false"); + }); + + it.each([PopupKind.MENU, PopupKind.LISTBOX, PopupKind.DIALOG])( + "announces an explicit %s before and after replacing the placeholder", + async (popupKind) => { + render( + Popup}> + + , + ); + expect(screen.getByRole("button", { name: "Open popup" })).toHaveAttribute("aria-haspopup", popupKind); + fireEvent.click(screen.getByRole("button", { name: "Open popup" })); + expect(await screen.findByText("Popup")).toBeVisible(); + expect(screen.getByRole("button", { name: "Open popup" })).toHaveAttribute("aria-haspopup", popupKind); + }, + ); + + it("does not advertise a menu to a custom target renderer", () => { + render( + Attached information

} + renderTarget={({ isOpen, ...targetProps }) => } + />, + ); + expect(screen.getByRole("button", { name: "Show information" })).toHaveAttribute("aria-haspopup", "false"); + }); + it("should not render placeholder automatically", () => { const { container } = render(); checkForPlaceholderClass(container, 0); diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index bed6fa6a9..e34b2a4e6 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -39,42 +39,37 @@ export interface MenuItemProps /** * Single item, used as child inside `Menu`. */ -export const MenuItem = ({ - children, - className = "", - icon, - onClick, - href, - text, - tooltip, - intent, - ...restProps -}: MenuItemProps) => { - return ( - - {text} - - ) : ( - text - ) - } - href={href} - onClick={(e: React.MouseEvent) => - openInNewTab(e as React.MouseEvent, onClick, href) - } - className={classNames(`${eccgui}-menu__item`, className, { - // control blueprint intent classes to enhance it by new options - [`${BlueprintClasses.getClassNamespace()}-intent-${intent}`]: intent, - })} - icon={icon ? typeof icon === "string" || Array.isArray(icon) ? : icon : false} - > - {children ?? null} - - ); -}; +export const MenuItem = React.forwardRef( + ({ children, className = "", icon, onClick, href, text, tooltip, intent, ...restProps }, ref) => { + return ( + + {text} + + ) : ( + text + ) + } + href={href} + onClick={(e: React.MouseEvent) => + openInNewTab(e as React.MouseEvent, onClick, href) + } + className={classNames(`${eccgui}-menu__item`, className, { + // control blueprint intent classes to enhance it by new options + [`${BlueprintClasses.getClassNamespace()}-intent-${intent}`]: intent, + })} + icon={icon ? typeof icon === "string" || Array.isArray(icon) ? : icon : false} + > + {children ?? null} + + ); + }, +); + +MenuItem.displayName = "MenuItem"; export default MenuItem; diff --git a/src/components/Select/Select.stories.tsx b/src/components/Select/Select.stories.tsx index 40ff4afeb..e8ca072ed 100644 --- a/src/components/Select/Select.stories.tsx +++ b/src/components/Select/Select.stories.tsx @@ -31,7 +31,7 @@ Default.args = { return { label: item }; }), itemRenderer: (item, props) => { - return ; + return ; }, fill: true, }; diff --git a/src/extensions/codemirror/tests/CodeEditor.test.tsx b/src/extensions/codemirror/tests/CodeEditor.test.tsx index 193c91306..362f3c813 100644 --- a/src/extensions/codemirror/tests/CodeEditor.test.tsx +++ b/src/extensions/codemirror/tests/CodeEditor.test.tsx @@ -59,8 +59,8 @@ describe("CodeEditor - markdown mode with toolbar", () => { fireEvent.click(getConfigMenuOverlay(container)); - const wrapLinesItem = await screen.findByText("wrapLines"); - expect(wrapLinesItem.closest("[aria-selected='true']")).not.toBeNull(); + const wrapLinesItem = await screen.findByRole("menuitemcheckbox", { name: "wrapLines" }); + expect(wrapLinesItem).toHaveAttribute("aria-checked", "true"); }); it("defaults preventLineNumbers to true in markdown mode with toolbar", async () => { @@ -68,8 +68,8 @@ describe("CodeEditor - markdown mode with toolbar", () => { fireEvent.click(getConfigMenuOverlay(container)); - const preventLineNumbersItem = await screen.findByText("preventLineNumbers"); - expect(preventLineNumbersItem.closest("[aria-selected='true']")).not.toBeNull(); + const preventLineNumbersItem = await screen.findByRole("menuitemcheckbox", { name: "preventLineNumbers" }); + expect(preventLineNumbersItem).toHaveAttribute("aria-checked", "true"); }); it("locks wrapLines in config menu when wrapLines prop is explicitly provided", async () => { diff --git a/src/extensions/codemirror/tests/EditorAppearanceConfigMenu.test.tsx b/src/extensions/codemirror/tests/EditorAppearanceConfigMenu.test.tsx index ee059da01..64e710f6f 100644 --- a/src/extensions/codemirror/tests/EditorAppearanceConfigMenu.test.tsx +++ b/src/extensions/codemirror/tests/EditorAppearanceConfigMenu.test.tsx @@ -105,7 +105,7 @@ describe("EditorAppearanceConfigMenu", () => { expect(preventLineNumbersItem.closest("[aria-disabled='true']")).toBeNull(); }); - it("selected config property is marked as selected in the menu", async () => { + it("enabled config property is exposed as a checked menu item", async () => { const config = { wrapLines: true, preventLineNumbers: false }; const setConfig = jest.fn(); @@ -113,11 +113,11 @@ describe("EditorAppearanceConfigMenu", () => { fireEvent.click(container.getElementsByClassName(contextOverlayClass)[0]); - const wrapLinesItem = await screen.findByText("wrapLines"); - expect(wrapLinesItem.closest("[aria-selected='true']")).not.toBeNull(); + const wrapLinesItem = await screen.findByRole("menuitemcheckbox", { name: "wrapLines" }); + expect(wrapLinesItem).toHaveAttribute("aria-checked", "true"); }); - it("unselected config property is not marked as selected in the menu", async () => { + it("false config property is exposed as an unchecked menu item", async () => { const config = { wrapLines: true, preventLineNumbers: false }; const setConfig = jest.fn(); @@ -125,7 +125,7 @@ describe("EditorAppearanceConfigMenu", () => { fireEvent.click(container.getElementsByClassName(contextOverlayClass)[0]); - const preventLineNumbersItem = await screen.findByText("preventLineNumbers"); - expect(preventLineNumbersItem.closest("[aria-selected='true']")).toBeNull(); + const preventLineNumbersItem = await screen.findByRole("menuitemcheckbox", { name: "preventLineNumbers" }); + expect(preventLineNumbersItem).toHaveAttribute("aria-checked", "false"); }); }); diff --git a/src/extensions/codemirror/toolbars/EditorAppearanceConfigMenu.tsx b/src/extensions/codemirror/toolbars/EditorAppearanceConfigMenu.tsx index 413242a33..bba429242 100644 --- a/src/extensions/codemirror/toolbars/EditorAppearanceConfigMenu.tsx +++ b/src/extensions/codemirror/toolbars/EditorAppearanceConfigMenu.tsx @@ -41,8 +41,9 @@ export function EditorAppearanceConfigMenu({ return ( { diff --git a/src/extensions/react-flow/handles/HandleTools.tsx b/src/extensions/react-flow/handles/HandleTools.tsx index fc904b070..0a84ae7b2 100644 --- a/src/extensions/react-flow/handles/HandleTools.tsx +++ b/src/extensions/react-flow/handles/HandleTools.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { PopoverInteractionKind as BlueprintPopoverInteractionKind } from "@blueprintjs/core"; +import { PopoverInteractionKind as BlueprintPopoverInteractionKind, PopupKind } from "@blueprintjs/core"; import { TestableComponent } from "../../../components"; import { CLASSPREFIX as eccgui } from "../../../configuration/constants"; @@ -30,6 +30,7 @@ export const HandleTools = ({ children, ...otherContextOverlayProps }: HandleToo return ( setIsOpen(false)} hoverCloseDelay={500} diff --git a/src/extensions/react-flow/tests/ToolsPopupKind.test.tsx b/src/extensions/react-flow/tests/ToolsPopupKind.test.tsx new file mode 100644 index 000000000..86ea4741a --- /dev/null +++ b/src/extensions/react-flow/tests/ToolsPopupKind.test.tsx @@ -0,0 +1,55 @@ +import React from "react"; +import { OverlaysProvider } from "@blueprintjs/core"; +import { fireEvent, render, screen } from "@testing-library/react"; + +import "@testing-library/jest-dom"; + +import Menu from "../../../components/Menu/Menu"; +import MenuItem from "../../../components/Menu/MenuItem"; +import { EdgeTools } from "../edges/EdgeTools"; +import { HandleTools } from "../handles/HandleTools"; +import { NodeTools } from "../nodes/NodeTools"; + +it("preserves the menu announcement for node actions", async () => { + render( + + + + + + + , + ); + const trigger = screen.getByRole("button"); + expect(trigger).toHaveAttribute("aria-haspopup", "menu"); + fireEvent.click(trigger); + expect(await screen.findByRole("menu")).toBeVisible(); +}); + +it("preserves the menu announcement for handle actions", async () => { + render( + + + + + + + , + ); + const trigger = screen.getByTestId("handle-actions"); + expect(trigger).toHaveAttribute("aria-haspopup", "menu"); + fireEvent.click(trigger); + expect(await screen.findByRole("menu")).toBeVisible(); +}); + +it("does not advertise a menu for edge action buttons", async () => { + const { container } = render( + + + + + , + ); + expect(await screen.findByRole("button", { name: "Delete connection" })).toBeVisible(); + expect(container.querySelector('[aria-haspopup="menu"]')).toBeNull(); +}); From d7f4319d29cc316034ccb599b8248486bab0f0dd Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Mon, 12 Oct 2026 12:15:29 +0200 Subject: [PATCH 02/19] Fix MultiSelect popup semantics and keyboard behavior - Use combobox, listbox and option semantics with active option tracking. - Label tag removal buttons and announce selection changes. - Prevent duplicate checkmarks and keyboard activation. - Restore input focus on Escape while preserving the filter. - Preserve filters and keep the popup open when toggling existing options. - Clear the filter only when creating a new tag. - Restore normal Tab and Shift+Tab navigation. - Prevent the entire dropdown from being announced on opening. --- src/components/MultiSelect/MultiSelect.tsx | 266 ++++++++++++++---- .../tests/MultiSuggestField.test.tsx | 245 +++++++++++++++- 2 files changed, 458 insertions(+), 53 deletions(-) diff --git a/src/components/MultiSelect/MultiSelect.tsx b/src/components/MultiSelect/MultiSelect.tsx index 497151d59..624b8e619 100644 --- a/src/components/MultiSelect/MultiSelect.tsx +++ b/src/components/MultiSelect/MultiSelect.tsx @@ -1,5 +1,5 @@ import React, { useRef } from "react"; -import { HTMLInputProps as BlueprintHTMLInputProps } from "@blueprintjs/core"; +import { Classes as BlueprintClasses, HTMLInputProps as BlueprintHTMLInputProps, mergeRefs } from "@blueprintjs/core"; import { ItemRendererProps as BlueprintItemRendererProps, MultiSelect as BlueprintMultiSelect, @@ -11,6 +11,7 @@ import { IntentBlueprint } from "../../common/Intent"; import { removeExtraSpaces } from "../../common/utils/stringUtils"; import { CLASSPREFIX as eccgui } from "../../configuration/constants"; import { TestableComponent } from "../interfaces"; +import { MenuItemProps } from "../Menu/MenuItem"; import { ContextOverlayProps, @@ -110,8 +111,8 @@ export interface MultiSuggestFieldCommonProps requestDelay?: number; /** - * Clear query when an option is selected or unselected. - * The query is empty then and the user need to enter a new query. + * Clear query when an existing option is selected or unselected. Defaults to false to preserve the filter. + * Creating a new item always clears the query. */ clearQueryOnSelection?: boolean; /** @@ -216,9 +217,11 @@ export function MultiSuggestField({ // Max height of the menu const [calculatedMaxHeight, setCalculatedMaxHeight] = React.useState(null); - // The active popover item is only needed for keyboard interaction and should not trigger rerenders. - const focusedItemRef = React.useRef(null); const [showSpinner, setShowSpinner] = React.useState(false); + const [selectionAnnouncement, setSelectionAnnouncement] = React.useState(""); + const [keepOpenAfterSelection, setKeepOpenAfterSelection] = React.useState(false); + const optionIds = React.useRef(new Map()); + const createdOptionId = React.useId(); const inputRef = React.useRef(null); const requestState = useRef<{ query?: string; @@ -226,12 +229,32 @@ export function MultiSuggestField({ }>({}); const selectionChange = useRef({ type: "none" }); + React.useLayoutEffect(() => { + const input = inputRef.current; + const target = input?.closest(`.${eccgui}-multiselect__target`); + if (!input || !target) return; + + // Blueprint puts the combobox role on a wrapper; active-descendant navigation needs it on the focused input. + target.removeAttribute("role"); + const updateExpandedState = () => { + const expanded = target.getAttribute("aria-expanded") ?? "false"; + input.setAttribute("aria-expanded", expanded); + if (expanded === "false") input.removeAttribute("aria-activedescendant"); + }; + updateExpandedState(); + const observer = new MutationObserver(updateExpandedState); + observer.observe(target, { attributes: true, attributeFilter: ["aria-expanded"] }); + return () => observer.disconnect(); + }, []); + /** Update external items when they change * e.g for auto-complete when query change */ React.useEffect(() => { setExternalItems(items); - setFilteredItems([...items, ...createdItems.current]); + if (!requestState.current.query || !runOnQueryChange) { + setFilteredItems(filterItems([...items, ...createdItems.current], requestState.current.query ?? "")); + } }, [items.map((item) => itemId(item)).join("|")]); React.useEffect(() => { @@ -305,6 +328,10 @@ export function MultiSuggestField({ * @param matcher */ const removeItemSelection = (matcher: string) => { + const removedItem = selectedItems.find((item) => itemId(item) === matcher); + if (removedItem) { + setSelectionAnnouncement(`Removed ${itemLabel(removedItem)}`); + } setSelectedItems((items) => { const removedItem = items.find((item) => itemId(item) === matcher); @@ -323,20 +350,35 @@ export function MultiSuggestField({ }); }; + const filterItems = (items: T[], query: string): T[] => { + if (!query) return items; + if (searchListPredicate) return searchListPredicate(items, query); + if (searchPredicate) return items.filter((item) => searchPredicate(item, query)); + return defaultSearchListPredicate(items, query); + }; + /** * selects and deselects an item from selection list * if the item exists it removes it instead * @param item */ const onItemSelect = (item: T) => { + const isNewItem = !!createNewItemFromQuery && !filteredItems.some((option) => itemId(option) === itemId(item)); + if (isNewItem && !createdItems.current.some((created) => itemId(created) === itemId(item))) { + createdItems.current = [...createdItems.current, item]; + } + inputRef.current?.removeAttribute("aria-activedescendant"); if (itemHasBeenSelectedAlready(itemId(item))) { removeItemSelection(itemId(item)); } else { + setSelectionAnnouncement(`Selected ${itemLabel(item)}`); selectionChange.current = { type: "selected", item }; setSelectedItems((items) => [...items, item]); } - if (clearQueryOnSelection) { + if (isNewItem || clearQueryOnSelection) { + // Blueprint otherwise closes a keyboard-opened popup when the cleared query reaches its QueryList. + setKeepOpenAfterSelection(true); requestState.current.query = ""; inputRef.current?.focus(); } else { @@ -361,16 +403,7 @@ export function MultiSuggestField({ if (requestState.current.query === query) { // Only use most recent request const outsideOptions = [...(resultFromQuery ?? externalItems)]; - let itemFilter = defaultSearchListPredicate; - if (searchListPredicate) { - itemFilter = searchListPredicate; - } else if (searchPredicate) { - itemFilter = (items, query) => { - return items.filter((item) => searchPredicate(item, query)); - }; - } - - setFilteredItems(itemFilter([...outsideOptions, ...createdItems.current], query)); + setFilteredItems(filterItems([...outsideOptions, ...createdItems.current], query)); setShowSpinner(false); } }; @@ -400,6 +433,7 @@ export function MultiSuggestField({ } return ( ({ */ const handleClear = () => { requestState.current.query = ""; + setSelectionAnnouncement("Selection cleared"); selectionChange.current = { type: "none" }; setSelectedItems([]); @@ -427,6 +462,10 @@ export function MultiSuggestField({ * @param index */ const removeTagFromSelectionViaIndex = (_label: React.ReactNode, index: number) => { + const removedItem = selectedItems[index]; + if (removedItem) { + setSelectionAnnouncement(`Removed ${itemLabel(removedItem)}`); + } setSelectedItems((items) => { const removedItem = items[index]; @@ -453,6 +492,9 @@ export function MultiSuggestField({ * @param event */ const handleOnKeyUp = (event: React.KeyboardEvent) => { + if (event.target !== inputRef.current) { + return; + } if (event.key === "Enter" && !filteredItems.length && !!requestState.current.query && createNewItemFromQuery) { if (!isValidNewOption || isValidNewOption(requestState.current.query)) { createNewItem(requestState.current.query); @@ -461,28 +503,6 @@ export function MultiSuggestField({ inputRef.current?.focus(); }; - /** - * added functionality to either create new item - * when there are no matching items or select an item on tab keypress - * @param event - */ - const handleOnKeyDown = (event: React.KeyboardEvent) => { - if (event.key === "Tab" && !!requestState.current.query) { - event.preventDefault(); - if (focusedItemRef.current) { - onItemSelect(focusedItemRef.current); - } else { - if (!isValidNewOption || isValidNewOption(requestState.current.query)) { - onItemSelect(createNewItem(requestState.current.query)); - } else { - return; - } - } - requestState.current.query = ""; - setTimeout(() => inputRef.current?.focus()); - } - }; - /** * create new item handler, displays the new item selector and creates a new item when selected * @param label ' @@ -498,22 +518,126 @@ export function MultiSuggestField({ }; return ( {`${newItemCreationText} '${label}'`}} /> ); }; + const handleOptionKeyDown = (event: React.KeyboardEvent) => { + if (!event.defaultPrevented && !event.nativeEvent.isComposing && ["Enter", " "].includes(event.key)) { + event.preventDefault(); + // Clicking focuses the input; prevent QueryList from selecting again on the ensuing input keyup. + event.stopPropagation(); + event.currentTarget.click(); + } + }; + + const renderOption = (item: T, props: BlueprintItemRendererProps) => { + const rendered = otherMultiSelectProps.itemRenderer + ? otherMultiSelectProps.itemRenderer(item, props) + : onItemRenderer(item, props); + if (!React.isValidElement(rendered) || rendered.type !== MenuItem) { + return rendered; + } + const optionId = props.id ?? `${createdOptionId}-item-${props.index}`; + const isSelected = itemHasBeenSelectedAlready(itemId(item)); + const hasSelectionIcon = rendered.props.icon === "state-checked" || rendered.props.icon === "state-unchecked"; + optionIds.current.set(itemId(item), optionId); + return React.cloneElement(rendered, { + roleStructure: "listoption", + onKeyDown: (event) => { + rendered.props.onKeyDown?.(event); + handleOptionKeyDown(event); + }, + // Blueprint adds its own tick for selected options, which duplicates our checkbox icon. + selected: isSelected && !hasSelectionIcon, + ref: mergeRefs(props.ref, (element: HTMLLIElement | null) => { + if (element) { + element.id = optionId; + element.setAttribute("aria-label", itemLabel(item)); + element.setAttribute("aria-selected", String(isSelected)); + if (element.parentElement?.id) { + inputRef.current?.setAttribute("aria-controls", element.parentElement.id); + } + if (props.modifiers.active) { + inputRef.current?.setAttribute("aria-activedescendant", optionId); + } + } + }), + }); + }; + + const renderCreateOption = (label: string, active: boolean, handleClick: React.MouseEventHandler) => { + const rendered = otherMultiSelectProps.createNewItemRenderer + ? otherMultiSelectProps.createNewItemRenderer(label, active, handleClick) + : newItemRenderer(label, active, handleClick); + return rendered && React.isValidElement(rendered) && rendered.type === MenuItem + ? React.cloneElement(rendered, { + roleStructure: "listoption", + onKeyDown: (event) => { + rendered.props.onKeyDown?.(event); + handleOptionKeyDown(event); + }, + selected: false, + ref: (element: HTMLLIElement | null) => { + if (element) { + element.id = createdOptionId; + if (element.parentElement?.id) { + inputRef.current?.setAttribute("aria-controls", element.parentElement.id); + } + if (active) { + inputRef.current?.setAttribute("aria-activedescendant", createdOptionId); + } + } + }, + }) + : rendered; + }; + + const providedTagProps = tagInputProps?.tagProps; + const getTagProps: NonNullable["tagInputProps"]>["tagProps"] = (tag, index) => { + const props = typeof providedTagProps === "function" ? providedTagProps(tag, index) : (providedTagProps ?? {}); + return { + minimal: true, + ...props, + onKeyDownCapture: (event: React.KeyboardEvent) => { + props.onKeyDownCapture?.(event); + if ((event.target as HTMLElement).matches(`.${BlueprintClasses.TAG_REMOVE}`)) { + event.stopPropagation(); + } + }, + onKeyUpCapture: (event: React.KeyboardEvent) => { + props.onKeyUpCapture?.(event); + if ((event.target as HTMLElement).matches(`.${BlueprintClasses.TAG_REMOVE}`)) { + event.stopPropagation(); + } + }, + ref: mergeRefs(typeof props.ref === "string" ? undefined : props.ref, (element: HTMLSpanElement | null) => { + // Blueprint hardcodes the same accessible name for every tag's remove button. + const removeButton = element?.querySelector(`.${BlueprintClasses.TAG_REMOVE}`); + if (removeButton && selectedItems[index]) { + removeButton.setAttribute("aria-label", `Removing tag '${itemLabel(selectedItems[index])}'`); + } + }), + }; + }; + // Clear button and spinner are both shown as "right element" const clearButton = selectedItems.length > 0 ? ( ({ onQueryChange={onQueryChange} items={filteredItems} onItemSelect={onItemSelect} - itemRenderer={onItemRenderer} itemsEqual={(a: T, b: T) => itemId(a) === itemId(b)} selectedItems={selectedItems} - noResults={} + noResults={} tagRenderer={(item) => itemLabel(item)} - createNewItemRenderer={newItemRenderer} - onActiveItemChange={(activeItem) => { - focusedItemRef.current = activeItem; - }} fill={fullWidth} createNewItemFromQuery={createNewItemFromQuery} disabled={disabled} @@ -553,9 +672,16 @@ export function MultiSuggestField({ inputProps: { id: "item", autoComplete: "off", + role: "combobox", + "aria-haspopup": "listbox", + "aria-autocomplete": "list", "data-test-id": dataTestId ? dataTestId + "_searchinput" : undefined, "data-testid": dataTestid ? dataTestid + "_searchinput" : undefined, ...inputProps, + onBlur: (event) => { + event.currentTarget.removeAttribute("aria-activedescendant"); + inputProps?.onBlur?.(event); + }, } as React.InputHTMLAttributes, className: classNames(`${eccgui}-multisuggestfield`, `${eccgui}-multiselect`, className, { [`${eccgui}-intent--${intent}`]: intent === "accent", @@ -564,7 +690,6 @@ export function MultiSuggestField({ inputRef: inputRef, intent: intent && intent !== "accent" ? intent : undefined, addOnBlur: true, - onKeyDown: handleOnKeyDown, onKeyUp: handleOnKeyUp, onRemove: removeTagFromSelectionViaIndex, rightElement: ( @@ -574,25 +699,34 @@ export function MultiSuggestField({ )} ), - tagProps: { minimal: true }, disabled, ...tagInputProps, + tagProps: getTagProps, ...spinnerProps, }} - popoverTargetProps={{ - className: `${eccgui}-multiselect__target`, - }} popoverProps={{ minimal: true, placement: "bottom-start", matchTargetWidth: fullWidth, ...contextOverlayProps, + onClosing: (node) => { + setKeepOpenAfterSelection(false); + contextOverlayProps?.onClosing?.(node); + }, + onOpening: (node) => { + // Blueprint makes the whole overlay live, which announces every option when it opens. + node.closest(`.${BlueprintClasses.OVERLAY}`)?.setAttribute("aria-live", "off"); + contextOverlayProps?.onOpening?.(node); + }, }} popoverContentProps={ { @@ -606,6 +740,28 @@ export function MultiSuggestField({ } as BlueprintMultiSelectProps["popoverContentProps"] } {...otherMultiSelectProps} + openOnKeyDown={keepOpenAfterSelection ? false : otherMultiSelectProps.openOnKeyDown} + popoverTargetProps={{ + className: `${eccgui}-multiselect__target`, + onKeyDown: (event) => { + // Blueprint explicitly blurs on Escape before invoking this handler. + if (event.key === "Escape") inputRef.current?.focus(); + }, + }} + itemRenderer={renderOption} + createNewItemRenderer={renderCreateOption} + onActiveItemChange={(activeItem, isCreateNewItem) => { + const optionId = isCreateNewItem + ? createdOptionId + : activeItem + ? optionIds.current.get(itemId(activeItem)) + : undefined; + if (optionId) { + inputRef.current?.setAttribute("aria-activedescendant", optionId); + } else { + inputRef.current?.removeAttribute("aria-activedescendant"); + } + }} /> ); @@ -616,8 +772,16 @@ export function MultiSuggestField({ {...{ "data-test-id": dataTestId, "data-testid": dataTestid }} > {contentMultiSelect} + + {selectionAnnouncement} + ) : ( - <>{contentMultiSelect} + <> + {contentMultiSelect} + + {selectionAnnouncement} + + ); } diff --git a/src/components/MultiSuggestField/tests/MultiSuggestField.test.tsx b/src/components/MultiSuggestField/tests/MultiSuggestField.test.tsx index 57bd31a13..2b62fe457 100644 --- a/src/components/MultiSuggestField/tests/MultiSuggestField.test.tsx +++ b/src/components/MultiSuggestField/tests/MultiSuggestField.test.tsx @@ -1,9 +1,10 @@ import React from "react"; -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import "@testing-library/jest-dom"; -import { MultiSuggestField } from "../../../../index"; +import { MenuItem, MultiSuggestField } from "../../../../index"; import { CustomSearch, Default, dropdownOnFocus, predefinedNotControlledValues } from "../MultiSuggestField.stories"; //const testLabels = ["label1", "label2", "label3", "label4", "label5"]; @@ -14,6 +15,241 @@ const items = new Array(50).fill(undefined).map((_, id) => { }); describe("MultiSuggestField", () => { + it.each([ + { query: "1", shift: true }, + { query: "new tag", shift: true }, + { query: "1", shift: false }, + { query: "new tag", shift: false }, + ])("moves focus without changing the selection for filter '$query' (Shift: $shift)", async ({ query, shift }) => { + const onSelection = jest.fn(); + render( + <> + + ({ testId: label, testLabel: label })} + /> + + , + ); + const input = screen.getByRole("combobox"); + await userEvent.click(input); + await userEvent.type(input, query); + await screen.findByRole("option", { name: `Add new item '${query}'` }); + await userEvent.keyboard("{ArrowDown}"); + onSelection.mockClear(); + await userEvent.tab({ shift }); + + expect(screen.getByRole("button", { name: shift ? "Previous control" : "Next control" })).toHaveFocus(); + expect(input).toHaveValue(query); + expect(onSelection).not.toHaveBeenCalled(); + await waitFor(() => expect(screen.queryByRole("listbox")).not.toBeInTheDocument()); + }); + + it.each([false, true])( + "closes on Escape and returns focus to the input (option focused: %s)", + async (focusOption) => { + render(); + const input = screen.getByRole("combobox"); + await userEvent.click(input); + await userEvent.type(input, "1"); + await screen.findByRole("listbox"); + await userEvent.keyboard("{ArrowDown}"); + if (focusOption) act(() => screen.getAllByRole("option")[0].querySelector("a")!.focus()); + await userEvent.keyboard("{Escape}"); + + await waitFor(() => expect(screen.queryByRole("listbox")).not.toBeInTheDocument()); + await waitFor(() => expect(input).toHaveFocus()); + expect(input).toHaveValue("1"); + expect(input).not.toHaveAttribute("aria-activedescendant"); + }, + ); + + it("keeps the filter when selecting and deselecting existing entries with Enter", async () => { + render(); + const input = screen.getByRole("combobox"); + await userEvent.click(input); + await userEvent.type(input, "1"); + await screen.findAllByRole("option"); + await userEvent.keyboard("{ArrowDown}{Enter}"); + + expect(screen.getByRole("listbox")).toBeInTheDocument(); + expect(input).toHaveValue("1"); + const selectedOption = screen + .getAllByRole("option") + .find((option) => option.getAttribute("aria-selected") === "true"); + expect(selectedOption).toBeDefined(); + act(() => selectedOption!.querySelector("a")!.focus()); + await userEvent.keyboard("{Enter}"); + expect(selectedOption).toHaveAttribute("aria-selected", "false"); + expect(input).toHaveValue("1"); + expect(screen.getAllByRole("option").every((option) => option.textContent?.includes("1"))).toBe(true); + expect(screen.getByRole("listbox")).toBeInTheDocument(); + expect(input).toHaveFocus(); + }); + + it("keeps the popup open when activating a filtered create-option with Enter", async () => { + render( + ({ testId: label, testLabel: label })} + />, + ); + const input = screen.getByRole("combobox"); + await userEvent.click(input); + await userEvent.type(input, "new tag"); + const option = await screen.findByRole("option", { name: "Add new item 'new tag'" }); + act(() => option.querySelector("a")!.focus()); + await userEvent.keyboard("{Enter}"); + + await waitFor(() => expect(input).toHaveValue("")); + expect(screen.getByRole("combobox")).toHaveAttribute("aria-expanded", "true"); + expect(screen.getByRole("listbox")).toBeInTheDocument(); + expect(input).toHaveFocus(); + }); + + it.each([false, true])("clears the filter on keyboard creation (existing matches: %s)", async (existingMatches) => { + const onSelection = jest.fn(); + render( + ({ testId: label, testLabel: label })} + />, + ); + const input = screen.getByRole("combobox"); + await userEvent.click(input); + await userEvent.type(input, "new tag"); + await screen.findByRole("option", { name: "Add new item 'new tag'" }); + if (existingMatches) await userEvent.keyboard("{ArrowDown}"); + await userEvent.keyboard("{Enter}"); + + expect(input).toHaveValue(""); + expect(input).toHaveFocus(); + expect(screen.getByRole("listbox")).toBeInTheDocument(); + expect(onSelection).toHaveBeenLastCalledWith( + expect.objectContaining({ + selectedItems: [{ testId: "new tag", testLabel: "new tag" }], + createdItems: [{ testId: "new tag", testLabel: "new tag" }], + }), + ); + }); + + it("exposes one combobox on the focused input and a multi-select listbox", async () => { + render(); + const input = screen.getByRole("combobox"); + expect(input.tagName).toBe("INPUT"); + expect(input).toHaveAttribute("aria-expanded", "false"); + await userEvent.click(input); + await userEvent.keyboard("{Enter}{ArrowDown}"); + await waitFor(() => expect(input).toHaveAttribute("aria-expanded", "true")); + expect(screen.getAllByRole("combobox")).toHaveLength(1); + expect(input).toHaveFocus(); + expect(input).toHaveAttribute("aria-haspopup", "listbox"); + expect(screen.getByRole("listbox")).toHaveAttribute("aria-multiselectable", "true"); + expect(screen.getByRole("listbox").closest(".bp6-overlay")).toHaveAttribute("aria-live", "off"); + await userEvent.keyboard("{Escape}"); + await waitFor(() => expect(input).toHaveAttribute("aria-expanded", "false")); + }); + + it("removes a focused tag with Enter and names the remove button for its tag", async () => { + const onSelection = jest.fn(); + const { container } = render( + , + ); + const tag = predefinedNotControlledValues.args.selectedItems[0]; + const removeButton = container.querySelector("[data-tag-index='0'] button") as HTMLButtonElement; + const removeButtonName = removeButton.getAttribute("aria-label"); + + act(() => removeButton.focus()); + await userEvent.keyboard("{Enter}"); + + expect(container.querySelector("[data-tag-index='0']")).toHaveTextContent( + predefinedNotControlledValues.args.selectedItems[1].testLabel, + ); + expect(container.querySelectorAll("[data-tag-index]")).toHaveLength(1); + expect(removeButtonName).toBe(`Removing tag '${tag.testLabel}'`); + expect(onSelection).toHaveBeenLastCalledWith( + expect.objectContaining({ + newlyRemoved: tag, + selectedItems: predefinedNotControlledValues.args.selectedItems.slice(1), + }), + ); + }); + + it("exposes the active listbox option to the focused input", async () => { + render(); + const input = screen.getByRole("combobox"); + await userEvent.click(input); + await userEvent.keyboard("{ArrowDown}"); + + const activeOptionId = input.getAttribute("aria-activedescendant"); + expect(activeOptionId).toBeTruthy(); + expect(document.getElementById(activeOptionId!)).toHaveAttribute("role", "option"); + + await userEvent.keyboard("{ArrowDown}"); + expect(input.getAttribute("aria-activedescendant")).not.toBe(activeOptionId); + }); + + it("announces the selected item after choosing it with Enter", async () => { + render(); + const input = screen.getByRole("combobox"); + await userEvent.click(input); + await userEvent.keyboard("{ArrowDown}{Enter}"); + + expect(screen.getByRole("status")).toHaveTextContent(/^Selected /); + }); + + it("connects the focused input to the listbox and labels its auxiliary buttons", async () => { + render(); + const input = screen.getByRole("combobox"); + await userEvent.click(input); + await userEvent.keyboard("{Enter}"); + + expect(input).toHaveAttribute("aria-controls", screen.getByRole("listbox").id); + expect(input).toHaveAttribute("aria-autocomplete", "list"); + expect(screen.getByRole("button", { name: "Clear all selected items" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Show options" })).toBeInTheDocument(); + }); + + it("keeps custom option and tag props while exposing option selection", async () => { + const selected = predefinedNotControlledValues.args.selectedItems[0]; + render( + ( + + )} + tagInputProps={{ tagProps: { intent: "warning" } }} + />, + ); + const input = screen.getByRole("combobox"); + await userEvent.click(input); + await userEvent.keyboard("{Enter}"); + await userEvent.keyboard("{ArrowDown}"); + + expect(document.getElementById(input.getAttribute("aria-activedescendant")!)).toHaveAttribute("role", "option"); + const option = screen.getByRole("option", { name: selected.testLabel }); + expect(option).toHaveAttribute("aria-selected", "true"); + expect(option.querySelector(".bp6-menu-item-selected-icon")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: `Removing tag '${selected.testLabel}'` })).toBeInTheDocument(); + }); + + it("shows only the checkbox icon for a selected option", async () => { + const selected = predefinedNotControlledValues.args.selectedItems[0]; + render(); + await userEvent.click(screen.getByRole("combobox")); + await userEvent.keyboard("{Enter}"); + + const option = screen.getByRole("option", { name: selected.testLabel }); + expect(option).toHaveAttribute("aria-selected", "true"); + expect(option.querySelector(".bp6-menu-item-icon")).toBeInTheDocument(); + expect(option.querySelector(".bp6-menu-item-selected-icon")).not.toBeInTheDocument(); + }); + describe("uncontrolled (when only selectedItems or onSelect is provided)", () => { it("should render default input", () => { const { container } = render(); @@ -71,6 +307,9 @@ describe("MultiSuggestField", () => { const listbox = screen.getByRole("listbox"); expect(listbox).toBeInTheDocument(); + expect(listbox.querySelectorAll('[role="option"]')).toHaveLength(dropdownOnFocus.args.items.length); + expect(listbox.querySelectorAll('[role="menuitem"]')).toHaveLength(0); + const menuItems = listbox.getElementsByClassName("eccgui-menu__item"); expect(menuItems.length).toBe(dropdownOnFocus.args.items.length); }); @@ -91,6 +330,8 @@ describe("MultiSuggestField", () => { await waitFor(() => { const listbox = screen.getByRole("listbox"); + expect(listbox.querySelectorAll('[role="option"]')).toHaveLength(1); + expect(listbox.querySelectorAll('[role="menuitem"]')).toHaveLength(0); const menuItems = listbox.getElementsByClassName("eccgui-menu__item"); expect(menuItems.length).toBe(1); From bb7766f282ab0d021a6dcee3b886b8f512531ed4 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Tue, 13 Oct 2026 14:56:51 +0200 Subject: [PATCH 03/19] Fix MultiSelect option merging and query reset behavior - Deduplicate supplied, remote and newly created options by ID. - Allow external option updates while preserving remote search results. - Recognise existing entries and remove duplicate creation handlers. - Preserve supplementary details in custom option accessible names. - Cancel pending searches and stop loading when resetting the query. --- CHANGELOG.md | 4 + src/components/MultiSelect/MultiSelect.tsx | 85 +++-- .../MultiSuggestField.regressions.test.tsx | 298 ++++++++++++++++++ 3 files changed, 338 insertions(+), 49 deletions(-) create mode 100644 src/components/MultiSuggestField/tests/MultiSuggestField.regressions.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 370e45ecd..ae55710d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p - Added explicitly `assert` dependency - the linter of `` 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 - Fix misused menu roles in selection popups and overlays +- `` + - improve keyboard navigation, tag removal and focus handling + - improve screen reader labels and selection announcements + - preserve filters and keep the dropdown open when toggling existing options ### Deprecated diff --git a/src/components/MultiSelect/MultiSelect.tsx b/src/components/MultiSelect/MultiSelect.tsx index 624b8e619..adef53ea1 100644 --- a/src/components/MultiSelect/MultiSelect.tsx +++ b/src/components/MultiSelect/MultiSelect.tsx @@ -6,6 +6,7 @@ import { MultiSelectProps as BlueprintMultiSelectProps, } from "@blueprintjs/select"; import classNames from "classnames"; +import { uniqBy } from "lodash"; import { IntentBlueprint } from "../../common/Intent"; import { removeExtraSpaces } from "../../common/utils/stringUtils"; @@ -207,7 +208,8 @@ export function MultiSuggestField({ // Options created by a user const createdItems = useRef([]); // Options passed outside (f.e. from the backend) - const [externalItems, setExternalItems] = React.useState([...items]); + const externalItems = useRef(items); + externalItems.current = items; // All options (created and passed) that match the query const [filteredItems, setFilteredItems] = React.useState([]); // All options (created and passed) selected by a user @@ -226,8 +228,11 @@ export function MultiSuggestField({ const requestState = useRef<{ query?: string; timeoutId?: number; + queryItems?: T[]; }>({}); const selectionChange = useRef({ type: "none" }); + const availableItems = (options: T[] = externalItems.current): T[] => + uniqBy([...options, ...createdItems.current], itemId); React.useLayoutEffect(() => { const input = inputRef.current; @@ -251,10 +256,9 @@ export function MultiSuggestField({ * e.g for auto-complete when query change */ React.useEffect(() => { - setExternalItems(items); - if (!requestState.current.query || !runOnQueryChange) { - setFilteredItems(filterItems([...items, ...createdItems.current], requestState.current.query ?? "")); - } + setFilteredItems( + filterItems(availableItems(requestState.current.queryItems ?? items), requestState.current.query ?? ""), + ); }, [items.map((item) => itemId(item)).join("|")]); React.useEffect(() => { @@ -357,14 +361,29 @@ export function MultiSuggestField({ return defaultSearchListPredicate(items, query); }; + const resetQuery = () => { + requestState.current.query = ""; + requestState.current.queryItems = undefined; + if (requestState.current.timeoutId) { + clearTimeout(requestState.current.timeoutId); + requestState.current.timeoutId = undefined; + } + setShowSpinner(false); + setFilteredItems(availableItems()); + }; + /** * selects and deselects an item from selection list * if the item exists it removes it instead * @param item */ const onItemSelect = (item: T) => { - const isNewItem = !!createNewItemFromQuery && !filteredItems.some((option) => itemId(option) === itemId(item)); - if (isNewItem && !createdItems.current.some((created) => itemId(created) === itemId(item))) { + const isNewItem = + !!createNewItemFromQuery && + ![...availableItems(), ...filteredItems, ...selectedItems].some( + (option) => itemId(option) === itemId(item), + ); + if (isNewItem) { createdItems.current = [...createdItems.current, item]; } inputRef.current?.removeAttribute("aria-activedescendant"); @@ -379,7 +398,7 @@ export function MultiSuggestField({ if (isNewItem || clearQueryOnSelection) { // Blueprint otherwise closes a keyboard-opened popup when the cleared query reaches its QueryList. setKeepOpenAfterSelection(true); - requestState.current.query = ""; + resetQuery(); inputRef.current?.focus(); } else { inputRef.current?.select(); @@ -393,6 +412,7 @@ export function MultiSuggestField({ const onQueryChange = (query: string) => { if (query.length && query !== requestState.current.query) { requestState.current.query = query; + requestState.current.queryItems = undefined; if (requestState.current.timeoutId) { clearTimeout(requestState.current.timeoutId); } @@ -402,16 +422,15 @@ export function MultiSuggestField({ const resultFromQuery = runOnQueryChange && (await runOnQueryChange(removeExtraSpaces(query))); if (requestState.current.query === query) { // Only use most recent request - const outsideOptions = [...(resultFromQuery ?? externalItems)]; - setFilteredItems(filterItems([...outsideOptions, ...createdItems.current], query)); + requestState.current.queryItems = resultFromQuery; + setFilteredItems(filterItems(availableItems(resultFromQuery ?? externalItems.current), query)); setShowSpinner(false); } }; requestState.current.timeoutId = window.setTimeout(fn, requestDelay && requestDelay > 0 ? requestDelay : 0); } else if (!query.length) { // if the query is empty we need to show all options and reset current query - requestState.current.query = ""; - setFilteredItems(() => [...externalItems, ...createdItems.current]); + resetQuery(); } }; @@ -448,12 +467,11 @@ export function MultiSuggestField({ * clear all selected items in the multi-select input */ const handleClear = () => { - requestState.current.query = ""; + resetQuery(); setSelectionAnnouncement("Selection cleared"); selectionChange.current = { type: "none" }; setSelectedItems([]); - setFilteredItems([...externalItems, ...createdItems.current]); }; /** @@ -475,34 +493,6 @@ export function MultiSuggestField({ }); }; - /** - * Utility function to create a new Item. createNewItemFromQuery is assumed to be defined! - */ - const createNewItem = (query: string): T => { - const newItem = createNewItemFromQuery!(query); - //set new items - createdItems.current = [...createdItems.current, newItem]; - setFilteredItems((items) => [...items, newItem]); - requestState.current.query = ""; - return newItem; - }; - - /** - * added functionality to create new item when there are no matching items on enter keypress - * @param event - */ - const handleOnKeyUp = (event: React.KeyboardEvent) => { - if (event.target !== inputRef.current) { - return; - } - if (event.key === "Enter" && !filteredItems.length && !!requestState.current.query && createNewItemFromQuery) { - if (!isValidNewOption || isValidNewOption(requestState.current.query)) { - createNewItem(requestState.current.query); - } - } - inputRef.current?.focus(); - }; - /** * create new item handler, displays the new item selector and creates a new item when selected * @param label ' @@ -512,10 +502,6 @@ export function MultiSuggestField({ */ const newItemRenderer = (label: string, active: boolean, handleClick: React.MouseEventHandler) => { if (!createNewItemFromQuery || (isValidNewOption && !isValidNewOption(label))) return undefined; - const clickHandler = (e: React.MouseEvent) => { - createNewItem(label); - handleClick(e); - }; return ( ({ icon="item-add-artefact" active={active} key={label} - onClick={clickHandler} + onClick={handleClick} shouldDismissPopover={false} text={{`${newItemCreationText} '${label}'`}} /> @@ -561,7 +547,9 @@ export function MultiSuggestField({ ref: mergeRefs(props.ref, (element: HTMLLIElement | null) => { if (element) { element.id = optionId; - element.setAttribute("aria-label", itemLabel(item)); + if (!otherMultiSelectProps.itemRenderer) { + element.setAttribute("aria-label", itemLabel(item)); + } element.setAttribute("aria-selected", String(isSelected)); if (element.parentElement?.id) { inputRef.current?.setAttribute("aria-controls", element.parentElement.id); @@ -690,7 +678,6 @@ export function MultiSuggestField({ inputRef: inputRef, intent: intent && intent !== "accent" ? intent : undefined, addOnBlur: true, - onKeyUp: handleOnKeyUp, onRemove: removeTagFromSelectionViaIndex, rightElement: ( <> diff --git a/src/components/MultiSuggestField/tests/MultiSuggestField.regressions.test.tsx b/src/components/MultiSuggestField/tests/MultiSuggestField.regressions.test.tsx new file mode 100644 index 000000000..cb94adb38 --- /dev/null +++ b/src/components/MultiSuggestField/tests/MultiSuggestField.regressions.test.tsx @@ -0,0 +1,298 @@ +import React from "react"; +import { act, fireEvent, render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import "@testing-library/jest-dom"; + +import { MenuItem, MultiSuggestField, MultiSuggestFieldSelectionProps } from "../../../../index"; + +interface Option { + id: string; + label: string; +} + +const itemId = (item: Option) => item.id; +const itemLabel = (item: Option) => item.label; +const createItem = (query: string): Option => ({ id: query, label: query }); +const requestDelay = 200; + +const deferredOptions = () => { + let resolve!: (items: Option[]) => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +}; + +const advanceTime = async (milliseconds = requestDelay) => { + await act(async () => { + jest.advanceTimersByTime(milliseconds); + }); +}; + +describe("MultiSuggestField regressions", () => { + beforeEach(() => jest.useFakeTimers()); + + afterEach(() => { + act(() => jest.runOnlyPendingTimers()); + jest.useRealTimers(); + }); + + it("shows created options once when the parent feeds selected values back as items", async () => { + const SelectionField = () => { + const [selected, setSelected] = React.useState([]); + const onSelection = React.useCallback(({ selectedItems }: MultiSuggestFieldSelectionProps