From a31f877fb8d3ad434ee74baba29c7aaf169104c5 Mon Sep 17 00:00:00 2001 From: Juan Andrade Date: Tue, 15 Oct 2024 10:45:30 -0400 Subject: [PATCH 1/5] Dropdown: use combobox role (instead of button) --- .../__tests__/multi-select.test.tsx | 66 +++++++++---------- .../__tests__/select-opener.test.tsx | 12 ++-- .../__tests__/single-select.test.tsx | 48 +++++++------- .../src/components/action-menu.tsx | 1 + .../src/components/dropdown-opener.tsx | 8 +++ .../src/components/select-opener.tsx | 9 ++- .../src/components/single-select.tsx | 1 + 7 files changed, 81 insertions(+), 64 deletions(-) diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx index 2989923bf..129728aa1 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx @@ -71,7 +71,7 @@ describe("MultiSelect", () => { const {userEvent} = doRender(uncontrolledMultiSelect); // Act - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect( @@ -103,7 +103,7 @@ describe("MultiSelect", () => { doRender(uncontrolledMultiSelect); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert // No items are selected, display placeholder because there is one @@ -127,7 +127,7 @@ describe("MultiSelect", () => { ); // Assert - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "item 1", ); }); @@ -149,7 +149,7 @@ describe("MultiSelect", () => { ); // Assert - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "1 student", ); }); @@ -171,7 +171,7 @@ describe("MultiSelect", () => { ); // Assert - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "Choose", ); }); @@ -194,7 +194,7 @@ describe("MultiSelect", () => { // Assert // More than one item is selected, display n itemTypes - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "2 students", ); }); @@ -217,7 +217,7 @@ describe("MultiSelect", () => { // Assert // All items are selected - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "All students", ); }); @@ -240,7 +240,7 @@ describe("MultiSelect", () => { ); // Assert - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "All students", ); }); @@ -259,7 +259,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("data-testid", "some-test-id"); @@ -524,7 +524,7 @@ describe("MultiSelect", () => { const {userEvent} = doRender(); - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Act @@ -597,7 +597,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Act const searchInput = await screen.findByPlaceholderText("Filter"); @@ -610,7 +610,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); @@ -627,7 +627,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); @@ -658,7 +658,7 @@ describe("MultiSelect", () => { // Act // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect( @@ -681,7 +681,7 @@ describe("MultiSelect", () => { , ); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); @@ -696,7 +696,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Act const searchInput = await screen.findByPlaceholderText("Filter"); @@ -723,7 +723,7 @@ describe("MultiSelect", () => { // Act // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect( @@ -747,7 +747,7 @@ describe("MultiSelect", () => { , ); - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); await userEvent.type(searchInput, "Item 2"); @@ -779,7 +779,7 @@ describe("MultiSelect", () => { , ); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); await userEvent.type(searchInput, "some text"); @@ -796,7 +796,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); await userEvent.type(searchInput, "Should be cleared"); @@ -814,7 +814,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const searchInput = await screen.findByPlaceholderText("Filter"); @@ -1430,7 +1430,7 @@ describe("MultiSelect", () => { rerender(); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // open dropdown await userEvent.click(opener); @@ -1646,7 +1646,7 @@ describe("MultiSelect", () => { // Act // Press the button - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -1674,7 +1674,7 @@ describe("MultiSelect", () => { // Act // Press the button - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -1789,7 +1789,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute( @@ -1808,7 +1808,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("id", id); @@ -1824,7 +1824,7 @@ describe("MultiSelect", () => { // Act // Open the dropdown - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert @@ -1847,7 +1847,7 @@ describe("MultiSelect", () => { // Act // Open the dropdown - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert @@ -1869,7 +1869,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const dropdown = await screen.findByRole("listbox", {hidden: true}); @@ -1888,7 +1888,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const dropdown = await screen.findByRole("listbox", {hidden: true}); @@ -1965,7 +1965,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("aria-haspopup", "listbox"); @@ -2004,7 +2004,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("aria-expanded", "false"); @@ -2020,7 +2020,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx index 8c7e461f8..854d8af7c 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx @@ -26,7 +26,7 @@ describe("SelectOpener", () => { // Act // Press the button. - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect(onOpenMock).toHaveBeenCalledTimes(1); @@ -51,7 +51,7 @@ describe("SelectOpener", () => { // Act // Press the button. - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -87,7 +87,7 @@ describe("SelectOpener", () => { // Act // Press the button. - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -131,7 +131,7 @@ describe("SelectOpener", () => { // Act // Press the button. - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect(onOpenMock).toHaveBeenCalledTimes(0); @@ -156,7 +156,7 @@ describe("SelectOpener", () => { // Act // Press the button. - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -192,7 +192,7 @@ describe("SelectOpener", () => { // Act // Press the button. - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx index deb11b651..a1f53ae64 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx @@ -61,7 +61,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveTextContent("Default placeholder"); @@ -82,7 +82,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveTextContent(""); @@ -102,7 +102,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveTextContent("Toggle A"); @@ -130,7 +130,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveTextContent("Plain Toggle A"); @@ -220,13 +220,13 @@ describe("SingleSelect", () => { ); // Act - await userEvent.click(await screen.findByRole("textbox")); + await userEvent.click(await screen.findByRole("combobox")); // wait for the dropdown to open await screen.findByRole("listbox", {hidden: true}); // Assert - expect(await screen.findByRole("textbox")).toHaveFocus(); + expect(await screen.findByRole("combobox")).toHaveFocus(); }); }); @@ -412,7 +412,7 @@ describe("SingleSelect", () => { // Act await userEvent.click( - await screen.findByRole("button", {name: "Choose"}), + await screen.findByRole("combobox", {name: /choose/i}), ); // Assert @@ -427,7 +427,7 @@ describe("SingleSelect", () => { ); // open the menu from the outside await userEvent.click( - await screen.findByRole("button", {name: "Choose"}), + await screen.findByRole("combobox", {name: /choose/i}), ); // Act @@ -471,7 +471,9 @@ describe("SingleSelect", () => { const userEvent = doRender(); // Act - const opener = await screen.findByRole("button", {name: "Choose"}); + const opener = await screen.findByRole("combobox", { + name: /choose/i, + }); // open the menu from the outside await userEvent.click(opener); // click on the dropdown anchor to hide the menu @@ -581,7 +583,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // open dropdown await userEvent.click(opener); @@ -626,7 +628,7 @@ describe("SingleSelect", () => { const userEvent = doRender(); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // open dropdown await userEvent.click(opener); await userEvent.click(await screen.findByText("Toggle B")); @@ -781,7 +783,7 @@ describe("SingleSelect", () => { , ); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); await userEvent.click(searchInput); @@ -1199,7 +1201,7 @@ describe("SingleSelect", () => { // Act // Press the button - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -1230,7 +1232,7 @@ describe("SingleSelect", () => { // Act // Press the button - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -1352,7 +1354,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute( @@ -1371,7 +1373,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("id", id); @@ -1387,7 +1389,7 @@ describe("SingleSelect", () => { // Act // Open the dropdown - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert @@ -1414,7 +1416,7 @@ describe("SingleSelect", () => { // Act // Open the dropdown - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert @@ -1440,7 +1442,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const dropdown = await screen.findByRole("listbox", {hidden: true}); @@ -1459,7 +1461,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const dropdown = await screen.findByRole("listbox", {hidden: true}); @@ -1538,7 +1540,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("aria-haspopup", "listbox"); @@ -1578,7 +1580,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("aria-expanded", "false"); @@ -1594,7 +1596,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert diff --git a/packages/wonder-blocks-dropdown/src/components/action-menu.tsx b/packages/wonder-blocks-dropdown/src/components/action-menu.tsx index ffbe93099..cf5878e06 100644 --- a/packages/wonder-blocks-dropdown/src/components/action-menu.tsx +++ b/packages/wonder-blocks-dropdown/src/components/action-menu.tsx @@ -267,6 +267,7 @@ export default class ActionMenu extends React.Component { ref={this.handleOpenerRef} testId={opener ? undefined : testId} opened={this.state.opened} + role="button" > {opener ? opener diff --git a/packages/wonder-blocks-dropdown/src/components/dropdown-opener.tsx b/packages/wonder-blocks-dropdown/src/components/dropdown-opener.tsx index b85dcc3b6..cdc9ab017 100644 --- a/packages/wonder-blocks-dropdown/src/components/dropdown-opener.tsx +++ b/packages/wonder-blocks-dropdown/src/components/dropdown-opener.tsx @@ -43,15 +43,21 @@ type Props = Partial> & { * The unique identifier for the opener. */ id?: string; + /** + * The role of the opener. + */ + role?: "combobox" | "button"; }; type DefaultProps = { disabled: Props["disabled"]; + role: Props["role"]; }; class DropdownOpener extends React.Component { static defaultProps: DefaultProps = { disabled: false, + role: "combobox", }; getTestIdFromProps: (childrenProps?: any) => string = (childrenProps) => { @@ -70,6 +76,7 @@ class DropdownOpener extends React.Component { "aria-controls": ariaControls, "aria-haspopup": ariaHasPopUp, id, + role, } = this.props; const renderedChildren = this.props.children({ ...eventState, @@ -83,6 +90,7 @@ class DropdownOpener extends React.Component { ...clickableChildrenProps, disabled, "aria-controls": ariaControls, + role, id, "aria-expanded": opened ? "true" : "false", "aria-haspopup": ariaHasPopUp, diff --git a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx index 0e9dd262c..8e00e2e75 100644 --- a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx +++ b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx @@ -12,7 +12,7 @@ import caretDownIcon from "@phosphor-icons/core/bold/caret-down-bold.svg"; import {DROPDOWN_ITEM_HEIGHT} from "../util/constants"; import {OptionLabel} from "../util/types"; -const StyledButton = addStyle("button"); +const StyledButton = addStyle("span"); type SelectOpenerProps = AriaProps & { /** @@ -38,6 +38,8 @@ type SelectOpenerProps = AriaProps & { * of this component. A placeholder has more faded text colors and styles. */ isPlaceholder: boolean; + + placeholder?: string; /** * Whether to display the "light" version of this component instead, for * use when the item is used on a dark background. @@ -130,6 +132,7 @@ export default class SelectOpener extends React.Component< error, id, isPlaceholder, + placeholder, light, open, testId, @@ -164,9 +167,11 @@ export default class SelectOpener extends React.Component< aria-expanded={open ? "true" : "false"} aria-haspopup="listbox" data-testid={testId} + tabIndex={0} id={id} + role="combobox" + aria-label={placeholder} style={style} - type="button" onClick={!disabled ? this.handleClick : undefined} onKeyDown={!disabled ? this.handleKeyDown : undefined} onKeyUp={!disabled ? this.handleKeyUp : undefined} diff --git a/packages/wonder-blocks-dropdown/src/components/single-select.tsx b/packages/wonder-blocks-dropdown/src/components/single-select.tsx index 52cb8be21..f30cdfc66 100644 --- a/packages/wonder-blocks-dropdown/src/components/single-select.tsx +++ b/packages/wonder-blocks-dropdown/src/components/single-select.tsx @@ -440,6 +440,7 @@ export default class SingleSelect extends React.Component { ) : ( Date: Tue, 15 Oct 2024 10:45:30 -0400 Subject: [PATCH 2/5] Dropdown: use combobox role (instead of button) --- .changeset/tasty-rockets-mix.md | 5 ++ .../__tests__/multi-select.test.tsx | 66 +++++++++---------- .../__tests__/select-opener.test.tsx | 12 ++-- .../__tests__/single-select.test.tsx | 48 +++++++------- .../src/components/action-menu.tsx | 1 + .../src/components/dropdown-opener.tsx | 8 +++ .../src/components/select-opener.tsx | 9 ++- .../src/components/single-select.tsx | 1 + 8 files changed, 86 insertions(+), 64 deletions(-) create mode 100644 .changeset/tasty-rockets-mix.md diff --git a/.changeset/tasty-rockets-mix.md b/.changeset/tasty-rockets-mix.md new file mode 100644 index 000000000..d5c5d397b --- /dev/null +++ b/.changeset/tasty-rockets-mix.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-dropdown": patch +--- + +Update dropdown openers to use `role="combobox"` instead of `button`. diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx index 2989923bf..129728aa1 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx @@ -71,7 +71,7 @@ describe("MultiSelect", () => { const {userEvent} = doRender(uncontrolledMultiSelect); // Act - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect( @@ -103,7 +103,7 @@ describe("MultiSelect", () => { doRender(uncontrolledMultiSelect); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert // No items are selected, display placeholder because there is one @@ -127,7 +127,7 @@ describe("MultiSelect", () => { ); // Assert - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "item 1", ); }); @@ -149,7 +149,7 @@ describe("MultiSelect", () => { ); // Assert - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "1 student", ); }); @@ -171,7 +171,7 @@ describe("MultiSelect", () => { ); // Assert - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "Choose", ); }); @@ -194,7 +194,7 @@ describe("MultiSelect", () => { // Assert // More than one item is selected, display n itemTypes - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "2 students", ); }); @@ -217,7 +217,7 @@ describe("MultiSelect", () => { // Assert // All items are selected - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "All students", ); }); @@ -240,7 +240,7 @@ describe("MultiSelect", () => { ); // Assert - expect(await screen.findByRole("button")).toHaveTextContent( + expect(await screen.findByRole("combobox")).toHaveTextContent( "All students", ); }); @@ -259,7 +259,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("data-testid", "some-test-id"); @@ -524,7 +524,7 @@ describe("MultiSelect", () => { const {userEvent} = doRender(); - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Act @@ -597,7 +597,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Act const searchInput = await screen.findByPlaceholderText("Filter"); @@ -610,7 +610,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); @@ -627,7 +627,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); @@ -658,7 +658,7 @@ describe("MultiSelect", () => { // Act // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect( @@ -681,7 +681,7 @@ describe("MultiSelect", () => { , ); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); @@ -696,7 +696,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Act const searchInput = await screen.findByPlaceholderText("Filter"); @@ -723,7 +723,7 @@ describe("MultiSelect", () => { // Act // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect( @@ -747,7 +747,7 @@ describe("MultiSelect", () => { , ); - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); await userEvent.type(searchInput, "Item 2"); @@ -779,7 +779,7 @@ describe("MultiSelect", () => { , ); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); await userEvent.type(searchInput, "some text"); @@ -796,7 +796,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); await userEvent.type(searchInput, "Should be cleared"); @@ -814,7 +814,7 @@ describe("MultiSelect", () => { // Arrange const {userEvent} = doRender(filterableMultiSelect); // open the dropdown menu - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const searchInput = await screen.findByPlaceholderText("Filter"); @@ -1430,7 +1430,7 @@ describe("MultiSelect", () => { rerender(); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // open dropdown await userEvent.click(opener); @@ -1646,7 +1646,7 @@ describe("MultiSelect", () => { // Act // Press the button - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -1674,7 +1674,7 @@ describe("MultiSelect", () => { // Act // Press the button - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -1789,7 +1789,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute( @@ -1808,7 +1808,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("id", id); @@ -1824,7 +1824,7 @@ describe("MultiSelect", () => { // Act // Open the dropdown - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert @@ -1847,7 +1847,7 @@ describe("MultiSelect", () => { // Act // Open the dropdown - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert @@ -1869,7 +1869,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const dropdown = await screen.findByRole("listbox", {hidden: true}); @@ -1888,7 +1888,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const dropdown = await screen.findByRole("listbox", {hidden: true}); @@ -1965,7 +1965,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("aria-haspopup", "listbox"); @@ -2004,7 +2004,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("aria-expanded", "false"); @@ -2020,7 +2020,7 @@ describe("MultiSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx index 8c7e461f8..854d8af7c 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx @@ -26,7 +26,7 @@ describe("SelectOpener", () => { // Act // Press the button. - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect(onOpenMock).toHaveBeenCalledTimes(1); @@ -51,7 +51,7 @@ describe("SelectOpener", () => { // Act // Press the button. - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -87,7 +87,7 @@ describe("SelectOpener", () => { // Act // Press the button. - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -131,7 +131,7 @@ describe("SelectOpener", () => { // Act // Press the button. - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); // Assert expect(onOpenMock).toHaveBeenCalledTimes(0); @@ -156,7 +156,7 @@ describe("SelectOpener", () => { // Act // Press the button. - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -192,7 +192,7 @@ describe("SelectOpener", () => { // Act // Press the button. - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx index deb11b651..a1f53ae64 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx @@ -61,7 +61,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveTextContent("Default placeholder"); @@ -82,7 +82,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveTextContent(""); @@ -102,7 +102,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveTextContent("Toggle A"); @@ -130,7 +130,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveTextContent("Plain Toggle A"); @@ -220,13 +220,13 @@ describe("SingleSelect", () => { ); // Act - await userEvent.click(await screen.findByRole("textbox")); + await userEvent.click(await screen.findByRole("combobox")); // wait for the dropdown to open await screen.findByRole("listbox", {hidden: true}); // Assert - expect(await screen.findByRole("textbox")).toHaveFocus(); + expect(await screen.findByRole("combobox")).toHaveFocus(); }); }); @@ -412,7 +412,7 @@ describe("SingleSelect", () => { // Act await userEvent.click( - await screen.findByRole("button", {name: "Choose"}), + await screen.findByRole("combobox", {name: /choose/i}), ); // Assert @@ -427,7 +427,7 @@ describe("SingleSelect", () => { ); // open the menu from the outside await userEvent.click( - await screen.findByRole("button", {name: "Choose"}), + await screen.findByRole("combobox", {name: /choose/i}), ); // Act @@ -471,7 +471,9 @@ describe("SingleSelect", () => { const userEvent = doRender(); // Act - const opener = await screen.findByRole("button", {name: "Choose"}); + const opener = await screen.findByRole("combobox", { + name: /choose/i, + }); // open the menu from the outside await userEvent.click(opener); // click on the dropdown anchor to hide the menu @@ -581,7 +583,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // open dropdown await userEvent.click(opener); @@ -626,7 +628,7 @@ describe("SingleSelect", () => { const userEvent = doRender(); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // open dropdown await userEvent.click(opener); await userEvent.click(await screen.findByText("Toggle B")); @@ -781,7 +783,7 @@ describe("SingleSelect", () => { , ); // open the dropdown menu - await userEvent.click(await screen.findByRole("button")); + await userEvent.click(await screen.findByRole("combobox")); const searchInput = await screen.findByPlaceholderText("Filter"); await userEvent.click(searchInput); @@ -1199,7 +1201,7 @@ describe("SingleSelect", () => { // Act // Press the button - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -1230,7 +1232,7 @@ describe("SingleSelect", () => { // Act // Press the button - const button = await screen.findByRole("button"); + const button = await screen.findByRole("combobox"); // NOTE: we need to use fireEvent here because await userEvent doesn't // support keyUp/Down events and we use these handlers to override // the default behavior of the button. @@ -1352,7 +1354,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute( @@ -1371,7 +1373,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("id", id); @@ -1387,7 +1389,7 @@ describe("SingleSelect", () => { // Act // Open the dropdown - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert @@ -1414,7 +1416,7 @@ describe("SingleSelect", () => { // Act // Open the dropdown - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert @@ -1440,7 +1442,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const dropdown = await screen.findByRole("listbox", {hidden: true}); @@ -1459,7 +1461,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); const dropdown = await screen.findByRole("listbox", {hidden: true}); @@ -1538,7 +1540,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("aria-haspopup", "listbox"); @@ -1578,7 +1580,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); // Assert expect(opener).toHaveAttribute("aria-expanded", "false"); @@ -1594,7 +1596,7 @@ describe("SingleSelect", () => { ); // Act - const opener = await screen.findByRole("button"); + const opener = await screen.findByRole("combobox"); await userEvent.click(opener); // Assert diff --git a/packages/wonder-blocks-dropdown/src/components/action-menu.tsx b/packages/wonder-blocks-dropdown/src/components/action-menu.tsx index ffbe93099..cf5878e06 100644 --- a/packages/wonder-blocks-dropdown/src/components/action-menu.tsx +++ b/packages/wonder-blocks-dropdown/src/components/action-menu.tsx @@ -267,6 +267,7 @@ export default class ActionMenu extends React.Component { ref={this.handleOpenerRef} testId={opener ? undefined : testId} opened={this.state.opened} + role="button" > {opener ? opener diff --git a/packages/wonder-blocks-dropdown/src/components/dropdown-opener.tsx b/packages/wonder-blocks-dropdown/src/components/dropdown-opener.tsx index b85dcc3b6..cdc9ab017 100644 --- a/packages/wonder-blocks-dropdown/src/components/dropdown-opener.tsx +++ b/packages/wonder-blocks-dropdown/src/components/dropdown-opener.tsx @@ -43,15 +43,21 @@ type Props = Partial> & { * The unique identifier for the opener. */ id?: string; + /** + * The role of the opener. + */ + role?: "combobox" | "button"; }; type DefaultProps = { disabled: Props["disabled"]; + role: Props["role"]; }; class DropdownOpener extends React.Component { static defaultProps: DefaultProps = { disabled: false, + role: "combobox", }; getTestIdFromProps: (childrenProps?: any) => string = (childrenProps) => { @@ -70,6 +76,7 @@ class DropdownOpener extends React.Component { "aria-controls": ariaControls, "aria-haspopup": ariaHasPopUp, id, + role, } = this.props; const renderedChildren = this.props.children({ ...eventState, @@ -83,6 +90,7 @@ class DropdownOpener extends React.Component { ...clickableChildrenProps, disabled, "aria-controls": ariaControls, + role, id, "aria-expanded": opened ? "true" : "false", "aria-haspopup": ariaHasPopUp, diff --git a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx index 0e9dd262c..8e00e2e75 100644 --- a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx +++ b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx @@ -12,7 +12,7 @@ import caretDownIcon from "@phosphor-icons/core/bold/caret-down-bold.svg"; import {DROPDOWN_ITEM_HEIGHT} from "../util/constants"; import {OptionLabel} from "../util/types"; -const StyledButton = addStyle("button"); +const StyledButton = addStyle("span"); type SelectOpenerProps = AriaProps & { /** @@ -38,6 +38,8 @@ type SelectOpenerProps = AriaProps & { * of this component. A placeholder has more faded text colors and styles. */ isPlaceholder: boolean; + + placeholder?: string; /** * Whether to display the "light" version of this component instead, for * use when the item is used on a dark background. @@ -130,6 +132,7 @@ export default class SelectOpener extends React.Component< error, id, isPlaceholder, + placeholder, light, open, testId, @@ -164,9 +167,11 @@ export default class SelectOpener extends React.Component< aria-expanded={open ? "true" : "false"} aria-haspopup="listbox" data-testid={testId} + tabIndex={0} id={id} + role="combobox" + aria-label={placeholder} style={style} - type="button" onClick={!disabled ? this.handleClick : undefined} onKeyDown={!disabled ? this.handleKeyDown : undefined} onKeyUp={!disabled ? this.handleKeyUp : undefined} diff --git a/packages/wonder-blocks-dropdown/src/components/single-select.tsx b/packages/wonder-blocks-dropdown/src/components/single-select.tsx index 52cb8be21..f30cdfc66 100644 --- a/packages/wonder-blocks-dropdown/src/components/single-select.tsx +++ b/packages/wonder-blocks-dropdown/src/components/single-select.tsx @@ -440,6 +440,7 @@ export default class SingleSelect extends React.Component { ) : ( Date: Fri, 18 Oct 2024 11:58:29 -0400 Subject: [PATCH 3/5] Switch to div, remove button wording --- .../wonder-blocks-dropdown/src/components/select-opener.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx index 8e00e2e75..3a7212a99 100644 --- a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx +++ b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx @@ -12,7 +12,7 @@ import caretDownIcon from "@phosphor-icons/core/bold/caret-down-bold.svg"; import {DROPDOWN_ITEM_HEIGHT} from "../util/constants"; import {OptionLabel} from "../util/types"; -const StyledButton = addStyle("span"); +const StyledDiv = addStyle("div"); type SelectOpenerProps = AriaProps & { /** @@ -161,7 +161,7 @@ export default class SelectOpener extends React.Component< ]; return ( - + ); } } From a0cbf60157836245295205f4940fc58e2e796f2a Mon Sep 17 00:00:00 2001 From: Juan Andrade Date: Mon, 4 Nov 2024 10:42:01 -0500 Subject: [PATCH 4/5] Revert back to button role=combobox --- .../wonder-blocks-dropdown/src/components/multi-select.tsx | 1 + .../wonder-blocks-dropdown/src/components/select-opener.tsx | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/wonder-blocks-dropdown/src/components/multi-select.tsx b/packages/wonder-blocks-dropdown/src/components/multi-select.tsx index 2545dc6b3..ca1239c21 100644 --- a/packages/wonder-blocks-dropdown/src/components/multi-select.tsx +++ b/packages/wonder-blocks-dropdown/src/components/multi-select.tsx @@ -545,6 +545,7 @@ export default class MultiSelect extends React.Component { disabled={isDisabled} id={uniqueOpenerId} aria-controls={dropdownId} + placeholder={noneSelected} isPlaceholder={menuText === noneSelected} light={light} onOpenChanged={this.handleOpenChanged} diff --git a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx index 3a7212a99..25a0ca7d5 100644 --- a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx +++ b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx @@ -12,7 +12,7 @@ import caretDownIcon from "@phosphor-icons/core/bold/caret-down-bold.svg"; import {DROPDOWN_ITEM_HEIGHT} from "../util/constants"; import {OptionLabel} from "../util/types"; -const StyledDiv = addStyle("div"); +const StyledButton = addStyle("button"); type SelectOpenerProps = AriaProps & { /** @@ -161,7 +161,7 @@ export default class SelectOpener extends React.Component< ]; return ( - + ); } } From d067480b4bc2e1b61e89489be6e7b8f193051346 Mon Sep 17 00:00:00 2001 From: Juan Andrade Date: Mon, 25 Nov 2024 11:37:04 -0500 Subject: [PATCH 5/5] Remove redundant tabindex --- packages/wonder-blocks-dropdown/src/components/select-opener.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx index 25a0ca7d5..774069f48 100644 --- a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx +++ b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx @@ -167,7 +167,6 @@ export default class SelectOpener extends React.Component< aria-expanded={open ? "true" : "false"} aria-haspopup="listbox" data-testid={testId} - tabIndex={0} id={id} role="combobox" aria-label={placeholder}