Skip to content

Commit 7ad22a7

Browse files
jaymantricursoragent
authored andcommitted
[origin] Default combobox clear to active (#28544)
## Reason [DES-36](https://lightspark.atlassian.net/browse/DES-36) needs edit-existing forms to avoid showing a noisy clear affordance for saved country/nationality values until the user is actually interacting with the field. Origin now makes that active behavior the default so consumers get the quieter edit-existing state without product-level props. ## Overview Adds `visibility` to `Combobox.Clear` with `active` and `always` modes while preserving Base UI's ownership of clear behavior and clearable state. `active` is the new default: the clear affordance is hidden at rest and appears while the field is focused or open. This intentionally changes existing `<Combobox.Clear />` consumers from "always visible when clearable" to "visible while active when clearable." Consumers that need the previous/create-flow behavior can opt into `visibility="always"`. Consumers that should not expose a clear affordance should omit `<Combobox.Clear />`. ## QA Notes Existing Combobox.Clear consumers should be checked with the new default in mind: - Edit-existing surfaces should no longer show the clear icon at rest when they load with saved values. - Focusing/opening the combobox should reveal the clear affordance when Base UI reports a clearable value. - Create/select flows that want the clear icon visible while a value exists should use `visibility="always"`. - Flows that should not offer clearing should omit `<Combobox.Clear />`. ## Storybook preview - Components/Combobox / Default: https://dev.dev.sparkinfra.net/app/origin-storybook-pr-28544/?path=/story/components-combobox--default - Components/Combobox / Default Active Clear: https://dev.dev.sparkinfra.net/app/origin-storybook-pr-28544/?path=/story/components-combobox--with-clear - Components/Combobox / Always Clear: https://dev.dev.sparkinfra.net/app/origin-storybook-pr-28544/?path=/story/components-combobox--always-clear ## Test Plan - `mise exec -- node -v` -> `v20.19.6` - `mise exec -- corepack yarn --version` -> `4.13.0` - `mise exec -- corepack yarn workspace @lightsparkdev/origin playwright test -c playwright-ct.config.ts src/components/Combobox/Combobox.test.tsx` - `mise exec -- corepack yarn workspace @lightsparkdev/origin types` - `mise exec -- corepack yarn workspace @lightsparkdev/origin lint` (passes with existing warnings in DatePicker/Sidebar) - `mise exec -- corepack yarn workspace @lightsparkdev/origin format` [DES-36]: https://lightspark.atlassian.net/browse/DES-36?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Cursor <cursoragent@cursor.com> GitOrigin-RevId: 9423d49750e801bb7e60f4a26b75457040dd226d
1 parent f7e42c2 commit 7ad22a7

9 files changed

Lines changed: 231 additions & 23 deletions

File tree

packages/origin/src/components/Combobox/Combobox.module.scss

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@
2424
padding-left: var(--spacing-2xs);
2525
}
2626

27-
&:has(.clear:not([hidden])) {
27+
&:has(.clearAlways:not([hidden])),
28+
&:focus-within:has(.clearActive:not([hidden])),
29+
&[data-focused]:has(.clearActive:not([hidden])),
30+
&[data-popup-open]:has(.clearActive:not([hidden])) {
2831
padding-right: calc(var(--spacing-xs) + 42px);
2932
}
3033

@@ -129,7 +132,6 @@
129132

130133
.clear {
131134
box-sizing: border-box;
132-
display: flex;
133135
align-items: center;
134136
justify-content: center;
135137
flex-shrink: 0;
@@ -157,6 +159,20 @@
157159
}
158160
}
159161

162+
.clearActive {
163+
display: none;
164+
}
165+
166+
.inputWrapper:focus-within .clearActive:not([hidden]),
167+
.inputWrapper[data-focused] .clearActive:not([hidden]),
168+
.inputWrapper[data-popup-open] .clearActive:not([hidden]) {
169+
display: flex;
170+
}
171+
172+
.clearAlways:not([hidden]) {
173+
display: flex;
174+
}
175+
160176
.clear svg {
161177
width: 17px;
162178
height: 17px;

packages/origin/src/components/Combobox/Combobox.stories.tsx

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Meta, StoryObj } from "@storybook/react";
22
import { useState } from "react";
3-
import { Combobox } from "./index";
3+
import { Combobox, type ComboboxClearProps } from "./index";
44
import { Field } from "@/components/Field";
55

66
const meta: Meta<typeof Combobox.Root> = {
@@ -36,6 +36,14 @@ export const Default: Story = {
3636
args: {
3737
disabled: false,
3838
},
39+
parameters: {
40+
docs: {
41+
description: {
42+
story:
43+
"No-clear reference. Omit Combobox.Clear when a flow should not expose a clear affordance.",
44+
},
45+
},
46+
},
3947
render: (args) => (
4048
<Combobox.Root items={fruits} {...args}>
4149
<Combobox.InputWrapper>
@@ -91,13 +99,22 @@ export const LongList: Story = {
9199
),
92100
};
93101

94-
export const WithClear: Story = {
95-
render: () => (
102+
function ClearStory({
103+
visibility,
104+
label,
105+
}: {
106+
visibility?: ComboboxClearProps["visibility"];
107+
label: string;
108+
}) {
109+
return (
96110
<Combobox.Root items={fruits} defaultValue="Apple">
97111
<Combobox.InputWrapper>
98-
<Combobox.Input placeholder="Select a fruit..." />
112+
<Combobox.Input placeholder={label} />
99113
<Combobox.ActionButtons>
100-
<Combobox.Clear aria-label="Clear selection" />
114+
<Combobox.Clear
115+
aria-label="Clear selection"
116+
visibility={visibility}
117+
/>
101118
<Combobox.Trigger aria-label="Open popup" />
102119
</Combobox.ActionButtons>
103120
</Combobox.InputWrapper>
@@ -117,7 +134,32 @@ export const WithClear: Story = {
117134
</Combobox.Positioner>
118135
</Combobox.Portal>
119136
</Combobox.Root>
120-
),
137+
);
138+
}
139+
140+
export const WithClear: Story = {
141+
name: "Default Active Clear",
142+
parameters: {
143+
docs: {
144+
description: {
145+
story:
146+
"Default edit-existing flow. The clear affordance is hidden at rest, appears while the field is focused or open, and is removed by Base UI once the value is cleared.",
147+
},
148+
},
149+
},
150+
render: () => <ClearStory label="Edit saved fruit..." />,
151+
};
152+
153+
export const AlwaysClear: Story = {
154+
parameters: {
155+
docs: {
156+
description: {
157+
story:
158+
"Opt in when a clear affordance should remain visible while the value is clearable. Base UI still removes it once the value is cleared.",
159+
},
160+
},
161+
},
162+
render: () => <ClearStory label="Select a fruit..." visibility="always" />,
121163
};
122164

123165
function MultipleField({

packages/origin/src/components/Combobox/Combobox.test-stories.tsx

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as React from "react";
22
import { useState } from "react";
3-
import { Combobox } from "./index";
3+
import { Combobox, type ComboboxClearProps } from "./index";
44

55
const fruits = [
66
"Apple",
@@ -307,12 +307,22 @@ export const TestComboboxWithGroups = () => (
307307
</Combobox.Root>
308308
);
309309

310-
export const TestComboboxWithClear = () => (
310+
export const TestComboboxWithClear = ({
311+
clearClassName,
312+
visibility,
313+
}: {
314+
clearClassName?: ComboboxClearProps["className"];
315+
visibility?: ComboboxClearProps["visibility"];
316+
} = {}) => (
311317
<Combobox.Root items={fruits} defaultValue="Apple">
312-
<Combobox.InputWrapper>
318+
<Combobox.InputWrapper data-testid="combobox-clear-wrapper">
313319
<Combobox.Input placeholder="Select a fruit..." />
314320
<Combobox.ActionButtons>
315-
<Combobox.Clear aria-label="Clear selection" />
321+
<Combobox.Clear
322+
aria-label="Clear selection"
323+
className={clearClassName}
324+
visibility={visibility}
325+
/>
316326
<Combobox.Trigger aria-label="Open popup" />
317327
</Combobox.ActionButtons>
318328
</Combobox.InputWrapper>
@@ -333,3 +343,10 @@ export const TestComboboxWithClear = () => (
333343
</Combobox.Portal>
334344
</Combobox.Root>
335345
);
346+
347+
export const TestComboboxWithClearClassNameCallback = () => (
348+
<TestComboboxWithClear
349+
clearClassName={(state) => (state.open ? "clear-open" : "clear-closed")}
350+
visibility="always"
351+
/>
352+
);

packages/origin/src/components/Combobox/Combobox.test.tsx

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
TestComboboxControlled,
99
TestComboboxWithGroups,
1010
TestComboboxWithClear,
11+
TestComboboxWithClearClassNameCallback,
1112
TestComboboxChipPassThrough,
1213
ConformanceInputWrapper,
1314
ConformanceActionButtons,
@@ -270,14 +271,103 @@ test.describe("Combobox", () => {
270271
});
271272

272273
test.describe("clear button", () => {
273-
test("clears selection on click", async ({ mount }) => {
274+
test("defaults to active visibility and clears selection on click", async ({
275+
mount,
276+
page,
277+
}) => {
274278
const component = await mount(<TestComboboxWithClear />);
275279
const input = component.getByPlaceholder("Select a fruit...");
276-
const clear = component.locator('button[class*="clear"]');
280+
const clear = component.getByRole("button", {
281+
name: "Clear selection",
282+
includeHidden: true,
283+
});
284+
285+
await expect(input).toHaveValue("Apple");
286+
await expect(clear).toHaveCSS("display", "none");
287+
288+
await input.click();
289+
await expect(page.getByRole("listbox")).toBeVisible();
290+
await expect(clear).toBeVisible();
291+
await clear.click();
292+
await expect(input).toHaveValue("");
293+
});
294+
295+
test("always clear remains visible while clearable", async ({ mount }) => {
296+
const component = await mount(
297+
<TestComboboxWithClear visibility="always" />,
298+
);
299+
const input = component.getByPlaceholder("Select a fruit...");
300+
const clear = component.getByRole("button", { name: "Clear selection" });
277301

278302
await expect(input).toHaveValue("Apple");
303+
await expect(clear).toBeVisible();
304+
await expect(clear).not.toHaveAttribute("visibility", "always");
305+
await expect(clear).not.toHaveAttribute(
306+
"data-clear-visibility",
307+
"always",
308+
);
279309
await clear.click();
280310
await expect(input).toHaveValue("");
311+
await expect(clear).toBeHidden();
312+
});
313+
314+
test("preserves Base UI stateful clear className callback", async ({
315+
mount,
316+
}) => {
317+
const component = await mount(<TestComboboxWithClearClassNameCallback />);
318+
const clear = component.getByRole("button", { name: "Clear selection" });
319+
320+
await expect(clear).toHaveClass(/clear-closed/);
321+
});
322+
323+
test("active clear is hidden at rest and appears while focused or open", async ({
324+
mount,
325+
page,
326+
}) => {
327+
const component = await mount(
328+
<TestComboboxWithClear visibility="active" />,
329+
);
330+
const input = component.getByPlaceholder("Select a fruit...");
331+
const wrapper = page.getByTestId("combobox-clear-wrapper");
332+
const clear = component.getByRole("button", {
333+
name: "Clear selection",
334+
includeHidden: true,
335+
});
336+
337+
const restingPadding = await wrapper.evaluate((element) =>
338+
Number.parseFloat(window.getComputedStyle(element).paddingRight),
339+
);
340+
341+
await expect(input).toHaveValue("Apple");
342+
await expect(clear).toHaveCSS("display", "none");
343+
await expect(
344+
component.getByRole("button", { name: "Clear selection" }),
345+
).toBeHidden();
346+
347+
await input.click();
348+
await expect(page.getByRole("listbox")).toBeVisible();
349+
await expect(clear).toBeVisible();
350+
351+
const activePadding = await wrapper.evaluate((element) =>
352+
Number.parseFloat(window.getComputedStyle(element).paddingRight),
353+
);
354+
expect(activePadding).toBeGreaterThan(restingPadding);
355+
356+
await clear.click();
357+
await expect(input).toHaveValue("");
358+
});
359+
360+
test("omitting Clear does not render the clear affordance", async ({
361+
mount,
362+
}) => {
363+
const component = await mount(<TestCombobox />);
364+
365+
await expect(
366+
component.getByRole("button", {
367+
name: "Clear selection",
368+
includeHidden: true,
369+
}),
370+
).toHaveCount(0);
281371
});
282372
});
283373

packages/origin/src/components/Combobox/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export type {
99
ActionButtonsProps as ComboboxActionButtonsProps,
1010
TriggerProps as ComboboxTriggerProps,
1111
ClearProps as ComboboxClearProps,
12+
ClearVisibility as ComboboxClearVisibility,
1213
PortalProps as ComboboxPortalProps,
1314
PositionerProps as ComboboxPositionerProps,
1415
PopupProps as ComboboxPopupProps,

packages/origin/src/components/Combobox/parts.tsx

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,22 +141,53 @@ export const Trigger = React.forwardRef<HTMLButtonElement, TriggerProps>(
141141
},
142142
);
143143

144-
export interface ClearProps extends BaseCombobox.Clear.Props {}
144+
export type ClearVisibility = "always" | "active";
145+
146+
export interface ClearProps
147+
extends Omit<BaseCombobox.Clear.Props, "keepMounted"> {
148+
/**
149+
* Controls when Origin shows the clear affordance. Base UI still owns the
150+
* clear behavior and whether a value is currently clearable. Defaults to
151+
* "active", which shows the affordance while the field is focused or open.
152+
*/
153+
visibility?: ClearVisibility;
154+
/**
155+
* Unsupported in Origin. Clear visibility relies on Base UI unmounting or
156+
* hiding the button when the value is not clearable.
157+
*/
158+
keepMounted?: never;
159+
}
145160

146161
/**
147162
* Combobox.Clear - Button to clear the selection.
148163
*
149164
* Renders as a small icon button with the X icon.
150-
* Uses Base UI's default behavior - only visible when there's a value to clear.
165+
* Uses Base UI's default behavior - only rendered when there's a value to clear.
151166
*/
152167
export const Clear = React.forwardRef<HTMLButtonElement, ClearProps>(
153-
function Clear({ className, children, ...props }, ref) {
168+
function Clear(
169+
{
170+
className,
171+
children,
172+
keepMounted: _keepMounted,
173+
visibility = "active",
174+
...props
175+
},
176+
ref,
177+
) {
178+
const originClassName = [
179+
styles.clear,
180+
visibility === "active" && styles.clearActive,
181+
visibility === "always" && styles.clearAlways,
182+
];
183+
const clearClassName =
184+
typeof className === "function"
185+
? (state: BaseCombobox.Clear.State) =>
186+
clsx(originClassName, className(state))
187+
: clsx(originClassName, className);
188+
154189
return (
155-
<BaseCombobox.Clear
156-
ref={ref}
157-
className={clsx(styles.clear, className)}
158-
{...props}
159-
>
190+
<BaseCombobox.Clear ref={ref} className={clearClassName} {...props}>
160191
{children ?? <CentralIcon name="IconCircleX" size={17} />}
161192
</BaseCombobox.Clear>
162193
);

packages/origin/src/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ export type {
3737
export { Checkbox } from "./components/Checkbox";
3838
export { Command } from "./components/Command";
3939
export { Combobox } from "./components/Combobox";
40+
export type {
41+
ComboboxClearProps,
42+
ComboboxClearVisibility,
43+
} from "./components/Combobox";
4044
export { ContextMenu } from "./components/ContextMenu";
4145
export { Dialog } from "./components/Dialog";
4246
export { Drawer, createHandle } from "./components/Drawer";

packages/origin/tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"@test-utils/*": ["./test-utils/*"]
1616
}
1717
},
18-
"include": ["src/**/*.ts", "src/**/*.tsx"],
18+
"include": ["src/**/*.ts", "src/**/*.tsx", "type-tests/**/*.tsx"],
1919
"exclude": [
2020
"node_modules",
2121
"tools",
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { Combobox } from "../src";
2+
3+
<Combobox.Clear className={(state) => (state.open ? "open" : "closed")} />;
4+
5+
// Origin clear visibility depends on Base UI owning mount/hidden state.
6+
// @ts-expect-error keepMounted is intentionally not part of the Origin API.
7+
<Combobox.Clear keepMounted />;

0 commit comments

Comments
 (0)