Merge pull request #280 from element-hq/controlled-dropdown

Allow dropdown state to be controlled
element-hq · Nov 21, 2024 · 228899c · 228899c
2 parents 2145bfc + b504449
commit 228899c
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 81 deletions.
diff --git a/src/components/Dropdown/Dropdown.test.tsx b/src/components/Dropdown/Dropdown.test.tsx
@@ -17,13 +17,34 @@ limitations under the License.
 import { describe, expect, it } from "vitest";
 import { composeStories } from "@storybook/react";
 import * as stories from "./Dropdown.stories";
-import { act, render, waitFor } from "@testing-library/react";
-import React from "react";
-import { userEvent } from "@storybook/test";
+import { render, screen } from "@testing-library/react";
+import React, { FC, useMemo, useState } from "react";
+import { Dropdown } from "./Dropdown";
+import userEvent from "@testing-library/user-event";
 
 const { Default, WithHelpLabel, WithError, WithDefaultValue } =
   composeStories(stories);
 
+const ControlledDropdown: FC = () => {
+  const [value, setValue] = useState("1");
+  const values = useMemo<[string, string][]>(
+    () => [
+      ["1", "Option 1"],
+      ["2", "Option 2"],
+    ],
+    [],
+  );
+  return (
+    <Dropdown
+      value={value}
+      onValueChange={setValue}
+      values={values}
+      placeholder=""
+      label="Label"
+    />
+  );
+};
+
 describe("Dropdown", () => {
   it("renders a Default dropdown", () => {
     const { container } = render(<Default />);
@@ -42,61 +63,65 @@ describe("Dropdown", () => {
     expect(container).toMatchSnapshot();
   });
   it("can be opened", async () => {
+    const user = userEvent.setup();
     const { getByRole, container } = render(<Default />);
-    await act(async () => {
-      await userEvent.click(getByRole("combobox"));
-    });
+    await user.click(getByRole("combobox"));
     expect(container).toMatchSnapshot();
   });
   it("can select a value", async () => {
+    const user = userEvent.setup();
     const { getByRole, container } = render(<Default />);
-    await act(async () => {
-      await userEvent.click(getByRole("combobox"));
-    });
+    await user.click(getByRole("combobox"));
 
-    await waitFor(() =>
-      expect(getByRole("option", { name: "Option 2" })).toBeVisible(),
-    );
+    expect(getByRole("option", { name: "Option 2" })).toBeVisible();
 
-    await act(async () => {
-      await userEvent.click(getByRole("option", { name: "Option 2" }));
-    });
+    await user.click(getByRole("option", { name: "Option 2" }));
 
     expect(getByRole("combobox")).toHaveTextContent("Option 2");
 
-    await act(async () => {
-      await userEvent.click(getByRole("combobox"));
-    });
+    await user.click(getByRole("combobox"));
 
-    await waitFor(() =>
-      expect(getByRole("option", { name: "Option 2" })).toHaveAttribute(
-        "aria-selected",
-        "true",
-      ),
+    expect(getByRole("option", { name: "Option 2" })).toHaveAttribute(
+      "aria-selected",
+      "true",
     );
 
     // Option 2 should be selected
     expect(container).toMatchSnapshot();
   });
   it("can use keyboard shortcuts", async () => {
+    const user = userEvent.setup();
     const { getByRole } = render(<Default />);
 
-    await act(async () => userEvent.type(getByRole("combobox"), "{arrowdown}"));
-    await waitFor(() =>
-      expect(getByRole("combobox")).toHaveAttribute("aria-expanded", "true"),
-    );
+    // arrowdown seems to already select Option 1... in real browsers this
+    // doesn't happen. Maybe it's a user-event thing? arrowup just opens the
+    // dropdown as we would expect.
+    await user.type(getByRole("combobox"), "{arrowup}");
+    expect(getByRole("combobox")).toHaveAttribute("aria-expanded", "true");
 
-    await act(async () => userEvent.keyboard("{arrowdown}"));
+    await user.keyboard("{arrowdown}");
     expect(getByRole("option", { name: "Option 1" })).toHaveFocus();
 
-    await act(async () => userEvent.keyboard("{End}"));
+    await user.keyboard("{End}");
     expect(getByRole("option", { name: "Option 3" })).toHaveFocus();
 
-    await act(async () => userEvent.keyboard("{Enter}"));
+    await user.keyboard("{Enter}");
 
-    await waitFor(() => {
-      expect(getByRole("combobox")).toHaveTextContent("Option 3");
-      expect(getByRole("combobox")).toHaveAttribute("aria-expanded", "false");
-    });
+    expect(getByRole("combobox")).toHaveTextContent("Option 3");
+    expect(getByRole("combobox")).toHaveAttribute("aria-expanded", "false");
+  });
+  it("supports controlled operation", async () => {
+    const user = userEvent.setup();
+    render(<ControlledDropdown />);
+
+    expect(screen.getByRole("option", { name: "Option 1" })).toHaveAttribute(
+      "aria-selected",
+      "true",
+    );
+    await user.click(screen.getByRole("option", { name: "Option 2" }));
+    expect(screen.getByRole("option", { name: "Option 2" })).toHaveAttribute(
+      "aria-selected",
+      "true",
+    );
   });
 });
diff --git a/src/components/Dropdown/Dropdown.tsx b/src/components/Dropdown/Dropdown.tsx
@@ -30,6 +30,7 @@ import React, {
   useRef,
   useState,
   KeyboardEvent,
+  useMemo,
 } from "react";
 
 import classNames from "classnames";
@@ -43,7 +44,11 @@ type DropdownProps = {
    */
   className?: string;
   /**
-   * The default value of the dropdown.
+   * The controlled value of the dropdown.
+   */
+  value?: string;
+  /**
+   * The default value of the dropdown, used when uncontrolled.
    */
   defaultValue?: string;
   /**
@@ -86,34 +91,46 @@ export const Dropdown = forwardRef<HTMLButtonElement, DropdownProps>(
       helpLabel,
       onValueChange,
       error,
+      value: controlledValue,
       defaultValue,
       values,
       ...props
     },
     ref,
   ) {
-    const [state, setState] = useInitialState(
-      values,
-      placeholder,
-      defaultValue,
+    const [uncontrolledValue, setUncontrolledValue] = useState(defaultValue);
+    const value = controlledValue ?? uncontrolledValue;
+    const text = useMemo(
+      () =>
+        value === undefined
+          ? placeholder
+          : (values.find(([v]) => v === value)?.[1] ?? placeholder),
+      [value, values, placeholder],
+    );
+
+    const setValue = useCallback(
+      (value: string) => {
+        setUncontrolledValue(value);
+        onValueChange?.(value);
+      },
+      [setUncontrolledValue, onValueChange],
     );
+
     const [open, setOpen, dropdownRef] = useOpen();
     const { listRef, onComboboxKeyDown, onOptionKeyDown } = useKeyboardShortcut(
       open,
       setOpen,
-      setState,
+      setValue,
     );
 
     const buttonRef = useRef<HTMLButtonElement | null>(null);
     useEffect(() => {
       // Focus the button when the value is set
       // Test if the value is undefined to avoid focusing on the first render
-      if (state.value !== undefined) {
-        buttonRef.current?.focus();
-      }
-    }, [state]);
+      if (value !== undefined) buttonRef.current?.focus();
+    }, [value]);
 
-    const hasPlaceholder = state.text === placeholder;
+    const hasPlaceholder = text === placeholder;
     const buttonClasses = classNames({
       [styles.placeholder]: hasPlaceholder,
     });
@@ -158,7 +175,7 @@ export const Dropdown = forwardRef<HTMLButtonElement, DropdownProps>(
           onKeyDown={onComboboxKeyDown}
           {...props}
         >
-          {state.text}
+          {text}
           <ChevronDown width="24" height="24" />
         </button>
         <div className={borderClasses} />
@@ -169,17 +186,16 @@ export const Dropdown = forwardRef<HTMLButtonElement, DropdownProps>(
             role="listbox"
             className={styles.content}
           >
-            {values.map(([value, text]) => (
+            {values.map(([v, text]) => (
               <DropdownItem
-                key={value}
+                key={v}
                 isDisplayed={open}
-                isSelected={state.value === value}
+                isSelected={value === v}
                 onClick={() => {
                   setOpen(false);
-                  setState({ value, text });
-                  onValueChange?.(value);
+                  setValue(v);
                 }}
-                onKeyDown={(e) => onOptionKeyDown(e, value, text)}
+                onKeyDown={(e) => onOptionKeyDown(e, v)}
               >
                 {text}
               </DropdownItem>
@@ -272,31 +288,6 @@ function useOpen(): [
   return [open, setOpen, ref];
 }
 
-/**
- * A hook to manage the initial state of the dropdown.
- * @param values - The values of the dropdown.
- * @param placeholder - The placeholder text.
- * @param defaultValue - The default value of the dropdown.
- */
-function useInitialState(
-  values: [string, string][],
-  placeholder: string,
-  defaultValue?: string,
-) {
-  return useState(() => {
-    const defaultTuple = {
-      value: undefined,
-      text: placeholder,
-    };
-    if (!defaultValue) return defaultTuple;
-
-    const foundTuple = values.find(([value]) => value === defaultValue);
-    return foundTuple
-      ? { value: foundTuple[0], text: foundTuple[1] }
-      : defaultTuple;
-  });
-}
-
 /**
  * A hook to manage the keyboard shortcuts of the dropdown.
  * @param open - the dropdown open state.
@@ -306,7 +297,7 @@ function useInitialState(
 function useKeyboardShortcut(
   open: boolean,
   setOpen: Dispatch<SetStateAction<boolean>>,
-  setValue: ({ text, value }: { text: string; value: string }) => void,
+  setValue: (value: string) => void,
 ) {
   const listRef = useRef<HTMLUListElement>(null);
   const onComboboxKeyDown = useCallback(
@@ -348,15 +339,15 @@ function useKeyboardShortcut(
   );
 
   const onOptionKeyDown = useCallback(
-    (evt: KeyboardEvent, value: string, text: string) => {
+    (evt: KeyboardEvent, value: string) => {
       const { key, altKey } = evt;
       evt.stopPropagation();
       evt.preventDefault();
 
       switch (key) {
         case "Enter":
         case " ": {
-          setValue({ text, value });
+          setValue(value);
           setOpen(false);
           break;
         }
@@ -373,7 +364,7 @@ function useKeyboardShortcut(
         }
         case "ArrowUp": {
           if (altKey) {
-            setValue({ text, value });
+            setValue(value);
             setOpen(false);
           } else {
             const currentFocus = document.activeElement;