From f7ff9a77234b2c735f62015a6cfa11a02975b6e9 Mon Sep 17 00:00:00 2001 From: Bea Esguerra Date: Wed, 12 Jun 2024 13:08:11 -0600 Subject: [PATCH] Disable keyboard interactions for DropdownCore if component is disabled (#2250) * Remove storybook controls for SingleSelect disabled story. Because the storybook args included opened:false, the select was always closed, which was hiding a bug where the disabled select could still be opened using keyboard arrows * Add disabled prop to DropdownCore so that it's interactions can be disabled if it is true. This is needed now that the dropdown opener could potentially not have the disabled attribute set (since we use aria-disabled now on the selectopener so that it can be focusable * Add tests to make sure event handlers aren't called when a component is disabled * Add changeset * Prevent single/multiselect from being in an open state if the disabled prop is true --- .changeset/curvy-frogs-listen.md | 6 + .../single-select.stories.tsx | 3 +- .../__tests__/dropdown-core.test.tsx | 76 +++++++++++++ .../__tests__/multi-select.test.tsx | 16 +++ .../__tests__/select-opener.test.tsx | 107 +++++++++++++++++- .../__tests__/single-select.test.tsx | 21 ++++ .../src/components/dropdown-core.tsx | 10 +- .../src/components/multi-select.tsx | 9 +- .../src/components/single-select.tsx | 9 +- 9 files changed, 249 insertions(+), 8 deletions(-) create mode 100644 .changeset/curvy-frogs-listen.md diff --git a/.changeset/curvy-frogs-listen.md b/.changeset/curvy-frogs-listen.md new file mode 100644 index 000000000..a9c47f90d --- /dev/null +++ b/.changeset/curvy-frogs-listen.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/wonder-blocks-dropdown": patch +--- + +SingleSelect and MultiSelect: Disable keyboard interactions to open the select if the `disabled` prop is set to `true`. Prevent select from being in an open state if +`disabled` prop is `true`. diff --git a/__docs__/wonder-blocks-dropdown/single-select.stories.tsx b/__docs__/wonder-blocks-dropdown/single-select.stories.tsx index 214a9d078..08b2d2e9b 100644 --- a/__docs__/wonder-blocks-dropdown/single-select.stories.tsx +++ b/__docs__/wonder-blocks-dropdown/single-select.stories.tsx @@ -343,9 +343,8 @@ export const LongOptionLabels: StoryComponentType = { * remains focusable while communicating to screen readers that it is disabled. */ export const Disabled: StoryComponentType = { - render: (args) => ( + render: () => ( {}} selectedValue="" diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/dropdown-core.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/dropdown-core.test.tsx index 13597bd51..3f247da6f 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/dropdown-core.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/dropdown-core.test.tsx @@ -772,4 +772,80 @@ describe("DropdownCore", () => { expect(container).toHaveTextContent("3 items"); }); }); + + describe("onOpenChanged", () => { + it("Should be triggered when the down key is pressed and the menu is closed", async () => { + // Arrange + const onOpenMock = jest.fn(); + + render( + } + onOpenChanged={onOpenMock} + />, + ); + // Act + // Press the button + const button = await screen.findByRole("button"); + // 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. + // eslint-disable-next-line testing-library/prefer-user-event + fireEvent.keyDown(button, { + keyCode: 40, + }); + // eslint-disable-next-line testing-library/prefer-user-event + fireEvent.keyUp(button, { + keyCode: 40, + }); + + // Assert + expect(onOpenMock).toHaveBeenCalledTimes(1); + expect(onOpenMock).toHaveBeenCalledWith(true); + }); + + it("Should not be triggered when the dropdown is disabled and the down key is pressed and the menu is closed", async () => { + // Arrange + const onOpenMock = jest.fn(); + + render( + } + onOpenChanged={onOpenMock} + disabled={true} + />, + ); + // Act + // Press the button + const button = await screen.findByRole("button"); + // 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. + // eslint-disable-next-line testing-library/prefer-user-event + fireEvent.keyDown(button, { + keyCode: 40, + }); + // eslint-disable-next-line testing-library/prefer-user-event + fireEvent.keyUp(button, { + keyCode: 40, + }); + + // Assert + expect(onOpenMock).toHaveBeenCalledTimes(0); + }); + }); }); 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 452b0b36b..3238d6278 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 @@ -1623,6 +1623,22 @@ describe("MultiSelect", () => { // Assert expect(multiSelect).not.toHaveAttribute("disabled"); }); + + it("should not be opened if it is disabled and `open` prop is set to true", () => { + // Arrange + + // Act + doRender( + + + + + , + ); + + // Assert + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + }); }); describe("a11y > Focusable", () => { 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 f3cad178d..8c7e461f8 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 @@ -5,7 +5,7 @@ import {userEvent} from "@testing-library/user-event"; import SelectOpener from "../select-opener"; describe("SelectOpener", () => { - describe("onClick", () => { + describe("onOpenChanged", () => { const children = "text"; it("should trigger using the mouse", async () => { // Arrange @@ -111,5 +111,110 @@ describe("SelectOpener", () => { // Assert expect(onOpenMock).toHaveBeenCalledTimes(1); }); + + it("should not trigger using the mouse if it is disabled", async () => { + // Arrange + const onOpenMock = jest.fn(); + + render( + + {children} + , + ); + + // Act + // Press the button. + await userEvent.click(await screen.findByRole("button")); + + // Assert + expect(onOpenMock).toHaveBeenCalledTimes(0); + }); + + it("should not trigger by pressing {Space} if it is disabled", async () => { + // Arrange + const onOpenMock = jest.fn(); + + render( + + {children} + , + ); + + // Act + // Press the button. + const button = await screen.findByRole("button"); + // 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. + // eslint-disable-next-line testing-library/prefer-user-event + fireEvent.keyDown(button, { + key: " ", + }); + // eslint-disable-next-line testing-library/prefer-user-event + fireEvent.keyUp(button, { + key: " ", + }); + + // Assert + expect(onOpenMock).toHaveBeenCalledTimes(0); + }); + + it("should not trigger by pressing {Enter} if it is disabled", async () => { + // Arrange + const onOpenMock = jest.fn(); + + render( + + {children} + , + ); + + // Act + // Press the button. + const button = await screen.findByRole("button"); + // 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. + // eslint-disable-next-line testing-library/prefer-user-event + fireEvent.keyDown(button, { + key: "Enter", + }); + // NOTE: We need to trigger multiple events to simulate the browser + // behavior of pressing Enter on a button. By default, browsers will + // trigger a click event on keyDown, but we need to trigger it on + // keyUp. + // eslint-disable-next-line testing-library/prefer-user-event + fireEvent.keyDown(button, { + key: "Enter", + }); + // eslint-disable-next-line testing-library/prefer-user-event + fireEvent.keyUp(button, { + key: "Enter", + }); + + // Assert + expect(onOpenMock).toHaveBeenCalledTimes(0); + }); }); }); 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 cc2898073..552f91e77 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 @@ -1166,6 +1166,27 @@ describe("SingleSelect", () => { // Assert expect(singleSelect).not.toHaveAttribute("disabled"); }); + + it("should not be opened if it is disabled and `open` prop is set to true", () => { + // Arrange + + // Act + doRender( + + + + + , + ); + + // Assert + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + }); }); describe("a11y > Focusable", () => { diff --git a/packages/wonder-blocks-dropdown/src/components/dropdown-core.tsx b/packages/wonder-blocks-dropdown/src/components/dropdown-core.tsx index d0a45dda7..1e7058e07 100644 --- a/packages/wonder-blocks-dropdown/src/components/dropdown-core.tsx +++ b/packages/wonder-blocks-dropdown/src/components/dropdown-core.tsx @@ -169,6 +169,10 @@ type ExportProps = Readonly<{ * top. The items will be filtered by the input. */ isFilterable?: boolean; + /** + * Whether the dropdown and it's interactions should be disabled. + */ + disabled?: boolean; // Optional props with defaults /** @@ -1040,12 +1044,12 @@ class DropdownCore extends React.Component { } render(): React.ReactNode { - const {open, opener, style, className} = this.props; + const {open, opener, style, className, disabled} = this.props; return ( diff --git a/packages/wonder-blocks-dropdown/src/components/multi-select.tsx b/packages/wonder-blocks-dropdown/src/components/multi-select.tsx index 09e98bbaa..28055ce52 100644 --- a/packages/wonder-blocks-dropdown/src/components/multi-select.tsx +++ b/packages/wonder-blocks-dropdown/src/components/multi-select.tsx @@ -241,7 +241,12 @@ export default class MultiSelect extends React.Component { state: State, ): Partial | null { return { - open: typeof props.opened === "boolean" ? props.opened : state.open, + // open should always be false if select is disabled + open: props.disabled + ? false + : typeof props.opened === "boolean" + ? props.opened + : state.open, }; } @@ -549,6 +554,7 @@ export default class MultiSelect extends React.Component { isFilterable, "aria-invalid": ariaInvalid, "aria-required": ariaRequired, + disabled, } = this.props; const {open, searchText} = this.state; const {clearSearch, filter, noResults, someSelected} = @@ -599,6 +605,7 @@ export default class MultiSelect extends React.Component { }} aria-invalid={ariaInvalid} aria-required={ariaRequired} + disabled={disabled} /> ); } diff --git a/packages/wonder-blocks-dropdown/src/components/single-select.tsx b/packages/wonder-blocks-dropdown/src/components/single-select.tsx index a599ab2a4..4c17c0a71 100644 --- a/packages/wonder-blocks-dropdown/src/components/single-select.tsx +++ b/packages/wonder-blocks-dropdown/src/components/single-select.tsx @@ -252,7 +252,12 @@ export default class SingleSelect extends React.Component { state: State, ): Partial | null { return { - open: typeof props.opened === "boolean" ? props.opened : state.open, + // open should always be false if select is disabled + open: props.disabled + ? false + : typeof props.opened === "boolean" + ? props.opened + : state.open, }; } @@ -448,6 +453,7 @@ export default class SingleSelect extends React.Component { style, "aria-invalid": ariaInvalid, "aria-required": ariaRequired, + disabled, } = this.props; const {searchText} = this.state; const allChildren = React.Children.toArray(children).filter(Boolean); @@ -484,6 +490,7 @@ export default class SingleSelect extends React.Component { labels={labels} aria-invalid={ariaInvalid} aria-required={ariaRequired} + disabled={disabled} /> ); }