diff --git a/.changeset/fix-combobox-query-clearing.md b/.changeset/fix-combobox-query-clearing.md new file mode 100644 index 0000000000..bb74fcf529 --- /dev/null +++ b/.changeset/fix-combobox-query-clearing.md @@ -0,0 +1,12 @@ +--- +"@cloudoperators/juno-ui-components": patch +--- + +fix(ui): improve ComboBox search query state management + +- Clear search query after option selection to prevent stale search terms +- Clear query and close dropdown when controlled value prop becomes empty +- Add comprehensive tests for controlled and uncontrolled scenarios +- Use ref instead of getElementById for blur to work in shadow DOM + +**Breaking change note:** When both `value=""` and `defaultValue` are provided, the ComboBox now shows empty instead of falling back to `defaultValue`. This is the correct behavior since `defaultValue` is documented as uncontrolled-mode only, but consumers relying on the previous fallback behavior should update their code to not pass `value=""` if they want `defaultValue` to apply. diff --git a/packages/ui-components/src/components/ComboBox/ComboBox.component.tsx b/packages/ui-components/src/components/ComboBox/ComboBox.component.tsx index a53f14582b..f3f79a6043 100644 --- a/packages/ui-components/src/components/ComboBox/ComboBox.component.tsx +++ b/packages/ui-components/src/components/ComboBox/ComboBox.component.tsx @@ -17,6 +17,7 @@ import React, { HTMLAttributes, FocusEventHandler, ChangeEventHandler, + useRef, } from "react" import { Combobox, ComboboxInput, ComboboxOptions, ComboboxButton } from "@headlessui/react" import { @@ -213,7 +214,7 @@ export const ComboBox = ({ successtext = "", truncateOptions = false, valid = false, - value = "", + value, valueLabel, width = "full", wrapperClassName = "", @@ -232,13 +233,23 @@ export const ComboBox = ({ new Map() ) const [query, setQuery] = useState("") - const [selectedValue, setSelectedValue] = useState(value) + const [selectedValue, setSelectedValue] = useState(value ?? "") const [isLoading, setIsLoading] = useState(false) const [hasError, setHasError] = useState(false) const [hasFocus, setFocus] = useState(false) const [isInvalid, setIsInvalid] = useState(false) const [isValid, setIsValid] = useState(false) + // Determine if component is in controlled mode (value prop was explicitly supplied, even if empty) + // Computed inline - always accurate, zero cost, no ratchet behavior + const isControlled = value !== undefined + + // Use a ref to track the previous value to detect when it changes from truthy to falsy + const previousValue = useRef(value ?? "") + + // Ref to the input element for programmatic blur (works in shadow DOM unlike getElementById) + const inputRef = useRef(null) + // Floating UI setup const { x, y, strategy, refs, context } = useFloating({ open: isOpen, @@ -296,9 +307,24 @@ export const ComboBox = ({ ) useEffect(() => { - setSelectedValue(value) + setSelectedValue(value ?? "") + // Clear query and close dropdown when value is explicitly cleared to "" + // Use value === "" instead of !value to distinguish intentional reset from prop removal (undefined) + if (value === "" && previousValue.current) { + clearQueryWithEvent() + // Blur the input to close Headless UI's Combobox properly + // This ensures aria-expanded syncs correctly with the actual open state + // Use ref instead of getElementById to work within shadow DOM + inputRef.current?.blur() + } + previousValue.current = value ?? "" }, [value]) + // Determine the effective value to pass to Headless UI + // In controlled mode (value prop was supplied), honor it even if empty string + // In uncontrolled mode, fall back to defaultValue when selectedValue is empty + const effectiveValue = isControlled ? selectedValue : selectedValue || defaultValue + useEffect(() => { setHasError(error) }, [error]) @@ -319,6 +345,9 @@ export const ComboBox = ({ const stringValue = value || "" setSelectedValue(stringValue) + // Clear the search query after selection and notify parent + clearQueryWithEvent() + if (stringValue) { setIsOpen(false) } @@ -331,6 +360,23 @@ export const ComboBox = ({ onInputChange && onInputChange(event) } + // Helper to clear query and notify parent via onInputChange + // This keeps internal and external state in sync for consumers that maintain their own search state + const clearQueryWithEvent = () => { + setQuery("") + if (inputRef.current && onInputChange) { + // Set the input's value directly + inputRef.current.value = "" + // Create a real Event object which has all standard methods (preventDefault, stopPropagation, etc.) + // Override read-only properties to match React's synthetic event structure + const event = new Event("input", { bubbles: true, cancelable: true }) + Object.defineProperty(event, "target", { value: inputRef.current }) + Object.defineProperty(event, "currentTarget", { value: inputRef.current }) + Object.defineProperty(event, "nativeEvent", { value: event }) + onInputChange(event as unknown as ChangeEvent) + } + } + const handleFocus = (event: FocusEvent) => { setFocus(true) @@ -406,12 +452,12 @@ export const ComboBox = ({ disabled={disabled || isLoading || hasError} name={name} onChange={handleChange} - value={selectedValue || defaultValue} + value={effectiveValue} as="div" {...props} > {({ open }) => { - // Update our open state when Headless UI updates it + // Synchronize our open state with Headless UI's open state useEffect(() => { if (open !== isOpen) { setIsOpen(open) @@ -451,6 +497,7 @@ export const ComboBox = ({ )} + ref={inputRef} autoComplete="off" aria-label={ariaLabel || label} aria-describedby={helptext ? helptextId : ""} diff --git a/packages/ui-components/src/components/ComboBox/ComboBox.test.tsx b/packages/ui-components/src/components/ComboBox/ComboBox.test.tsx index d55d81d812..0d2b5484c3 100644 --- a/packages/ui-components/src/components/ComboBox/ComboBox.test.tsx +++ b/packages/ui-components/src/components/ComboBox/ComboBox.test.tsx @@ -583,4 +583,332 @@ describe("ComboBox", () => { expect(screen.getByRole("combobox")).toBeInTheDocument() expect(screen.getByRole("combobox")).toHaveAttribute("data-lolo", "1234") }) + + test("clears internal state when controlled value prop becomes empty", async () => { + const { rerender } = render( + + + + + + + ) + + const input = screen.getByRole("combobox") + // When controlled with value="option1", it shows the value + expect(input).toHaveValue("option1") + + // Rerender updates the same component instance with new props (simulating what supernova does) + // Update value prop to empty (simulating what supernova does after selection) + rerender( + + + + + + + ) + + // Should clear the value + await waitFor(() => { + expect(input).toHaveValue("") + }) + }) + + test("updates displayed value when controlled value prop changes to a new value", async () => { + const { rerender } = render( + + + + + + + ) + + const input = screen.getByRole("combobox") + expect(input).toHaveValue("option1") + + // Rerender updates the same component instance with new props + // Update value prop to a different value + rerender( + + + + + + + ) + + // Should update to the new value + await waitFor(() => { + expect(input).toHaveValue("option2") + }) + }) + + test("uncontrolled combobox: select option and it remains visible in the display", async () => { + const user = userEvent.setup() + const mockOnChange = vi.fn() + + render( + + + + + + + + ) + + const input = screen.getByRole("combobox") + expect(input).toHaveValue("") + + // Click to open dropdown + await user.click(input) + await waitFor(() => expect(screen.getByRole("listbox")).toBeInTheDocument()) + + // Select an option + await user.click(screen.getByRole("option", { name: "Option 2" })) + + // onChange should be called + expect(mockOnChange).toHaveBeenCalledWith("option2") + + // Dropdown should close + await waitFor(() => expect(screen.queryByRole("listbox")).not.toBeInTheDocument()) + + // The selected value should remain visible in uncontrolled mode (displays the label) + expect(input).toHaveValue("Option 2") + }) + + test("uncontrolled combobox: query resets after selecting a filtered option", async () => { + const user = userEvent.setup() + + render( + + + Apple + Banana + Cherry + + + ) + + const input = screen.getByRole("combobox") + + // Type a search that excludes Apple and Cherry (only Banana contains "ban") + await user.type(input, "ban") + await waitFor(() => { + expect(screen.getByRole("listbox")).toBeInTheDocument() + expect(screen.queryByRole("option", { name: "Apple" })).not.toBeInTheDocument() + expect(screen.getByRole("option", { name: "Banana" })).toBeInTheDocument() + expect(screen.queryByRole("option", { name: "Cherry" })).not.toBeInTheDocument() + }) + + // Select Banana - this triggers setQuery("") in handleChange + await user.click(screen.getByRole("option", { name: "Banana" })) + + // Dropdown should close and show selected value + await waitFor(() => { + expect(screen.queryByRole("listbox")).not.toBeInTheDocument() + expect(input).toHaveValue("Banana") + }) + + // Clear input value and click to reopen dropdown + // If setQuery("") wasn't called in handleChange, the old filter "ban" would still be active + await user.clear(input) + await user.click(input) + + // All options should be visible because query was reset to "" when Banana was selected + await waitFor(() => { + expect(screen.getByRole("listbox")).toBeInTheDocument() + expect(screen.getByRole("option", { name: "Apple" })).toBeInTheDocument() + expect(screen.getByRole("option", { name: "Banana" })).toBeInTheDocument() + expect(screen.getByRole("option", { name: "Cherry" })).toBeInTheDocument() + }) + }) + + test("honors explicitly cleared controlled value over defaultValue", async () => { + const { rerender } = render( + + + + + + + ) + + const input = screen.getByRole("combobox") + expect(input).toHaveValue("option1") + + // Explicitly clear the controlled value while defaultValue is still nonempty + rerender( + + + + + + + ) + + // Should show empty value, not fall back to defaultValue + await waitFor(() => { + expect(input).toHaveValue("") + }) + }) + + test("honors initially empty controlled value over defaultValue", () => { + render( + + + + + + + ) + + const input = screen.getByRole("combobox") + // Should show empty value because value="" was explicitly supplied, not defaultValue + expect(input).toHaveValue("") + }) + + test("closes dropdown when controlled value is cleared while menu is open", async () => { + const user = userEvent.setup() + const { rerender } = render( + + + + + + + ) + + const input = screen.getByRole("combobox") + + // Open the dropdown while value is set + await user.click(input) + await waitFor(() => { + expect(screen.getByRole("listbox")).toBeInTheDocument() + }) + + // Clear the controlled value while menu is open + rerender( + + + + + + + ) + + // Dropdown should close and aria-expanded should be false + await waitFor(() => { + expect(screen.queryByRole("listbox")).not.toBeInTheDocument() + expect(input).toHaveAttribute("aria-expanded", "false") + expect(input).toHaveValue("") + }) + }) + + test("resets query and restores all options when controlled value is cleared after filtering", async () => { + const user = userEvent.setup() + const { rerender } = render( + + + Apple + Banana + Cherry + + + ) + + const input = screen.getByRole("combobox") + const toggle = screen.getByRole("button") + + // Type a query that filters to only one option + await user.type(input, "ban") + await waitFor(() => { + expect(screen.getByRole("listbox")).toBeInTheDocument() + expect(screen.queryByRole("option", { name: "Apple" })).not.toBeInTheDocument() + expect(screen.getByRole("option", { name: "Banana" })).toBeInTheDocument() + expect(screen.queryByRole("option", { name: "Cherry" })).not.toBeInTheDocument() + }) + + // Select the filtered option + await user.click(screen.getByRole("option", { name: "Banana" })) + + // Simulate controlled component setting the value + rerender( + + + Apple + Banana + Cherry + + + ) + + await waitFor(() => { + expect(input).toHaveValue("Banana") + }) + + // Now clear the controlled value (simulating what supernova does) + rerender( + + + Apple + Banana + Cherry + + + ) + + // Value should be cleared and query should be reset + await waitFor(() => { + expect(input).toHaveValue("") + }) + + // Reopen the dropdown using the toggle button and verify all options are present (query was reset) + await user.click(toggle) + await waitFor(() => { + expect(screen.getByRole("listbox")).toBeInTheDocument() + expect(screen.getByRole("option", { name: "Apple" })).toBeInTheDocument() + expect(screen.getByRole("option", { name: "Banana" })).toBeInTheDocument() + expect(screen.getByRole("option", { name: "Cherry" })).toBeInTheDocument() + }) + }) + + test("fires onInputChange with empty value when query is cleared after selection", async () => { + // Track values at call time, not by reference + const capturedValues: string[] = [] + const handleInputChange = vi.fn((e: React.ChangeEvent) => { + capturedValues.push(e.target.value) + }) + const handleChange = vi.fn() + + await waitFor(() => + render( + + + Apple + Banana + + + ) + ) + + const user = userEvent.setup() + const toggle = screen.getByRole("button") + + // Open dropdown using toggle button + await waitFor(() => user.click(toggle)) + expect(screen.getByRole("listbox")).toBeInTheDocument() + + // Select an option (this triggers onChange and should also trigger onInputChange with empty value) + await waitFor(async () => { + await user.click(screen.getByRole("option", { name: "Apple" })) + expect(handleChange).toHaveBeenCalled() + }) + + // onInputChange should be fired with empty value when query is cleared after selection + expect(handleInputChange).toHaveBeenCalled() + expect(capturedValues).toContain("") + }) })