diff --git a/src/components/Dropdown/Dropdown.test.tsx b/src/components/Dropdown/Dropdown.test.tsx index 6005a6a0..b5995aa9 100644 --- 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 ( + + ); +}; + describe("Dropdown", () => { it("renders a Default dropdown", () => { const { container } = render(); @@ -42,61 +63,65 @@ describe("Dropdown", () => { expect(container).toMatchSnapshot(); }); it("can be opened", async () => { + const user = userEvent.setup(); const { getByRole, container } = render(); - 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(); - 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(); - 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(); + + 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 index 73679f57..efcdce52 100644 --- 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( 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(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( onKeyDown={onComboboxKeyDown} {...props} > - {state.text} + {text}
@@ -169,17 +186,16 @@ export const Dropdown = forwardRef( role="listbox" className={styles.content} > - {values.map(([value, text]) => ( + {values.map(([v, text]) => ( { setOpen(false); - setState({ value, text }); - onValueChange?.(value); + setValue(v); }} - onKeyDown={(e) => onOptionKeyDown(e, value, text)} + onKeyDown={(e) => onOptionKeyDown(e, v)} > {text} @@ -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>, - setValue: ({ text, value }: { text: string; value: string }) => void, + setValue: (value: string) => void, ) { const listRef = useRef(null); const onComboboxKeyDown = useCallback( @@ -348,7 +339,7 @@ function useKeyboardShortcut( ); const onOptionKeyDown = useCallback( - (evt: KeyboardEvent, value: string, text: string) => { + (evt: KeyboardEvent, value: string) => { const { key, altKey } = evt; evt.stopPropagation(); evt.preventDefault(); @@ -356,7 +347,7 @@ function useKeyboardShortcut( 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;