From de2086a6a7da6609aeb99346ddc8f2cc28d1d834 Mon Sep 17 00:00:00 2001 From: HenriqueLimas Date: Wed, 26 Aug 2026 14:58:17 -0700 Subject: [PATCH 1/2] feat(evo-react): add combobox --- .changeset/calm-comboboxes-listen.md | 5 + .claude/skills/evo-app-migrate-react/SKILL.md | 1 + .../components/evo-combobox.md | 66 ++++ ...2026-08-25-carousel-autoplay-test-flake.md | 12 + ...6-08-25-evo-marko-combobox-option-value.md | 12 + .../2026-08-25-evo-react-lint-baseline.md | 12 + packages/evo-react/src/combobox/README.md | 5 + .../src/combobox/combobox-option.tsx | 95 ++++++ .../src/combobox/combobox.stories.tsx | 100 ++++++ packages/evo-react/src/combobox/combobox.tsx | 285 ++++++++++++++++++ packages/evo-react/src/combobox/context.tsx | 52 ++++ packages/evo-react/src/combobox/index.ts | 10 + .../test/__snapshots__/test.server.tsx.snap | 9 + .../src/combobox/test/test.browser.tsx | 234 ++++++++++++++ .../src/combobox/test/test.server.tsx | 47 +++ packages/evo-react/src/combobox/types.ts | 89 ++++++ .../src/utils/use-active-descendant.ts | 186 ++++++++++++ 17 files changed, 1220 insertions(+) create mode 100644 .changeset/calm-comboboxes-listen.md create mode 100644 .claude/skills/evo-app-migrate-react/components/evo-combobox.md create mode 100644 agent-feedback/items/2026-08-25-carousel-autoplay-test-flake.md create mode 100644 agent-feedback/items/2026-08-25-evo-marko-combobox-option-value.md create mode 100644 agent-feedback/items/2026-08-25-evo-react-lint-baseline.md create mode 100644 packages/evo-react/src/combobox/README.md create mode 100644 packages/evo-react/src/combobox/combobox-option.tsx create mode 100644 packages/evo-react/src/combobox/combobox.stories.tsx create mode 100644 packages/evo-react/src/combobox/combobox.tsx create mode 100644 packages/evo-react/src/combobox/context.tsx create mode 100644 packages/evo-react/src/combobox/index.ts create mode 100644 packages/evo-react/src/combobox/test/__snapshots__/test.server.tsx.snap create mode 100644 packages/evo-react/src/combobox/test/test.browser.tsx create mode 100644 packages/evo-react/src/combobox/test/test.server.tsx create mode 100644 packages/evo-react/src/combobox/types.ts create mode 100644 packages/evo-react/src/utils/use-active-descendant.ts diff --git a/.changeset/calm-comboboxes-listen.md b/.changeset/calm-comboboxes-listen.md new file mode 100644 index 00000000000..d82c5d35c08 --- /dev/null +++ b/.changeset/calm-comboboxes-listen.md @@ -0,0 +1,5 @@ +--- +"@evo-web/react": patch +--- + +Add EvoCombobox component. diff --git a/.claude/skills/evo-app-migrate-react/SKILL.md b/.claude/skills/evo-app-migrate-react/SKILL.md index 45d87254716..9e59f4b4606 100644 --- a/.claude/skills/evo-app-migrate-react/SKILL.md +++ b/.claude/skills/evo-app-migrate-react/SKILL.md @@ -70,6 +70,7 @@ When migrating a listed component, read the linked file completely and apply the - `ebay-character-count`: [evo-character-count.md](components/evo-character-count.md) - `ebay-checkbox`: [evo-checkbox.md](components/evo-checkbox.md) - `ebay-chip`: [evo-chip.md](components/evo-chip.md) +- `ebay-combobox`: [evo-combobox.md](components/evo-combobox.md) - `ebay-confirm-dialog`: [evo-confirm-dialog.md](components/evo-confirm-dialog.md) - `ebay-cta-button`: [evo-cta-button.md](components/evo-cta-button.md) - `ebay-details`: [evo-details.md](components/evo-details.md) diff --git a/.claude/skills/evo-app-migrate-react/components/evo-combobox.md b/.claude/skills/evo-app-migrate-react/components/evo-combobox.md new file mode 100644 index 00000000000..3ae7c6131e5 --- /dev/null +++ b/.claude/skills/evo-app-migrate-react/components/evo-combobox.md @@ -0,0 +1,66 @@ +# ebay-combobox → evo-combobox + +EvoCombobox is a textbox with text suggestions. It renders its input and listbox internally. Pass `EvoComboboxOption` components directly as children and replace `EbayComboboxButton` with the root `postfix` prop. + +## Composition + +**Before:** + +```tsx + + + + + + +``` + +**After:** + +```tsx +, + buttonProps: { a11yText: "Clear", onClick: clear }, + }} +> + + +``` + +Options render their `text` and do not accept `children`. The new `sticky` prop keeps an option visible when `autocomplete="list"` filters other options. + +## Value changes + +Replace `onInputChange` with `onValueChange`. It receives the displayed string after typing or committed option selection. + +```tsx + + + +``` + +The legacy blur-based `onChange` behavior is removed; use native `onBlur` when needed. Root `onSelect` is removed because this component does not manage selected option identity. + +## Open state + +Rename open-state props and callbacks: + +```diff +- expanded={expanded} +- onExpand={() => setExpanded(true)} +- onCollapse={() => setExpanded(false)} ++ open={open} ++ onOpenChange={setOpen} +``` + +Use `defaultOpen` for an initially open uncontrolled combobox. The new `strategy` prop accepts `"absolute"` or `"fixed"` and defaults to `"absolute"`. + +## Removed props + +- `dropdownRef`: use the native React 19 input `ref`; listbox refs are internal. +- `opaqueLabel`: not supported by Evo Marko or Evo React input conventions. +- `onFloatingLabelInit`: the floating label no longer has an imperative initialization phase. +- `onSelect`: use `onValueChange` for committed textbox value changes. +- Option `value`: EvoCombobox does not manage machine-readable selection identity. +- Option `selected`: selection state is managed by `EvoCombobox`. diff --git a/agent-feedback/items/2026-08-25-carousel-autoplay-test-flake.md b/agent-feedback/items/2026-08-25-carousel-autoplay-test-flake.md new file mode 100644 index 00000000000..0b985757747 --- /dev/null +++ b/agent-feedback/items/2026-08-25-carousel-autoplay-test-flake.md @@ -0,0 +1,12 @@ +--- +type: dx +impact: med +effort: med +site: packages/ebayui-core/src/components/ebay-carousel/test/test.browser.js › waitForCarouselUpdate +--- + +# Stabilize the carousel autoplay wraparound test + +The discrete-carousel autoplay scenario can time out waiting for one `move` event when the full repository build runs package tests concurrently. The same file passes in isolation, so the assertion depends on timing or shared browser load rather than a deterministic state transition. Drive the autoplay clock explicitly or wait on the final rendered state without relying on a short real-time event window. + +Check: Run `npm run build`; the scenario `when auto play runs at the end > then it is displaying the first item` may report zero `move` events, while `cd packages/ebayui-core && npx vitest run src/components/ebay-carousel/test/test.browser.js --browser.headless` passes all 53 tests. diff --git a/agent-feedback/items/2026-08-25-evo-marko-combobox-option-value.md b/agent-feedback/items/2026-08-25-evo-marko-combobox-option-value.md new file mode 100644 index 00000000000..a58b9776482 --- /dev/null +++ b/agent-feedback/items/2026-08-25-evo-marko-combobox-option-value.md @@ -0,0 +1,12 @@ +--- +type: unclear +impact: med +effort: low +site: packages/evo-marko/src/tags/evo-combobox/index.marko › ComboboxOption +--- + +# Define the Evo Marko combobox option value contract + +`ComboboxOption` declares `text` and `sticky` but the Storybook `option` argTypes also document `value` as an optional value that defaults to `text`. The template spreads `value` onto the option div while selection and `aria-selected` use only `text`, so consumers cannot tell whether `value` is supported metadata or an accidental HTML attribute. Add `value` to the interface with defined selection semantics, or remove it from the story controls. + +Check: Render `<@option text="Campaign" value="campaign-id"/>`, select the option, and observe that the input contains `Campaign` while `value="campaign-id"` is passed to the option div without participating in selection. diff --git a/agent-feedback/items/2026-08-25-evo-react-lint-baseline.md b/agent-feedback/items/2026-08-25-evo-react-lint-baseline.md new file mode 100644 index 00000000000..27c3cfd6391 --- /dev/null +++ b/agent-feedback/items/2026-08-25-evo-react-lint-baseline.md @@ -0,0 +1,12 @@ +--- +type: dx +impact: med +effort: med +site: packages/evo-react/package.json › scripts.lint +--- + +# Restore a clean Evo React lint baseline + +The package lint command fails in existing alert-dialog and confirm-dialog files on `autoFocus` and `closedby`, in button and icon-button stories and tests on empty custom anchors, and in the generated icon story on `@ts-nocheck`. These unrelated failures prevent component migrations from using the workspace lint command as a merge gate. Fix the source violations or narrow generated-file linting with an intentional repository rule. + +Check: Run `npm run lint -w packages/evo-react` and observe failures outside `src/combobox/`. diff --git a/packages/evo-react/src/combobox/README.md b/packages/evo-react/src/combobox/README.md new file mode 100644 index 00000000000..b7658854916 --- /dev/null +++ b/packages/evo-react/src/combobox/README.md @@ -0,0 +1,5 @@ +# EvoCombobox + +## Documentation + +[Storybook](https://opensource.ebay.com/evo-web/react/?path=/docs/form-input-evo-combobox--documentation) diff --git a/packages/evo-react/src/combobox/combobox-option.tsx b/packages/evo-react/src/combobox/combobox-option.tsx new file mode 100644 index 00000000000..57a9a6740ad --- /dev/null +++ b/packages/evo-react/src/combobox/combobox-option.tsx @@ -0,0 +1,95 @@ +import { useEffect, useId } from "react"; +import type { Ref, RefObject } from "react"; +import classNames from "classnames"; +import { useActiveDescendantItem } from "../utils/use-active-descendant"; +import { useRefTee } from "../utils/use-ref-tee"; +import { useComboboxContext } from "./context"; +import type { EvoComboboxOptionProps } from "./types"; + +function matchesFilter(text: string, filterValue: string) { + return text.toLowerCase().includes(filterValue.trim().toLowerCase()); +} + +export function EvoComboboxOption({ + className, + onClick, + onKeyDown, + onMouseDown, + ref, + sticky = false, + text, + ...rest +}: EvoComboboxOptionProps) { + const generatedId = useId(); + const { + activeDescendant, + autocomplete, + displayedValue, + filterValue, + selectOption, + } = useComboboxContext(); + const [optionRef, internalRef] = useRefTee( + ref as Ref, + null, + ); + const hidden = + autocomplete === "list" && !sticky && !matchesFilter(text, filterValue); + const { isActive } = useActiveDescendantItem({ + activeDescendant, + enabled: !hidden, + item: { + key: generatedId, + id: generatedId, + ref: internalRef as RefObject, + data: text, + }, + }); + + useEffect(() => { + if (isActive) { + internalRef.current?.scrollIntoView({ block: "nearest" }); + } + }, [internalRef, isActive]); + + if (hidden) { + return null; + } + + return ( +
{ + event.preventDefault(); + onMouseDown?.(event); + }} + onClick={(event) => { + onClick?.(event); + if (!event.defaultPrevented) { + selectOption(text); + } + }} + onKeyDown={(event) => { + onKeyDown?.(event); + if ( + !event.defaultPrevented && + (event.key === "Enter" || event.key === " ") + ) { + event.preventDefault(); + selectOption(text); + } + }} + > + {text} +
+ ); +} diff --git a/packages/evo-react/src/combobox/combobox.stories.tsx b/packages/evo-react/src/combobox/combobox.stories.tsx new file mode 100644 index 00000000000..cac6192822d --- /dev/null +++ b/packages/evo-react/src/combobox/combobox.stories.tsx @@ -0,0 +1,100 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { action } from "storybook/actions"; +import { EvoIconClose12 } from "../icon/icons/close-12"; +import { EvoCombobox } from "./combobox"; +import { EvoComboboxOption } from "./combobox-option"; + +const meta: Meta = { + title: "form input/evo-combobox", + component: EvoCombobox, + subcomponents: { EvoComboboxOption }, + tags: ["autodocs"], + parameters: { + docs: { + description: { + component: ` +A text input with a filtered listbox of selectable options. + +## Usage + +\`\`\`tsx +import { + EvoCombobox, + EvoComboboxOption, +} from "@evo-web/react/combobox"; +\`\`\` + `, + }, + }, + }, + argTypes: { + autocomplete: { + control: "select", + options: ["none", "list"], + }, + listSelection: { + control: "select", + options: ["automatic", "manual"], + }, + strategy: { + control: "select", + options: ["absolute", "fixed"], + }, + borderless: { + control: "boolean", + }, + defaultOpen: { + control: "boolean", + }, + disabled: { + control: "boolean", + }, + floatingLabel: { + control: "text", + }, + fluid: { + control: "boolean", + }, + open: { + control: "boolean", + }, + postfix: { + control: false, + }, + onValueChange: { + action: "valueChange", + table: { category: "Events" }, + }, + onOpenChange: { + action: "openChange", + table: { category: "Events" }, + }, + }, + args: { + floatingLabel: "Campaign", + autocomplete: "none", + listSelection: "automatic", + placeholder: "Choose a campaign", + postfix: { + icon: , + buttonProps: { + a11yText: "Clear", + onClick: action("clear"), + }, + }, + }, +}; + +export default meta; +type Story = StoryObj; + +export const Default: Story = { + render: (args) => ( + + + + + + + ), +}; diff --git a/packages/evo-react/src/combobox/combobox.tsx b/packages/evo-react/src/combobox/combobox.tsx new file mode 100644 index 00000000000..a0fa133ffa1 --- /dev/null +++ b/packages/evo-react/src/combobox/combobox.tsx @@ -0,0 +1,285 @@ +import { useCallback, useId, useState } from "react"; +import type { + ChangeEvent, + FocusEvent, + KeyboardEvent, + MouseEvent, + RefObject, +} from "react"; +import classNames from "classnames"; +import { EvoIconButton } from "../icon-button"; +import { useActiveDescendant } from "../utils/use-active-descendant"; +import { useExpander } from "../utils/use-expander"; +import { useFloatingLabel } from "../utils/use-floating-label"; +import { useRefTee } from "../utils/use-ref-tee"; +import { ComboboxProvider } from "./context"; +import type { EvoComboboxProps } from "./types"; +import "@ebay/skin/combobox.mjs"; + +export function EvoCombobox({ + autocomplete = "none", + borderless = false, + children, + className, + defaultOpen = false, + defaultValue = "", + disabled, + floatingLabel: floatingLabelText, + fluid = false, + id, + listSelection = "automatic", + onBlur, + onChange, + onClick, + onFocus, + onKeyDown, + onOpenChange, + onValueChange, + open, + placeholder, + postfix, + ref, + strategy = "absolute", + style, + value, + ...inputProps +}: EvoComboboxProps) { + const generatedId = useId(); + const inputId = id ?? generatedId; + const listboxId = `${inputId}-listbox`; + const isControlled = value !== undefined; + const [uncontrolledValue, setUncontrolledValue] = useState(defaultValue); + const [temporaryValue, setTemporaryValue] = useState(null); + const [focused, setFocused] = useState(false); + const currentValue = isControlled ? value : uncontrolledValue; + + if (listSelection === "manual" && temporaryValue !== null) { + setTemporaryValue(null); + } + + const displayedValue = temporaryValue ?? currentValue; + const expander = useExpander({ + defaultOpen, + offset: 4, + onOpenChange, + open, + placement: "bottom-start", + strategy, + }); + const [setListboxElement, listboxRef] = useRefTee( + expander.refs.setFloating, + null, + ); + const [setForwardedInputElement] = useRefTee( + ref, + null, + ); + const activeDescendant = useActiveDescendant({ + containerRef: listboxRef as RefObject, + shouldWrap: true, + }); + const floatingLabel = useFloatingLabel({ + containerTagName: fluid ? "div" : "span", + disabled, + focused: focused || expander.open, + text: floatingLabelText, + value: displayedValue, + }); + + const setInputElement = useCallback( + (node: HTMLInputElement | null) => { + expander.refs.setReference(node); + setForwardedInputElement(node); + }, + [expander.refs.setReference, setForwardedInputElement], + ); + + const requestOpen = useCallback( + (nextOpen: boolean) => { + if (nextOpen !== expander.open) { + expander.setOpen(nextOpen); + } + }, + [expander.open, expander.setOpen], + ); + + const updateValue = useCallback( + (nextValue: string) => { + setTemporaryValue(null); + if (!isControlled) { + setUncontrolledValue(nextValue); + } + if (nextValue !== currentValue) { + onValueChange?.(nextValue); + } + }, + [currentValue, isControlled, onValueChange], + ); + + const selectOption = useCallback( + (text: string) => { + updateValue(text); + activeDescendant.reset(); + requestOpen(false); + }, + [activeDescendant.reset, requestOpen, updateValue], + ); + + const handleFocusOut = (event: FocusEvent) => { + if (event.currentTarget.contains(event.relatedTarget as Node | null)) { + return; + } + + setFocused(false); + const preview = temporaryValue; + requestOpen(false); + activeDescendant.reset(); + + if (preview === null) { + return; + } + + updateValue(preview); + }; + + const handleChange = (event: ChangeEvent) => { + activeDescendant.reset(); + updateValue(event.currentTarget.value); + requestOpen(true); + onChange?.(event); + }; + + const handleFocus = (event: FocusEvent) => { + setFocused(true); + requestOpen(true); + onFocus?.(event); + }; + + const handleBlur = (event: FocusEvent) => { + onBlur?.(event); + }; + + const handleClick = (event: MouseEvent) => { + requestOpen(true); + onClick?.(event); + }; + + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === "ArrowDown" || event.key === "ArrowUp") { + event.preventDefault(); + if (!expander.open) { + requestOpen(true); + } else { + const option = + event.key === "ArrowDown" + ? activeDescendant.activateNext() + : activeDescendant.activatePrevious(); + if (listSelection === "automatic") { + setTemporaryValue(option?.data ?? null); + } + } + } else if (event.key === "Enter" && expander.open) { + const option = activeDescendant.getActiveItem(); + if (option) { + event.preventDefault(); + selectOption(option.data); + } else { + requestOpen(false); + } + } else if (event.key === "Escape") { + setTemporaryValue(null); + activeDescendant.reset(); + requestOpen(false); + } + + onKeyDown?.(event); + }; + + const Wrapper = fluid ? "div" : "span"; + const activeOption = expander.open + ? activeDescendant.getActiveItem() + : undefined; + const postfixButtonProps = postfix?.buttonProps; + const postfixMouseDown = postfixButtonProps?.onMouseDown; + + return ( + + + + + + + {postfix && + (postfixButtonProps ? ( + { + event.preventDefault(); + postfixMouseDown?.(event); + }} + > + {postfix.icon} + + ) : ( + postfix.icon + ))} + +
+ {children} +
+ + + + ); +} diff --git a/packages/evo-react/src/combobox/context.tsx b/packages/evo-react/src/combobox/context.tsx new file mode 100644 index 00000000000..83d37f0e74c --- /dev/null +++ b/packages/evo-react/src/combobox/context.tsx @@ -0,0 +1,52 @@ +import { createContext, use, useMemo } from "react"; +import type { ReactNode } from "react"; +import type { ActiveDescendant } from "../utils/use-active-descendant"; +import type { ComboboxAutocomplete } from "./types"; + +export type ComboboxContextValue = { + activeDescendant: ActiveDescendant; + autocomplete: ComboboxAutocomplete; + displayedValue: string; + filterValue: string; + selectOption: (text: string) => void; +}; + +const ComboboxContext = createContext( + undefined, +); + +export function useComboboxContext() { + const context = use(ComboboxContext); + if (!context) { + throw new Error( + "EvoComboboxOption must be used within an EvoCombobox component", + ); + } + return context; +} + +type ComboboxProviderProps = ComboboxContextValue & { + children?: ReactNode; +}; + +export function ComboboxProvider({ + activeDescendant, + autocomplete, + displayedValue, + filterValue, + selectOption, + children, +}: ComboboxProviderProps) { + const value = useMemo( + () => ({ + activeDescendant, + autocomplete, + displayedValue, + filterValue, + selectOption, + }), + [activeDescendant, autocomplete, displayedValue, filterValue, selectOption], + ); + + return {children}; +} diff --git a/packages/evo-react/src/combobox/index.ts b/packages/evo-react/src/combobox/index.ts new file mode 100644 index 00000000000..5430e70ee07 --- /dev/null +++ b/packages/evo-react/src/combobox/index.ts @@ -0,0 +1,10 @@ +export { EvoCombobox } from "./combobox"; +export { EvoComboboxOption } from "./combobox-option"; +export type { + ComboboxAutocomplete, + ComboboxListSelection, + ComboboxStrategy, + EvoComboboxOptionProps, + EvoComboboxPostfixProps, + EvoComboboxProps, +} from "./types"; diff --git a/packages/evo-react/src/combobox/test/__snapshots__/test.server.tsx.snap b/packages/evo-react/src/combobox/test/__snapshots__/test.server.tsx.snap new file mode 100644 index 00000000000..467c1c0b2f6 --- /dev/null +++ b/packages/evo-react/src/combobox/test/__snapshots__/test.server.tsx.snap @@ -0,0 +1,9 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`EvoCombobox SSR > renders a fluid combobox with a floating label 1`] = `"
August Campaign
Basic Offer
"`; + +exports[`EvoCombobox SSR > renders an actionable postfix 1`] = `"
August Campaign
Basic Offer
"`; + +exports[`EvoCombobox SSR > renders an open borderless combobox 1`] = `""`; + +exports[`EvoCombobox SSR > renders the default combobox 1`] = `"
August Campaign
Basic Offer
"`; diff --git a/packages/evo-react/src/combobox/test/test.browser.tsx b/packages/evo-react/src/combobox/test/test.browser.tsx new file mode 100644 index 00000000000..b3266440b0a --- /dev/null +++ b/packages/evo-react/src/combobox/test/test.browser.tsx @@ -0,0 +1,234 @@ +import { createRef, useState } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { userEvent } from "vitest/browser"; +import { render } from "vitest-browser-react"; +import { EvoIconClose12 } from "../../icon/icons/close-12"; +import { EvoCombobox } from "../combobox"; +import { EvoComboboxOption } from "../combobox-option"; +import type { EvoComboboxProps } from "../types"; + +function ComboboxFixture(props: EvoComboboxProps) { + return ( + + + + + ); +} + +describe("evo-combobox", () => { + let user: ReturnType; + + beforeEach(() => { + user = userEvent.setup(); + }); + + afterEach(() => { + user.cleanup(); + }); + + describe("ARIA attributes", () => { + it("links the combobox to its listbox", async () => { + const screen = await render(); + const input = screen.getByRole("combobox", { name: "Campaign" }); + + await user.click(input); + + const listbox = screen.getByRole("listbox"); + await expect.element(input).toHaveAttribute("aria-expanded", "true"); + await expect + .element(input) + .toHaveAttribute("aria-controls", listbox.element().id); + await expect + .element(input) + .toHaveAttribute("aria-owns", listbox.element().id); + }); + + it("sets the active descendant during keyboard navigation", async () => { + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + + const option = screen.getByRole("option", { name: "August Campaign" }); + await expect + .element(input) + .toHaveAttribute("aria-activedescendant", option.element().id); + await expect.element(option).toHaveAttribute("aria-selected", "true"); + }); + }); + + describe("value changes", () => { + it("passes the displayed value to onValueChange while typing", async () => { + const onValueChange = vi.fn(); + const screen = await render( + , + ); + + await user.type(screen.getByRole("combobox"), "Basic"); + + expect(onValueChange).toHaveBeenLastCalledWith("Basic"); + await expect.element(screen.getByRole("combobox")).toHaveValue("Basic"); + }); + + it("passes the native change event to onChange while typing", async () => { + const onChange = vi.fn(); + const screen = await render( + onChange(event.currentTarget.value)} + />, + ); + + await user.type(screen.getByRole("combobox"), "B"); + + expect(onChange).toHaveBeenCalledWith("B"); + }); + + it("passes selected option text only to onValueChange", async () => { + const onChange = vi.fn(); + const onValueChange = vi.fn(); + const screen = await render( + , + ); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.click(screen.getByRole("option", { name: "August Campaign" })); + + expect(onValueChange).toHaveBeenCalledWith("August Campaign"); + expect(onChange).not.toHaveBeenCalled(); + await expect.element(input).toHaveValue("August Campaign"); + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + }); + + it("supports controlled values for typing and selection", async () => { + function ControlledCombobox() { + const [value, setValue] = useState(""); + return ; + } + + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.type(input, "B"); + await expect.element(input).toHaveValue("B"); + await user.click(screen.getByRole("option", { name: "Basic Offer" })); + await expect.element(input).toHaveValue("Basic Offer"); + }); + }); + + describe("filtering", () => { + it("filters options by text when autocomplete is list", async () => { + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.type(input, "Basic"); + + await expect + .element(screen.getByRole("option", { name: "August Campaign" })) + .not.toBeInTheDocument(); + await expect + .element(screen.getByRole("option", { name: "Basic Offer" })) + .toBeInTheDocument(); + }); + + it("keeps sticky options visible when they do not match", async () => { + const screen = await render( + + + + , + ); + + await user.type(screen.getByRole("combobox"), "missing"); + + await expect + .element(screen.getByRole("option", { name: "Create campaign" })) + .toBeInTheDocument(); + await expect + .element(screen.getByRole("option", { name: "Basic Offer" })) + .not.toBeInTheDocument(); + }); + }); + + describe("keyboard interactions", () => { + it("previews and selects options in automatic mode", async () => { + const onValueChange = vi.fn(); + const screen = await render( + , + ); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + + await expect.element(input).toHaveValue("August Campaign"); + expect(onValueChange).not.toHaveBeenCalled(); + + await user.keyboard("{Enter}"); + + expect(onValueChange).toHaveBeenCalledWith("August Campaign"); + }); + + it("does not preview option text in manual mode", async () => { + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + + await expect.element(input).toHaveValue(""); + await expect.element(input).toHaveAttribute("aria-activedescendant"); + }); + + it("restores the committed value on Escape", async () => { + const screen = await render( + , + ); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + await expect.element(input).toHaveValue("August Campaign"); + + await user.keyboard("{Escape}"); + + await expect.element(input).toHaveValue("Initial value"); + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + }); + }); + + describe("postfix action", () => { + it("runs the button callback without closing the listbox", async () => { + const onClick = vi.fn(); + const screen = await render( + , + buttonProps: { a11yText: "Clear", onClick }, + }} + />, + ); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.click(screen.getByRole("button", { name: "Clear" })); + + expect(onClick).toHaveBeenCalledTimes(1); + await expect.element(input).toHaveFocus(); + await expect.element(input).toHaveAttribute("aria-expanded", "true"); + }); + }); + + describe("refs", () => { + it("passes the ref to the input", async () => { + const ref = createRef(); + + await render(); + + expect(ref.current).toBeInstanceOf(HTMLInputElement); + expect(ref.current?.getAttribute("role")).toBe("combobox"); + }); + }); +}); diff --git a/packages/evo-react/src/combobox/test/test.server.tsx b/packages/evo-react/src/combobox/test/test.server.tsx new file mode 100644 index 00000000000..41ce0b14308 --- /dev/null +++ b/packages/evo-react/src/combobox/test/test.server.tsx @@ -0,0 +1,47 @@ +import { describe, expect, it } from "vitest"; +import { renderToString } from "react-dom/server"; +import { EvoIconClose12 } from "../../icon/icons/close-12"; +import { EvoCombobox } from "../combobox"; +import { EvoComboboxOption } from "../combobox-option"; + +function renderCombobox(props: Parameters[0] = {}) { + return renderToString( + + + + , + ); +} + +describe("EvoCombobox SSR", () => { + it("renders the default combobox", () => { + expect(renderCombobox()).toMatchSnapshot(); + }); + + it("renders a fluid combobox with a floating label", () => { + expect( + renderCombobox({ + defaultValue: "August Campaign", + floatingLabel: "Campaign", + fluid: true, + }), + ).toMatchSnapshot(); + }); + + it("renders an open borderless combobox", () => { + expect( + renderCombobox({ borderless: true, defaultOpen: true }), + ).toMatchSnapshot(); + }); + + it("renders an actionable postfix", () => { + expect( + renderCombobox({ + postfix: { + icon: , + buttonProps: { a11yText: "Clear" }, + }, + }), + ).toMatchSnapshot(); + }); +}); diff --git a/packages/evo-react/src/combobox/types.ts b/packages/evo-react/src/combobox/types.ts new file mode 100644 index 00000000000..6fafd154bea --- /dev/null +++ b/packages/evo-react/src/combobox/types.ts @@ -0,0 +1,89 @@ +import type { ComponentProps, CSSProperties, ReactNode } from "react"; +import type { Strategy } from "@floating-ui/react"; +import type { NativeIconButtonProps } from "../icon-button"; + +/** How the combobox filters its options. */ +export type ComboboxAutocomplete = "list" | "none"; + +/** How keyboard navigation previews the active option. */ +export type ComboboxListSelection = "automatic" | "manual"; + +/** Floating UI positioning strategy used by the listbox. */ +export type ComboboxStrategy = Strategy; + +/** Optional icon rendered after the combobox input. */ +export type EvoComboboxPostfixProps = { + /** Icon displayed after the input. */ + icon: ReactNode; + /** Makes the icon actionable using an EvoIconButton. */ + buttonProps?: Omit; +}; + +export type EvoComboboxProps = Omit< + ComponentProps<"input">, + | "aria-activedescendant" + | "aria-autocomplete" + | "aria-controls" + | "aria-expanded" + | "aria-haspopup" + | "aria-owns" + | "autoComplete" + | "children" + | "className" + | "defaultValue" + | "onSelect" + | "prefix" + | "role" + | "style" + | "type" + | "value" +> & { + /** + * Filters visible options to those whose text includes the current input + * value. Defaults to `"none"`. + */ + autocomplete?: ComboboxAutocomplete; + /** Removes the input border. */ + borderless?: boolean; + /** EvoComboboxOption children rendered in the component's listbox. */ + children?: ReactNode; + /** Class name applied to the combobox wrapper. */ + className?: string; + /** Initial open state for uncontrolled usage. Ignored when `open` is provided. */ + defaultOpen?: boolean; + /** Initial input value for uncontrolled usage. Ignored when `value` is provided. */ + defaultValue?: string; + /** Floating label text shown above the input when focused or filled. */ + floatingLabel?: string; + /** Stretches the component to fill its container. */ + fluid?: boolean; + /** + * Whether arrow-key navigation previews the highlighted option text in the + * input. Defaults to `"automatic"`. + */ + listSelection?: ComboboxListSelection; + /** Called with the displayed input value after typing or option selection. */ + onValueChange?: (value: string) => void; + /** Called when the listbox requests to open or close. */ + onOpenChange?: (open: boolean) => void; + /** Controlled listbox visibility. Manage changes with `onOpenChange`. */ + open?: boolean; + /** Optional icon or icon button rendered after the input. */ + postfix?: EvoComboboxPostfixProps; + /** Listbox positioning strategy. Defaults to `"absolute"`. */ + strategy?: ComboboxStrategy; + /** Style applied to the combobox wrapper. */ + style?: CSSProperties; + /** Controlled displayed input value. */ + value?: string; +}; + +export type EvoComboboxOptionProps = Omit< + ComponentProps<"div">, + "aria-selected" | "children" | "id" | "role" | "tabIndex" +> & { + /** Always shown during list autocomplete, even when its text does not match. */ + sticky?: boolean; + /** Display text shown by the option and written into the input when selected. */ + text: string; +}; diff --git a/packages/evo-react/src/utils/use-active-descendant.ts b/packages/evo-react/src/utils/use-active-descendant.ts new file mode 100644 index 00000000000..991cae35cd2 --- /dev/null +++ b/packages/evo-react/src/utils/use-active-descendant.ts @@ -0,0 +1,186 @@ +import { useCallback, useLayoutEffect, useRef, useState } from "react"; +import type { RefObject } from "react"; + +export type ActiveDescendantItem = { + key: Key; + id: string; + ref: RefObject; + data: Data; +}; + +export type ActiveDescendant = { + activeKey: Key | null; + registerItem: (item: ActiveDescendantItem) => () => void; + getActiveItem: () => ActiveDescendantItem | undefined; + activateNext: () => ActiveDescendantItem | undefined; + activatePrevious: () => ActiveDescendantItem | undefined; + reset: () => void; +}; + +type UseActiveDescendantOptions = { + containerRef: RefObject; + shouldWrap?: boolean; +}; + +type UseActiveDescendantItemOptions = { + activeDescendant: ActiveDescendant; + item: ActiveDescendantItem; + enabled?: boolean; +}; + +function compareDomOrder( + first: ActiveDescendantItem, + second: ActiveDescendantItem, +) { + const firstElement = first.ref.current; + const secondElement = second.ref.current; + + if (!firstElement || !secondElement || firstElement === secondElement) { + return 0; + } + + return firstElement.compareDocumentPosition(secondElement) & + Node.DOCUMENT_POSITION_FOLLOWING + ? -1 + : 1; +} + +export function useActiveDescendant({ + containerRef, + shouldWrap = true, +}: UseActiveDescendantOptions): ActiveDescendant { + const itemsRef = useRef(new Map>()); + const [activeKey, setActiveKeyState] = useState(null); + const activeKeyRef = useRef(null); + activeKeyRef.current = activeKey; + + const getItems = useCallback(() => { + const container = containerRef.current; + + return [...itemsRef.current.values()] + .filter(({ ref }) => { + const element = ref.current; + return Boolean(element && container?.contains(element)); + }) + .sort(compareDomOrder); + }, [containerRef]); + + const getActiveItem = useCallback(() => { + const key = activeKeyRef.current; + if (key === null) { + return undefined; + } + + return getItems().find((item) => item.key === key); + }, [getItems]); + + const activateKey = useCallback((key: Key | null) => { + if (key === null) { + activeKeyRef.current = null; + setActiveKeyState(null); + return; + } + + const item = itemsRef.current.get(key); + if (!item || !item.ref.current) { + return; + } + + activeKeyRef.current = key; + setActiveKeyState(key); + }, []); + + const reset = useCallback(() => { + activateKey(null); + }, [activateKey]); + + const registerItem = useCallback( + (item: ActiveDescendantItem) => { + const key = item.key; + itemsRef.current.set(key, item); + + return () => { + if (itemsRef.current.get(key) !== item) { + return; + } + + itemsRef.current.delete(key); + if (activeKeyRef.current === key) { + activateKey(null); + } + }; + }, + [activateKey], + ); + + const move = useCallback( + (direction: 1 | -1) => { + const items = getItems(); + if (items.length === 0) { + return undefined; + } + + const currentIndex = items.findIndex( + ({ key }) => key === activeKeyRef.current, + ); + let nextIndex: number | null; + + if (currentIndex === -1) { + nextIndex = direction === 1 ? 0 : items.length - 1; + } else { + const rawIndex = currentIndex + direction; + if (rawIndex < 0 || rawIndex >= items.length) { + nextIndex = shouldWrap ? null : currentIndex; + } else { + nextIndex = rawIndex; + } + } + + if (nextIndex === null) { + activateKey(null); + return undefined; + } + + const nextItem = items[nextIndex]; + activateKey(nextItem.key); + return nextItem; + }, + [activateKey, getItems, shouldWrap], + ); + + const activateNext = useCallback(() => move(1), [move]); + const activatePrevious = useCallback(() => move(-1), [move]); + + return { + activeKey, + registerItem, + getActiveItem, + activateNext, + activatePrevious, + reset, + }; +} + +export function useActiveDescendantItem({ + activeDescendant, + item, + enabled = true, +}: UseActiveDescendantItemOptions): { isActive: boolean } { + const itemRef = useRef(item); + itemRef.current.key = item.key; + itemRef.current.id = item.id; + itemRef.current.ref = item.ref; + itemRef.current.data = item.data; + + useLayoutEffect(() => { + if (!enabled) { + return; + } + + return activeDescendant.registerItem(itemRef.current); + }, [activeDescendant.registerItem, enabled]); + + return { + isActive: enabled && activeDescendant.activeKey === item.key, + }; +} From 59635a27d8f2aeb38669f2c7be0f7a4dbfa01294 Mon Sep 17 00:00:00 2001 From: HenriqueLimas Date: Thu, 3 Sep 2026 16:16:13 -0700 Subject: [PATCH 2/2] fix(evo-react): harden combobox preview, disabled state, and listbox width Keep keyboard preview tied to the active option so controlled updates cannot leave stale text. Block interaction while disabled. Apply Skin listbox positioning classes like Marko. Co-authored-by: Cursor --- .../components/evo-combobox.md | 8 +- ...2026-08-25-carousel-autoplay-test-flake.md | 12 - ...6-08-25-evo-marko-combobox-option-value.md | 12 - .../2026-08-25-evo-react-lint-baseline.md | 12 - .../src/combobox/combobox-option.tsx | 21 +- .../src/combobox/combobox.stories.tsx | 51 ++- packages/evo-react/src/combobox/combobox.tsx | 270 ++++++++---- packages/evo-react/src/combobox/context.tsx | 12 +- .../test/__snapshots__/test.server.tsx.snap | 12 +- .../src/combobox/test/test.browser.tsx | 387 +++++++++++++++++- .../src/combobox/test/test.server.tsx | 12 + .../src/utils/use-active-descendant.ts | 62 ++- packages/evo-react/src/utils/use-expander.ts | 11 +- 13 files changed, 719 insertions(+), 163 deletions(-) delete mode 100644 agent-feedback/items/2026-08-25-carousel-autoplay-test-flake.md delete mode 100644 agent-feedback/items/2026-08-25-evo-marko-combobox-option-value.md delete mode 100644 agent-feedback/items/2026-08-25-evo-react-lint-baseline.md diff --git a/.claude/skills/evo-app-migrate-react/components/evo-combobox.md b/.claude/skills/evo-app-migrate-react/components/evo-combobox.md index 3ae7c6131e5..e52b3dd2081 100644 --- a/.claude/skills/evo-app-migrate-react/components/evo-combobox.md +++ b/.claude/skills/evo-app-migrate-react/components/evo-combobox.md @@ -40,7 +40,9 @@ Replace `onInputChange` with `onValueChange`. It receives the displayed string a ``` -The legacy blur-based `onChange` behavior is removed; use native `onBlur` when needed. Root `onSelect` is removed because this component does not manage selected option identity. +The legacy blur-based `onChange` behavior is removed; use native `onBlur` when needed. Native `onBlur` runs after Evo's internal focus-leave processing, so controlled consumers may restore a value in their blur handler without the wrapper overwriting it afterward. + +EvoCombobox represents textbox text, not selected option identity. Text suggestions and free-form autocomplete can migrate to the text-only API. Legacy uses of option `value`, root `onSelect`, or separate ID and label state are entity selectors; entity selectors have no direct Evo replacement yet and require manual engineering review. Agents must not remove identity handling automatically. ## Open state @@ -62,5 +64,5 @@ Use `defaultOpen` for an initially open uncontrolled combobox. The new `strategy - `opaqueLabel`: not supported by Evo Marko or Evo React input conventions. - `onFloatingLabelInit`: the floating label no longer has an imperative initialization phase. - `onSelect`: use `onValueChange` for committed textbox value changes. -- Option `value`: EvoCombobox does not manage machine-readable selection identity. -- Option `selected`: selection state is managed by `EvoCombobox`. +- Option `value`: Text suggestion migrations should use the option's `text`; legacy machine-readable values identify an entity selector and require manual engineering review. +- Option `selected`: EvoCombobox does not manage committed option identity; do not remove legacy identity handling automatically. diff --git a/agent-feedback/items/2026-08-25-carousel-autoplay-test-flake.md b/agent-feedback/items/2026-08-25-carousel-autoplay-test-flake.md deleted file mode 100644 index 0b985757747..00000000000 --- a/agent-feedback/items/2026-08-25-carousel-autoplay-test-flake.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -type: dx -impact: med -effort: med -site: packages/ebayui-core/src/components/ebay-carousel/test/test.browser.js › waitForCarouselUpdate ---- - -# Stabilize the carousel autoplay wraparound test - -The discrete-carousel autoplay scenario can time out waiting for one `move` event when the full repository build runs package tests concurrently. The same file passes in isolation, so the assertion depends on timing or shared browser load rather than a deterministic state transition. Drive the autoplay clock explicitly or wait on the final rendered state without relying on a short real-time event window. - -Check: Run `npm run build`; the scenario `when auto play runs at the end > then it is displaying the first item` may report zero `move` events, while `cd packages/ebayui-core && npx vitest run src/components/ebay-carousel/test/test.browser.js --browser.headless` passes all 53 tests. diff --git a/agent-feedback/items/2026-08-25-evo-marko-combobox-option-value.md b/agent-feedback/items/2026-08-25-evo-marko-combobox-option-value.md deleted file mode 100644 index a58b9776482..00000000000 --- a/agent-feedback/items/2026-08-25-evo-marko-combobox-option-value.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -type: unclear -impact: med -effort: low -site: packages/evo-marko/src/tags/evo-combobox/index.marko › ComboboxOption ---- - -# Define the Evo Marko combobox option value contract - -`ComboboxOption` declares `text` and `sticky` but the Storybook `option` argTypes also document `value` as an optional value that defaults to `text`. The template spreads `value` onto the option div while selection and `aria-selected` use only `text`, so consumers cannot tell whether `value` is supported metadata or an accidental HTML attribute. Add `value` to the interface with defined selection semantics, or remove it from the story controls. - -Check: Render `<@option text="Campaign" value="campaign-id"/>`, select the option, and observe that the input contains `Campaign` while `value="campaign-id"` is passed to the option div without participating in selection. diff --git a/agent-feedback/items/2026-08-25-evo-react-lint-baseline.md b/agent-feedback/items/2026-08-25-evo-react-lint-baseline.md deleted file mode 100644 index 27c3cfd6391..00000000000 --- a/agent-feedback/items/2026-08-25-evo-react-lint-baseline.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -type: dx -impact: med -effort: med -site: packages/evo-react/package.json › scripts.lint ---- - -# Restore a clean Evo React lint baseline - -The package lint command fails in existing alert-dialog and confirm-dialog files on `autoFocus` and `closedby`, in button and icon-button stories and tests on empty custom anchors, and in the generated icon story on `@ts-nocheck`. These unrelated failures prevent component migrations from using the workspace lint command as a merge gate. Fix the source violations or narrow generated-file linting with an intentional repository rule. - -Check: Run `npm run lint -w packages/evo-react` and observe failures outside `src/combobox/`. diff --git a/packages/evo-react/src/combobox/combobox-option.tsx b/packages/evo-react/src/combobox/combobox-option.tsx index 57a9a6740ad..43d0b4e5199 100644 --- a/packages/evo-react/src/combobox/combobox-option.tsx +++ b/packages/evo-react/src/combobox/combobox-option.tsx @@ -1,4 +1,4 @@ -import { useEffect, useId } from "react"; +import { useId } from "react"; import type { Ref, RefObject } from "react"; import classNames from "classnames"; import { useActiveDescendantItem } from "../utils/use-active-descendant"; @@ -27,6 +27,7 @@ export function EvoComboboxOption({ displayedValue, filterValue, selectOption, + disabled, } = useComboboxContext(); const [optionRef, internalRef] = useRefTee( ref as Ref, @@ -45,12 +46,6 @@ export function EvoComboboxOption({ }, }); - useEffect(() => { - if (isActive) { - internalRef.current?.scrollIntoView({ block: "nearest" }); - } - }, [internalRef, isActive]); - if (hidden) { return null; } @@ -70,15 +65,25 @@ export function EvoComboboxOption({ )} onMouseDown={(event) => { event.preventDefault(); - onMouseDown?.(event); + if (!disabled) { + onMouseDown?.(event); + } }} onClick={(event) => { + if (disabled) { + return; + } + onClick?.(event); if (!event.defaultPrevented) { selectOption(text); } }} onKeyDown={(event) => { + if (disabled) { + return; + } + onKeyDown?.(event); if ( !event.defaultPrevented && diff --git a/packages/evo-react/src/combobox/combobox.stories.tsx b/packages/evo-react/src/combobox/combobox.stories.tsx index cac6192822d..26cb27a7d22 100644 --- a/packages/evo-react/src/combobox/combobox.stories.tsx +++ b/packages/evo-react/src/combobox/combobox.stories.tsx @@ -1,6 +1,6 @@ +import { useState } from "react"; import type { Meta, StoryObj } from "@storybook/react-vite"; -import { action } from "storybook/actions"; -import { EvoIconClose12 } from "../icon/icons/close-12"; +import { EvoIconClear16 } from "../icon/icons/clear-16"; import { EvoCombobox } from "./combobox"; import { EvoComboboxOption } from "./combobox-option"; @@ -75,13 +75,6 @@ import { autocomplete: "none", listSelection: "automatic", placeholder: "Choose a campaign", - postfix: { - icon: , - buttonProps: { - a11yText: "Clear", - onClick: action("clear"), - }, - }, }, }; @@ -98,3 +91,43 @@ export const Default: Story = { ), }; + +export const Controlled: Story = { + args: { + value: "August Campaign", + }, + render: (args) => ( + + + + + + + ), +}; + +export const Postfix: Story = { + render: (args) => { + const [value, setValue] = useState("August Campaign"); + + return ( + , + buttonProps: { + a11yText: "Clear", + onClick: () => setValue(""), + }, + }} + > + + + + + + ); + }, +}; diff --git a/packages/evo-react/src/combobox/combobox.tsx b/packages/evo-react/src/combobox/combobox.tsx index a0fa133ffa1..1da46e8777d 100644 --- a/packages/evo-react/src/combobox/combobox.tsx +++ b/packages/evo-react/src/combobox/combobox.tsx @@ -1,4 +1,4 @@ -import { useCallback, useId, useState } from "react"; +import { useCallback, useEffect, useId, useState } from "react"; import type { ChangeEvent, FocusEvent, @@ -16,6 +16,16 @@ import { ComboboxProvider } from "./context"; import type { EvoComboboxProps } from "./types"; import "@ebay/skin/combobox.mjs"; +type TemporaryValue = { + key: string; + origin: string; + value: string; +}; + +const activeDescendantScrollIntoView: ScrollIntoViewOptions = { + block: "nearest", +}; + export function EvoCombobox({ autocomplete = "none", borderless = false, @@ -49,23 +59,21 @@ export function EvoCombobox({ const listboxId = `${inputId}-listbox`; const isControlled = value !== undefined; const [uncontrolledValue, setUncontrolledValue] = useState(defaultValue); - const [temporaryValue, setTemporaryValue] = useState(null); + const [temporaryValue, setTemporaryValue] = useState( + null, + ); const [focused, setFocused] = useState(false); const currentValue = isControlled ? value : uncontrolledValue; - - if (listSelection === "manual" && temporaryValue !== null) { - setTemporaryValue(null); - } - - const displayedValue = temporaryValue ?? currentValue; const expander = useExpander({ defaultOpen, offset: 4, onOpenChange, open, placement: "bottom-start", + resetOnDisabled: Boolean(disabled), strategy, }); + const effectiveOpen = !disabled && expander.open; const [setListboxElement, listboxRef] = useRefTee( expander.refs.setFloating, null, @@ -76,12 +84,37 @@ export function EvoCombobox({ ); const activeDescendant = useActiveDescendant({ containerRef: listboxRef as RefObject, + scrollIntoView: activeDescendantScrollIntoView, shouldWrap: true, }); + const activeOption = effectiveOpen + ? activeDescendant.getActiveItem() + : undefined; + const hasValidTemporaryValue = + temporaryValue !== null && + listSelection === "automatic" && + temporaryValue.origin === currentValue && + temporaryValue.key === activeOption?.key; + + if (temporaryValue !== null && !hasValidTemporaryValue) { + setTemporaryValue(null); + } + + const displayedValue = hasValidTemporaryValue + ? temporaryValue.value + : currentValue; + const previewValue = hasValidTemporaryValue ? temporaryValue.value : null; + + useEffect(() => { + if (disabled) { + activeDescendant.reset(); + } + }, [activeDescendant.reset, disabled]); + const floatingLabel = useFloatingLabel({ containerTagName: fluid ? "div" : "span", disabled, - focused: focused || expander.open, + focused: focused || effectiveOpen, text: floatingLabelText, value: displayedValue, }); @@ -96,11 +129,15 @@ export function EvoCombobox({ const requestOpen = useCallback( (nextOpen: boolean) => { + if (disabled && nextOpen) { + return; + } + if (nextOpen !== expander.open) { expander.setOpen(nextOpen); } }, - [expander.open, expander.setOpen], + [disabled, expander.open, expander.setOpen], ); const updateValue = useCallback( @@ -118,89 +155,163 @@ export function EvoCombobox({ const selectOption = useCallback( (text: string) => { + if (disabled) { + return; + } + updateValue(text); activeDescendant.reset(); requestOpen(false); }, - [activeDescendant.reset, requestOpen, updateValue], + [activeDescendant.reset, disabled, requestOpen, updateValue], ); - const handleFocusOut = (event: FocusEvent) => { - if (event.currentTarget.contains(event.relatedTarget as Node | null)) { - return; - } + const handleFocusOut = useCallback( + (event: FocusEvent) => { + if (event.currentTarget.contains(event.relatedTarget as Node | null)) { + return; + } - setFocused(false); - const preview = temporaryValue; - requestOpen(false); - activeDescendant.reset(); + setFocused(false); + requestOpen(false); + activeDescendant.reset(); - if (preview === null) { - return; - } + if (previewValue !== null) { + updateValue(previewValue); + } + }, + [activeDescendant.reset, previewValue, requestOpen, updateValue], + ); - updateValue(preview); - }; + const handleChange = useCallback( + (event: ChangeEvent) => { + if (disabled) { + return; + } - const handleChange = (event: ChangeEvent) => { - activeDescendant.reset(); - updateValue(event.currentTarget.value); - requestOpen(true); - onChange?.(event); - }; + activeDescendant.reset(); + updateValue(event.currentTarget.value); + requestOpen(true); + onChange?.(event); + }, + [activeDescendant.reset, disabled, onChange, requestOpen, updateValue], + ); - const handleFocus = (event: FocusEvent) => { - setFocused(true); - requestOpen(true); - onFocus?.(event); - }; + const handleFocus = useCallback( + (event: FocusEvent) => { + if (disabled) { + return; + } - const handleBlur = (event: FocusEvent) => { - onBlur?.(event); - }; + setFocused(true); + requestOpen(true); + onFocus?.(event); + }, + [disabled, onFocus, requestOpen], + ); - const handleClick = (event: MouseEvent) => { - requestOpen(true); - onClick?.(event); - }; + const handleBlur = useCallback( + (event: FocusEvent) => { + onBlur?.(event); + }, + [onBlur], + ); - const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === "ArrowDown" || event.key === "ArrowUp") { - event.preventDefault(); - if (!expander.open) { - requestOpen(true); - } else { - const option = - event.key === "ArrowDown" - ? activeDescendant.activateNext() - : activeDescendant.activatePrevious(); - if (listSelection === "automatic") { - setTemporaryValue(option?.data ?? null); - } + const handleClick = useCallback( + (event: MouseEvent) => { + if (disabled) { + return; } - } else if (event.key === "Enter" && expander.open) { - const option = activeDescendant.getActiveItem(); - if (option) { + + requestOpen(true); + onClick?.(event); + }, + [disabled, onClick, requestOpen], + ); + + const handleKeyDown = useCallback( + (event: KeyboardEvent) => { + if (disabled) { + return; + } + + if (event.key === "ArrowDown" || event.key === "ArrowUp") { event.preventDefault(); - selectOption(option.data); - } else { + if (!effectiveOpen) { + requestOpen(true); + } else { + const option = + event.key === "ArrowDown" + ? activeDescendant.activateNext() + : activeDescendant.activatePrevious(); + if (listSelection === "automatic") { + setTemporaryValue( + option + ? { + key: option.key, + origin: currentValue, + value: option.data, + } + : null, + ); + } + } + } else if (event.key === "Enter" && effectiveOpen) { + const option = activeDescendant.getActiveItem(); + if (option) { + event.preventDefault(); + selectOption(option.data); + } else { + requestOpen(false); + } + } else if (event.key === "Escape") { + setTemporaryValue(null); + activeDescendant.reset(); requestOpen(false); } - } else if (event.key === "Escape") { - setTemporaryValue(null); - activeDescendant.reset(); - requestOpen(false); - } - onKeyDown?.(event); - }; + onKeyDown?.(event); + }, + [ + activeDescendant.activateNext, + activeDescendant.activatePrevious, + activeDescendant.getActiveItem, + activeDescendant.reset, + currentValue, + disabled, + effectiveOpen, + listSelection, + onKeyDown, + requestOpen, + selectOption, + ], + ); const Wrapper = fluid ? "div" : "span"; - const activeOption = expander.open - ? activeDescendant.getActiveItem() - : undefined; const postfixButtonProps = postfix?.buttonProps; const postfixMouseDown = postfixButtonProps?.onMouseDown; + const postfixClick = postfixButtonProps?.onClick; + const handlePostfixClick = useCallback( + (event: MouseEvent) => { + if (disabled) { + return; + } + + setTemporaryValue(null); + activeDescendant.reset(); + postfixClick?.(event); + }, + [activeDescendant.reset, disabled, postfixClick], + ); + const handlePostfixMouseDown = useCallback( + (event: MouseEvent) => { + event.preventDefault(); + if (!disabled) { + postfixMouseDown?.(event); + } + }, + [disabled, postfixMouseDown], + ); return ( @@ -216,11 +328,11 @@ export function EvoCombobox({ className={classNames( "combobox", fluid && "combobox--fluid", - expander.open && "combobox--expanded", + effectiveOpen && "combobox--expanded", className, )} style={style} - onBlur={handleFocusOut} + onBlurCapture={handleFocusOut} > { - event.preventDefault(); - postfixMouseDown?.(event); - }} + onClick={handlePostfixClick} + onMouseDown={handlePostfixMouseDown} > {postfix.icon} @@ -273,7 +383,11 @@ export function EvoCombobox({ id={listboxId} ref={setListboxElement} role="listbox" - className="combobox__listbox" + className={classNames( + "combobox__listbox", + "combobox__listbox--set-position", + strategy === "fixed" && "combobox__listbox--fixed", + )} style={expander.floatingStyles} > {children} diff --git a/packages/evo-react/src/combobox/context.tsx b/packages/evo-react/src/combobox/context.tsx index 83d37f0e74c..94a07ecf14a 100644 --- a/packages/evo-react/src/combobox/context.tsx +++ b/packages/evo-react/src/combobox/context.tsx @@ -8,6 +8,7 @@ export type ComboboxContextValue = { autocomplete: ComboboxAutocomplete; displayedValue: string; filterValue: string; + disabled?: boolean; selectOption: (text: string) => void; }; @@ -34,6 +35,7 @@ export function ComboboxProvider({ autocomplete, displayedValue, filterValue, + disabled, selectOption, children, }: ComboboxProviderProps) { @@ -43,9 +45,17 @@ export function ComboboxProvider({ autocomplete, displayedValue, filterValue, + disabled, selectOption, }), - [activeDescendant, autocomplete, displayedValue, filterValue, selectOption], + [ + activeDescendant, + autocomplete, + disabled, + displayedValue, + filterValue, + selectOption, + ], ); return {children}; diff --git a/packages/evo-react/src/combobox/test/__snapshots__/test.server.tsx.snap b/packages/evo-react/src/combobox/test/__snapshots__/test.server.tsx.snap index 467c1c0b2f6..192ad2859f2 100644 --- a/packages/evo-react/src/combobox/test/__snapshots__/test.server.tsx.snap +++ b/packages/evo-react/src/combobox/test/__snapshots__/test.server.tsx.snap @@ -1,9 +1,13 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`EvoCombobox SSR > renders a fluid combobox with a floating label 1`] = `"
August Campaign
Basic Offer
"`; +exports[`EvoCombobox SSR > renders a disabled combobox as closed 1`] = `""`; -exports[`EvoCombobox SSR > renders an actionable postfix 1`] = `"
August Campaign
Basic Offer
"`; +exports[`EvoCombobox SSR > renders a fixed-strategy listbox 1`] = `""`; -exports[`EvoCombobox SSR > renders an open borderless combobox 1`] = `""`; +exports[`EvoCombobox SSR > renders a fluid combobox with a floating label 1`] = `"
August Campaign
Basic Offer
"`; -exports[`EvoCombobox SSR > renders the default combobox 1`] = `"
August Campaign
Basic Offer
"`; +exports[`EvoCombobox SSR > renders an actionable postfix 1`] = `"
August Campaign
Basic Offer
"`; + +exports[`EvoCombobox SSR > renders an open borderless combobox 1`] = `""`; + +exports[`EvoCombobox SSR > renders the default combobox 1`] = `"
August Campaign
Basic Offer
"`; diff --git a/packages/evo-react/src/combobox/test/test.browser.tsx b/packages/evo-react/src/combobox/test/test.browser.tsx index b3266440b0a..87b2d404b46 100644 --- a/packages/evo-react/src/combobox/test/test.browser.tsx +++ b/packages/evo-react/src/combobox/test/test.browser.tsx @@ -1,4 +1,4 @@ -import { createRef, useState } from "react"; +import { createRef, useRef, useState } from "react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { userEvent } from "vitest/browser"; import { render } from "vitest-browser-react"; @@ -25,6 +25,7 @@ describe("evo-combobox", () => { afterEach(() => { user.cleanup(); + vi.restoreAllMocks(); }); describe("ARIA attributes", () => { @@ -57,6 +58,22 @@ describe("evo-combobox", () => { .toHaveAttribute("aria-activedescendant", option.element().id); await expect.element(option).toHaveAttribute("aria-selected", "true"); }); + + it("scrolls each newly active option without scrolling on reset", async () => { + const scrollIntoView = vi.spyOn(HTMLElement.prototype, "scrollIntoView"); + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + await user.keyboard("{ArrowDown}"); + + expect(scrollIntoView).toHaveBeenCalledTimes(2); + expect(scrollIntoView).toHaveBeenLastCalledWith({ block: "nearest" }); + + await user.keyboard("{Escape}"); + expect(scrollIntoView).toHaveBeenCalledTimes(2); + }); }); describe("value changes", () => { @@ -116,6 +133,58 @@ describe("evo-combobox", () => { await user.click(screen.getByRole("option", { name: "Basic Offer" })); await expect.element(input).toHaveValue("Basic Offer"); }); + + it("does not let a controlled update leave a stale keyboard preview", async () => { + function ControlledCombobox() { + const [value, setValue] = useState(""); + return ( + { + if (event.key === "ArrowDown") { + setValue("Externally changed"); + } + }} + /> + ); + } + + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + + await expect.element(input).toHaveValue("Externally changed"); + }); + + it("does not let an active option removal leave a stale preview", async () => { + function FilteredCombobox() { + const [showFirst, setShowFirst] = useState(true); + return ( + { + if (event.key === "ArrowDown") { + setShowFirst(false); + } + }} + > + {showFirst && } + + + ); + } + + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + + await expect.element(input).toHaveValue(""); + }); }); describe("filtering", () => { @@ -197,6 +266,213 @@ describe("evo-combobox", () => { await expect.element(input).toHaveValue("Initial value"); await expect.element(input).toHaveAttribute("aria-expanded", "false"); }); + + it("clears a preview when list selection changes to manual", async () => { + function ChangingSelectionCombobox() { + const [selection, setSelection] = + useState("automatic"); + const arrowPresses = useRef(0); + + return ( + { + if (event.key === "ArrowDown" && ++arrowPresses.current === 2) { + setSelection("manual"); + } + }} + > + + + + ); + } + + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + await expect.element(input).toHaveValue("August Campaign"); + await user.keyboard("{ArrowDown}"); + + await expect.element(input).toHaveValue(""); + }); + + it("commits preview before native onBlur and allows restoration", async () => { + const onValueChange = vi.fn(); + + function ControlledCombobox() { + const [value, setValue] = useState("Initial value"); + return ( + { + onValueChange(nextValue); + setValue(nextValue); + }} + onBlur={() => setValue("Restored value")} + /> + ); + } + + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + await user.tab(); + + await expect.element(input).toHaveValue("Restored value"); + expect(onValueChange).toHaveBeenCalledTimes(1); + expect(onValueChange).toHaveBeenCalledWith("August Campaign"); + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + }); + }); + + describe("disabled state", () => { + it("keeps a default-open combobox closed when disabled", async () => { + const screen = await render(); + const input = screen.getByRole("combobox"); + const wrapper = screen.container.querySelector(".combobox"); + + await expect.element(input).toBeDisabled(); + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + expect(wrapper).not.toHaveClass("combobox--expanded"); + }); + + it("does not expose a controlled open state while disabled", async () => { + const screen = await render( + , + ); + const input = screen.getByRole("combobox"); + const wrapper = screen.container.querySelector(".combobox"); + + await expect.element(input).toBeDisabled(); + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + expect(wrapper).not.toHaveClass("combobox--expanded"); + }); + + it("does not open from focus, click, or keyboard input while disabled", async () => { + const onOpenChange = vi.fn(); + const onFocus = vi.fn(); + const screen = await render( + , + ); + const input = screen.getByRole("combobox"); + + input.element().dispatchEvent(new FocusEvent("focus", { bubbles: true })); + input.element().dispatchEvent(new MouseEvent("click", { bubbles: true })); + input + .element() + .dispatchEvent( + new KeyboardEvent("keydown", { bubbles: true, key: "ArrowDown" }), + ); + + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + expect(onOpenChange).not.toHaveBeenCalled(); + expect(onFocus).not.toHaveBeenCalled(); + }); + + it("clears active state and stays closed after an uncontrolled combobox is re-enabled", async () => { + function ToggleableCombobox() { + const [disabled, setDisabled] = useState(false); + return ( + <> + + + + ); + } + + const screen = await render(); + const input = screen.getByRole("combobox"); + const toggle = screen.getByRole("button", { name: "Toggle disabled" }); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + await expect.element(input).toHaveAttribute("aria-activedescendant"); + + await user.click(toggle); + await expect.element(input).toBeDisabled(); + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + await expect.element(input).not.toHaveAttribute("aria-activedescendant"); + + await user.click(toggle); + await expect.element(input).not.toBeDisabled(); + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + await expect.element(input).not.toHaveAttribute("aria-activedescendant"); + }); + + it("follows a controlled open state after being re-enabled", async () => { + function ToggleableCombobox() { + const [disabled, setDisabled] = useState(true); + return ( + <> + + + + ); + } + + const screen = await render(); + const input = screen.getByRole("combobox"); + const toggle = screen.getByRole("button", { name: "Toggle disabled" }); + + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + await user.click(toggle); + await expect.element(input).toHaveAttribute("aria-expanded", "true"); + }); + + it("does not invoke disabled option handlers or select an option", async () => { + const onClick = vi.fn(); + const onKeyDown = vi.fn(); + const onMouseDown = vi.fn(); + const onValueChange = vi.fn(); + const screen = await render( + + + , + ); + const option = + screen.container.querySelector('[role="option"]'); + + option?.dispatchEvent(new MouseEvent("mousedown", { bubbles: true })); + option?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + option?.dispatchEvent( + new KeyboardEvent("keydown", { bubbles: true, key: "Enter" }), + ); + + expect(onClick).not.toHaveBeenCalled(); + expect(onKeyDown).not.toHaveBeenCalled(); + expect(onMouseDown).not.toHaveBeenCalled(); + expect(onValueChange).not.toHaveBeenCalled(); + }); }); describe("postfix action", () => { @@ -219,6 +495,96 @@ describe("evo-combobox", () => { await expect.element(input).toHaveFocus(); await expect.element(input).toHaveAttribute("aria-expanded", "true"); }); + + it("does not open or focus the input from a closed postfix action", async () => { + const onClick = vi.fn(); + const screen = await render( + , + buttonProps: { a11yText: "Action", onClick }, + }} + />, + ); + const input = screen.getByRole("combobox"); + + await user.click(screen.getByRole("button", { name: "Action" })); + + expect(onClick).toHaveBeenCalledTimes(1); + await expect.element(input).not.toHaveFocus(); + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + }); + + it("keeps keyboard focus on the postfix after activation", async () => { + const screen = await render( + , + buttonProps: { a11yText: "Action" }, + }} + />, + ); + const button = screen.getByRole("button", { name: "Action" }); + const input = screen.getByRole("combobox"); + + button.element().focus(); + await user.keyboard("{Enter}"); + + await expect.element(button).toHaveFocus(); + await expect.element(input).toHaveAttribute("aria-expanded", "false"); + }); + + it("clears keyboard preview before a controlled clear callback", async () => { + function ControlledCombobox() { + const [value, setValue] = useState(""); + return ( + , + buttonProps: { + a11yText: "Clear", + onClick: () => setValue(""), + }, + }} + /> + ); + } + + const screen = await render(); + const input = screen.getByRole("combobox"); + + await user.click(input); + await user.keyboard("{ArrowDown}"); + await expect.element(input).toHaveValue("August Campaign"); + + await user.click(screen.getByRole("button", { name: "Clear" })); + + await expect.element(input).toHaveValue(""); + await expect.element(input).toHaveFocus(); + }); + + it("does not run a disabled postfix callback", async () => { + const onClick = vi.fn(); + const screen = await render( + , + buttonProps: { a11yText: "Clear", onClick }, + }} + />, + ); + + const button = screen.getByRole("button", { name: "Clear" }); + await expect.element(button).toBeDisabled(); + button + .element() + .dispatchEvent(new MouseEvent("click", { bubbles: true })); + + expect(onClick).not.toHaveBeenCalled(); + }); }); describe("refs", () => { @@ -231,4 +597,23 @@ describe("evo-combobox", () => { expect(ref.current?.getAttribute("role")).toBe("combobox"); }); }); + + describe("listbox layout", () => { + it("makes the open listbox at least as wide as the combobox", async () => { + const screen = await render( +
+ +
, + ); + const input = screen.getByRole("combobox"); + + await user.click(input); + + const listbox = screen.getByRole("listbox").element(); + const wrapper = screen.container.querySelector(".combobox"); + expect(listbox.getBoundingClientRect().width).toBeGreaterThanOrEqual( + (wrapper?.getBoundingClientRect().width ?? 0) - 1, + ); + }); + }); }); diff --git a/packages/evo-react/src/combobox/test/test.server.tsx b/packages/evo-react/src/combobox/test/test.server.tsx index 41ce0b14308..cc594faac33 100644 --- a/packages/evo-react/src/combobox/test/test.server.tsx +++ b/packages/evo-react/src/combobox/test/test.server.tsx @@ -34,6 +34,18 @@ describe("EvoCombobox SSR", () => { ).toMatchSnapshot(); }); + it("renders a fixed-strategy listbox", () => { + expect( + renderCombobox({ defaultOpen: true, strategy: "fixed" }), + ).toMatchSnapshot(); + }); + + it("renders a disabled combobox as closed", () => { + expect( + renderCombobox({ defaultOpen: true, disabled: true }), + ).toMatchSnapshot(); + }); + it("renders an actionable postfix", () => { expect( renderCombobox({ diff --git a/packages/evo-react/src/utils/use-active-descendant.ts b/packages/evo-react/src/utils/use-active-descendant.ts index 991cae35cd2..b39025bb21b 100644 --- a/packages/evo-react/src/utils/use-active-descendant.ts +++ b/packages/evo-react/src/utils/use-active-descendant.ts @@ -1,4 +1,4 @@ -import { useCallback, useLayoutEffect, useRef, useState } from "react"; +import { useCallback, useLayoutEffect, useMemo, useRef, useState } from "react"; import type { RefObject } from "react"; export type ActiveDescendantItem = { @@ -20,6 +20,7 @@ export type ActiveDescendant = { type UseActiveDescendantOptions = { containerRef: RefObject; shouldWrap?: boolean; + scrollIntoView?: ScrollIntoViewOptions; }; type UseActiveDescendantItemOptions = { @@ -47,6 +48,7 @@ function compareDomOrder( export function useActiveDescendant({ containerRef, + scrollIntoView, shouldWrap = true, }: UseActiveDescendantOptions): ActiveDescendant { const itemsRef = useRef(new Map>()); @@ -74,21 +76,27 @@ export function useActiveDescendant({ return getItems().find((item) => item.key === key); }, [getItems]); - const activateKey = useCallback((key: Key | null) => { - if (key === null) { - activeKeyRef.current = null; - setActiveKeyState(null); - return; - } + const activateKey = useCallback( + (key: Key | null) => { + if (key === null) { + activeKeyRef.current = null; + setActiveKeyState(null); + return; + } - const item = itemsRef.current.get(key); - if (!item || !item.ref.current) { - return; - } + const item = itemsRef.current.get(key); + if (!item || !item.ref.current) { + return; + } - activeKeyRef.current = key; - setActiveKeyState(key); - }, []); + activeKeyRef.current = key; + setActiveKeyState(key); + if (scrollIntoView) { + item.ref.current.scrollIntoView?.(scrollIntoView); + } + }, + [scrollIntoView], + ); const reset = useCallback(() => { activateKey(null); @@ -151,14 +159,24 @@ export function useActiveDescendant({ const activateNext = useCallback(() => move(1), [move]); const activatePrevious = useCallback(() => move(-1), [move]); - return { - activeKey, - registerItem, - getActiveItem, - activateNext, - activatePrevious, - reset, - }; + return useMemo( + () => ({ + activeKey, + registerItem, + getActiveItem, + activateNext, + activatePrevious, + reset, + }), + [ + activeKey, + activateNext, + activatePrevious, + getActiveItem, + registerItem, + reset, + ], + ); } export function useActiveDescendantItem({ diff --git a/packages/evo-react/src/utils/use-expander.ts b/packages/evo-react/src/utils/use-expander.ts index e849d093a37..212d0511afe 100644 --- a/packages/evo-react/src/utils/use-expander.ts +++ b/packages/evo-react/src/utils/use-expander.ts @@ -8,7 +8,7 @@ import { useFloating, } from "@floating-ui/react"; import type { Middleware, Placement, Strategy } from "@floating-ui/react"; -import { useCallback, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import type { CSSProperties } from "react"; type UseExpanderOptions = { @@ -21,6 +21,8 @@ type UseExpanderOptions = { flip?: boolean; shift?: boolean; inline?: boolean; + /** Closes uncontrolled state when the owning component is disabled. */ + resetOnDisabled?: boolean; }; export function useExpander({ @@ -33,12 +35,19 @@ export function useExpander({ flip = true, shift = true, inline = true, + resetOnDisabled = false, }: UseExpanderOptions = {}) { const isControlled = open !== undefined; const [uncontrolledOpen, setUncontrolledOpen] = useState(defaultOpen); const currentOpen = isControlled ? open : uncontrolledOpen; const arrowRef = useRef(null); + useEffect(() => { + if (resetOnDisabled && !isControlled) { + setUncontrolledOpen(false); + } + }, [isControlled, resetOnDisabled]); + const setOpen = useCallback( (nextOpen: boolean) => { if (!isControlled) {