Disable keyboard interactions for DropdownCore if component is disabl…

…ed (#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
Khan · Jun 12, 2024 · f7ff9a7 · f7ff9a7
1 parent 308fe79
commit f7ff9a7
Show file tree

Hide file tree

Showing 9 changed files with 249 additions and 8 deletions.
diff --git a/.changeset/curvy-frogs-listen.md 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
@@ -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: () => (
         <SingleSelect
-            {...args}
             placeholder="Choose a fruit"
             onChange={() => {}}
             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
@@ -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(
+                <DropdownCore
+                    initialFocusedIndex={undefined}
+                    onSearchTextChanged={jest.fn()}
+                    // mock the items (3 options)
+                    items={items}
+                    role="listbox"
+                    open={false}
+                    // mock the opener elements
+                    opener={<button />}
+                    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(
+                <DropdownCore
+                    initialFocusedIndex={undefined}
+                    onSearchTextChanged={jest.fn()}
+                    // mock the items (3 options)
+                    items={items}
+                    role="listbox"
+                    open={false}
+                    // mock the opener elements
+                    opener={<button />}
+                    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
@@ -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(
+                <MultiSelect onChange={jest.fn()} disabled={true} opened={true}>
+                    <OptionItem label="item 1" value="1" />
+                    <OptionItem label="item 2" value="2" />
+                    <OptionItem label="item 3" value="3" />
+                </MultiSelect>,
+            );
+
+            // 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
@@ -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(
+                <SelectOpener
+                    onOpenChanged={onOpenMock}
+                    disabled={true}
+                    error={false}
+                    isPlaceholder={false}
+                    light={false}
+                    open={false}
+                >
+                    {children}
+                </SelectOpener>,
+            );
+
+            // 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(
+                <SelectOpener
+                    onOpenChanged={onOpenMock}
+                    disabled={true}
+                    error={false}
+                    isPlaceholder={false}
+                    light={false}
+                    open={false}
+                >
+                    {children}
+                </SelectOpener>,
+            );
+
+            // 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(
+                <SelectOpener
+                    onOpenChanged={onOpenMock}
+                    disabled={true}
+                    error={false}
+                    isPlaceholder={false}
+                    light={false}
+                    open={false}
+                >
+                    {children}
+                </SelectOpener>,
+            );
+
+            // 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
@@ -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(
+                <SingleSelect
+                    placeholder="Default placeholder"
+                    onChange={jest.fn()}
+                    disabled={true}
+                    opened={true}
+                >
+                    <OptionItem label="item 1" value="1" />
+                    <OptionItem label="item 2" value="2" />
+                    <OptionItem label="item 3" value="3" />
+                </SingleSelect>,
+            );
+
+            // 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
@@ -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<Props, State> {
     }
 
     render(): React.ReactNode {
-        const {open, opener, style, className} = this.props;
+        const {open, opener, style, className, disabled} = this.props;
 
         return (
             <View
-                onKeyDown={this.handleKeyDown}
-                onKeyUp={this.handleKeyUp}
+                onKeyDown={!disabled ? this.handleKeyDown : undefined}
+                onKeyUp={!disabled ? this.handleKeyUp : undefined}
                 style={[styles.menuWrapper, style]}
                 className={className}
             >

diff --git 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<Props, State> {
         state: State,
     ): Partial<State> | 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<Props, State> {
             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<Props, State> {
                 }}
                 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
@@ -252,7 +252,12 @@ export default class SingleSelect extends React.Component<Props, State> {
         state: State,
     ): Partial<State> | 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<Props, State> {
             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<Props, State> {
                 labels={labels}
                 aria-invalid={ariaInvalid}
                 aria-required={ariaRequired}
+                disabled={disabled}
             />
         );
     }