Skip to content

Commit 286adf5

Browse files
committed
checkbox: drive danger from data-invalid, drop the redundant tone prop
The error look is a STATE, not a prop. Base UI's Field.Root propagates data-invalid to the Checkbox.Root, so the box cva rests at the neutral border and recolors to danger off data-invalid:border-danger-strong — removing the tone axis from checkbox-box and the tone prop from Checkbox, CheckboxField, CheckboxGroupFieldOption (an invalid field now tones every option danger for free, no cascade). Verified end-to-end: play tests assert data-invalid reaches the checkbox box and the danger border renders. The menu checkbox indicator drops its tone=neutral arg (rests neutral).
1 parent 20e9408 commit 286adf5

15 files changed

Lines changed: 129 additions & 91 deletions

File tree

packages/propel/src/components/checkbox-field/checkbox-field.stories.tsx

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ const meta = {
1111
name: "emailUpdates",
1212
label: "Email updates",
1313
magnitude: "md",
14-
tone: "neutral",
1514
value: "enabled",
1615
},
1716
} satisfies Meta<typeof CheckboxField>;
@@ -27,6 +26,39 @@ export const Default: Story = {
2726
},
2827
};
2928

29+
/**
30+
* Setting `error` marks the field invalid. Base UI's `Field.Root` propagates that validity to the
31+
* checkbox box as `data-invalid`, and the box recolors its border to `danger` — no `tone` prop. A
32+
* resting field is shown alongside so the danger border is visibly (and assertably) different.
33+
*/
34+
export const Invalid: Story = {
35+
parameters: { controls: { disable: true } },
36+
render: () => (
37+
<div className="flex flex-col gap-4">
38+
<CheckboxField name="resting" label="Resting" magnitude="md" value="a" />
39+
<CheckboxField
40+
name="terms"
41+
label="Accept the terms"
42+
magnitude="md"
43+
value="b"
44+
error="You must accept the terms to continue."
45+
/>
46+
</div>
47+
),
48+
play: async ({ canvas }) => {
49+
const [resting, invalid] = canvas.getAllByRole("checkbox");
50+
// The error-free field leaves the box in its resting (non-invalid) state.
51+
await expect(resting).not.toHaveAttribute("data-invalid");
52+
// The invalid field propagates `data-invalid` onto the box (Base UI Field -> Checkbox.Root).
53+
await expect(invalid).toHaveAttribute("data-invalid");
54+
await expect(invalid).toHaveClass("data-invalid:border-danger-strong");
55+
// ...and the danger border actually renders: its color differs from the resting box's border.
56+
await expect(getComputedStyle(invalid).borderColor).not.toBe(
57+
getComputedStyle(resting).borderColor,
58+
);
59+
},
60+
};
61+
3062
export const RendersCheckbox: Story = {
3163
tags: ["!dev", "!autodocs", "!manifest"],
3264
play: async ({ canvas }) => {

packages/propel/src/components/checkbox-field/checkbox-field.tsx

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
CheckboxFieldControl,
55
type CheckboxFieldControlProps,
66
} from "../../internal/checkbox-field-control";
7-
import type { CheckboxTone } from "../../ui/checkbox/index";
87
import { Field } from "../../ui/field/field";
98
import { FieldItem } from "../../ui/field/field-item";
109
import type { FieldMagnitude } from "../../ui/field/variants";
@@ -13,7 +12,7 @@ import { FieldHelperText } from "../field/field-helper-text";
1312

1413
export type CheckboxFieldProps = Omit<
1514
CheckboxFieldControlProps,
16-
"aria-label" | "label" | "inlineStartNode" | "tone"
15+
"aria-label" | "label" | "inlineStartNode"
1716
> & {
1817
/** Helper text shown below the control. Replaced by `error` when an error is set. */
1918
hint?: React.ReactNode;
@@ -25,8 +24,6 @@ export type CheckboxFieldProps = Omit<
2524
magnitude: FieldMagnitude;
2625
/** Optional supporting text announced as the checkbox description. */
2726
description?: React.ReactNode;
28-
/** Resting color of the box. */
29-
tone: CheckboxTone;
3027
};
3128

3229
/** Ready-to-use single checkbox field with label, description, and helper/error text. */
@@ -36,19 +33,14 @@ export function CheckboxField({
3633
hint,
3734
error,
3835
magnitude,
39-
tone,
4036
name,
4137
disabled,
4238
...controlProps
4339
}: CheckboxFieldProps) {
4440
return (
45-
<Field
46-
name={name}
47-
disabled={disabled}
48-
invalid={error != null || tone === "danger" || undefined}
49-
>
41+
<Field name={name} disabled={disabled} invalid={error != null || undefined}>
5042
<FieldItem disabled={disabled}>
51-
<CheckboxFieldControl tone={tone} disabled={disabled} {...controlProps} />
43+
<CheckboxFieldControl disabled={disabled} {...controlProps} />
5244
<FieldItemContent magnitude={magnitude} description={description}>
5345
{label}
5446
</FieldItemContent>

packages/propel/src/components/checkbox-group-field/checkbox-group-field-option.tsx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,38 +5,34 @@ import {
55
type CheckboxFieldControlProps,
66
} from "../../internal/checkbox-field-control";
77
import { useFieldOptionMagnitude } from "../../internal/field-option-magnitude";
8-
import type { CheckboxTone } from "../../ui/checkbox/index";
98
import { FieldItem } from "../../ui/field/field-item";
109
import type { FieldMagnitude } from "../../ui/field/variants";
1110
import { FieldItemContent } from "../field";
1211

1312
export type CheckboxGroupFieldOptionProps = Omit<
1413
CheckboxFieldControlProps,
15-
"aria-label" | "label" | "inlineStartNode" | "tone"
14+
"aria-label" | "label" | "inlineStartNode"
1615
> & {
1716
/** Visible option label. */
1817
label: React.ReactNode;
1918
/** Optional supporting text announced as the checkbox description. */
2019
description?: React.ReactNode;
2120
/** Label and description size. Inherited from `CheckboxGroupField` when omitted. */
2221
magnitude?: FieldMagnitude;
23-
/** Resting color of the box. */
24-
tone: CheckboxTone;
2522
};
2623

2724
/** A checkbox option row for use inside `CheckboxGroupField` or a custom `CheckboxGroup`. */
2825
export function CheckboxGroupFieldOption({
2926
label,
3027
description,
3128
magnitude: magnitudeProp,
32-
tone,
3329
...props
3430
}: CheckboxGroupFieldOptionProps) {
3531
const magnitude = useFieldOptionMagnitude(magnitudeProp);
3632

3733
return (
3834
<FieldItem disabled={props.disabled}>
39-
<CheckboxFieldControl tone={tone} {...props} />
35+
<CheckboxFieldControl {...props} />
4036
<FieldItemContent magnitude={magnitude} description={description}>
4137
{label}
4238
</FieldItemContent>

packages/propel/src/components/checkbox-group-field/checkbox-group-field.stories.tsx

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,8 @@ const meta = {
1717
defaultValue: ["email"],
1818
children: (
1919
<>
20-
<CheckboxGroupFieldOption value="email" label="Email" tone="neutral" />
21-
<CheckboxGroupFieldOption
22-
value="slack"
23-
label="Slack"
24-
description="Workspace alerts."
25-
tone="neutral"
26-
/>
20+
<CheckboxGroupFieldOption value="email" label="Email" />
21+
<CheckboxGroupFieldOption value="slack" label="Slack" description="Workspace alerts." />
2722
</>
2823
),
2924
},
@@ -34,6 +29,21 @@ type Story = StoryObj<typeof meta>;
3429

3530
export const Default: Story = { args: { hint: "At least one channel is recommended." } };
3631

32+
/**
33+
* Setting `error` marks the whole group invalid. Base UI's `Field.Root` propagates that validity to
34+
* every checkbox box as `data-invalid`, so each option's border recolors to `danger` automatically
35+
* — no per-option `tone`.
36+
*/
37+
export const Invalid: Story = {
38+
args: { error: "Choose at least one channel." },
39+
play: async ({ canvas }) => {
40+
for (const box of canvas.getAllByRole("checkbox")) {
41+
await expect(box).toHaveAttribute("data-invalid");
42+
await expect(box).toHaveClass("data-invalid:border-danger-strong");
43+
}
44+
},
45+
};
46+
3747
export const RendersGroup: Story = {
3848
tags: ["!dev", "!autodocs", "!manifest"],
3949
play: async ({ canvas }) => {

packages/propel/src/components/checkbox-group/checkbox-group.stories.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ export const Default: Story = {
2222
args: { density: "comfortable", defaultValue: ["https"] },
2323
render: (args) => (
2424
<CheckboxGroup {...args} aria-label="Allowed protocols">
25-
<Checkbox tone="neutral" value="http" label="HTTP" />
26-
<Checkbox tone="neutral" value="https" label="HTTPS" />
27-
<Checkbox tone="neutral" value="ssh" label="SSH" />
25+
<Checkbox value="http" label="HTTP" />
26+
<Checkbox value="https" label="HTTPS" />
27+
<Checkbox value="ssh" label="SSH" />
2828
</CheckboxGroup>
2929
),
3030
play: async ({ canvas }) => {
@@ -41,12 +41,12 @@ export const Density: Story = {
4141
render: () => (
4242
<div className="flex items-start gap-10">
4343
<CheckboxGroup density="comfortable" defaultValue={["daily"]} aria-label="Comfortable">
44-
<Checkbox tone="neutral" value="daily" label="Daily" />
45-
<Checkbox tone="neutral" value="weekly" label="Weekly" />
44+
<Checkbox value="daily" label="Daily" />
45+
<Checkbox value="weekly" label="Weekly" />
4646
</CheckboxGroup>
4747
<CheckboxGroup density="compact" defaultValue={["daily"]} aria-label="Compact">
48-
<Checkbox tone="neutral" value="daily" label="Daily" />
49-
<Checkbox tone="neutral" value="weekly" label="Weekly" />
48+
<Checkbox value="daily" label="Daily" />
49+
<Checkbox value="weekly" label="Weekly" />
5050
</CheckboxGroup>
5151
</div>
5252
),
@@ -58,8 +58,8 @@ export const SelectionBehavior: Story = {
5858
args: { density: "comfortable", defaultValue: [] },
5959
render: (args) => (
6060
<CheckboxGroup {...args} aria-label="Allowed protocols">
61-
<Checkbox tone="neutral" value="http" label="HTTP" />
62-
<Checkbox tone="neutral" value="https" label="HTTPS" />
61+
<Checkbox value="http" label="HTTP" />
62+
<Checkbox value="https" label="HTTPS" />
6363
</CheckboxGroup>
6464
),
6565
play: async ({ canvas, userEvent }) => {

packages/propel/src/components/checkbox/checkbox.stories.tsx

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Repeat } from "lucide-react";
33
import * as React from "react";
44
import { expect, userEvent } from "storybook/test";
55

6+
import { Field } from "../../ui/field/field";
67
import {
78
Checkbox,
89
CheckboxIndeterminateIndicator,
@@ -21,7 +22,6 @@ const meta = {
2122
CheckboxIndeterminateIndicator,
2223
},
2324
args: {
24-
tone: "neutral",
2525
"aria-label": "Example",
2626
},
2727
parameters: {
@@ -55,11 +55,11 @@ export const States: Story = {
5555
parameters: { controls: { disable: true } },
5656
render: () => (
5757
<div className="flex items-center gap-4">
58-
<Checkbox tone="neutral" label="Unchecked" />
59-
<Checkbox tone="neutral" label="Checked" defaultChecked />
60-
<Checkbox tone="neutral" label="Indeterminate" indeterminate />
61-
<Checkbox tone="neutral" label="Disabled" disabled />
62-
<Checkbox tone="neutral" label="Disabled checked" disabled defaultChecked />
58+
<Checkbox label="Unchecked" />
59+
<Checkbox label="Checked" defaultChecked />
60+
<Checkbox label="Indeterminate" indeterminate />
61+
<Checkbox label="Disabled" disabled />
62+
<Checkbox label="Disabled checked" disabled defaultChecked />
6363
</div>
6464
),
6565
play: async ({ canvas }) => {
@@ -77,13 +77,13 @@ export const States: Story = {
7777
*/
7878
export const WithoutLabel: Story = {
7979
parameters: { controls: { disable: true } },
80-
render: () => <Checkbox tone="neutral" aria-label="Select row" />,
80+
render: () => <Checkbox aria-label="Select row" />,
8181
};
8282

8383
/** A single labeled checkbox; the whole row is the clickable label. */
8484
export const WithLabel: Story = {
8585
parameters: { controls: { disable: true } },
86-
render: () => <Checkbox tone="neutral" label="Send me product updates" defaultChecked />,
86+
render: () => <Checkbox label="Send me product updates" defaultChecked />,
8787
};
8888

8989
/**
@@ -94,7 +94,6 @@ export const WithIcon: Story = {
9494
parameters: { controls: { disable: true } },
9595
render: () => (
9696
<Checkbox
97-
tone="neutral"
9897
inlineStartNode={<Repeat aria-hidden className="size-3.5" />}
9998
label="Sync automatically"
10099
defaultChecked
@@ -105,30 +104,43 @@ export const WithIcon: Story = {
105104
/** The mixed state renders `aria-checked="mixed"` and shows a dash. */
106105
export const Indeterminate: Story = {
107106
parameters: { controls: { disable: true } },
108-
render: () => <Checkbox tone="neutral" label="Select all" indeterminate />,
107+
render: () => <Checkbox label="Select all" indeterminate />,
109108
play: async ({ canvas }) => {
110109
await expect(canvas.getByRole("checkbox")).toHaveAttribute("aria-checked", "mixed");
111110
},
112111
};
113112

114113
/**
115-
* The Figma "Error" state. The danger tone only colors the _unchecked_ border red; once checked,
116-
* the fill is the same accent blue as every other tone.
114+
* The Figma "Error" state. The danger look is a STATE, not a prop: inside an invalid `Field.Root`
115+
* Base UI propagates `data-invalid` to the box, which recolors its _unchecked_ border red. Once
116+
* checked, the fill is the same accent blue as every other state. A resting checkbox is shown
117+
* alongside for contrast.
117118
*/
118119
export const Error: Story = {
119120
parameters: { controls: { disable: true } },
120121
render: () => (
121122
<div className="flex items-center gap-4">
122-
<Checkbox tone="danger" label="Required" />
123-
<Checkbox tone="danger" label="Required (checked)" defaultChecked />
123+
<Checkbox label="Resting" />
124+
<Field invalid>
125+
<Checkbox label="Required" />
126+
</Field>
127+
<Field invalid>
128+
<Checkbox label="Required (checked)" defaultChecked />
129+
</Field>
124130
</div>
125131
),
126132
play: async ({ canvas }) => {
127-
const [unchecked, checked] = canvas.getAllByRole("checkbox");
128-
// Unchecked danger box: red border, no accent fill.
133+
const [resting, unchecked, checked] = canvas.getAllByRole("checkbox");
134+
// The resting box has no field-invalid state.
135+
await expect(resting).not.toHaveAttribute("data-invalid");
136+
// The invalid `Field` propagates `data-invalid` onto the box (Base UI Field -> Checkbox.Root).
137+
await expect(unchecked).toHaveAttribute("data-invalid");
129138
await expect(unchecked).toHaveAttribute("aria-checked", "false");
130-
await expect(unchecked).toHaveClass("border-danger-strong");
131-
// Checked danger box: accent-blue fill, like every other tone.
139+
// ...and the danger border actually renders: its border color differs from the resting box.
140+
await expect(getComputedStyle(unchecked).borderColor).not.toBe(
141+
getComputedStyle(resting).borderColor,
142+
);
143+
// Checked invalid box: accent-blue fill, like every other state.
132144
await expect(checked).toHaveAttribute("aria-checked", "true");
133145
await expect(checked).toHaveClass("data-checked:bg-accent-primary");
134146
},
@@ -186,7 +198,7 @@ export const BoxDoesNotShiftOnToggle: Story = {
186198
const [indeterminate, setIndeterminate] = React.useState(false);
187199
return (
188200
<div>
189-
<Checkbox tone="neutral" aria-label="Shift target" indeterminate={indeterminate} />
201+
<Checkbox aria-label="Shift target" indeterminate={indeterminate} />
190202
<button type="button" onClick={() => setIndeterminate(true)}>
191203
make indeterminate
192204
</button>

packages/propel/src/components/checkbox/checkbox.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ import {
1010
type CheckboxProps as CheckboxElementProps,
1111
} from "../../ui/checkbox";
1212

13-
export type { CheckboxTone } from "../../ui/checkbox";
14-
1513
export type CheckboxProps = CheckboxElementProps & {
1614
/**
1715
* Optional text shown beside the box; the whole row becomes the clickable label. Omit it for a
@@ -31,15 +29,15 @@ export type CheckboxProps = CheckboxElementProps & {
3129
* The ready-made checkbox: composes the atomic `Checkbox` box with its check and indeterminate
3230
* indicators, and optionally wraps the row in a clickable `CheckboxLabel` with an icon slot.
3331
*/
34-
export function Checkbox({ tone, label, inlineStartNode, id, ...props }: CheckboxProps) {
32+
export function Checkbox({ label, inlineStartNode, id, ...props }: CheckboxProps) {
3533
// Generate a stable id so an explicit `label` can be associated with the box.
3634
const generatedId = React.useId();
3735
const checkboxId = id ?? generatedId;
3836

3937
// Only force the generated id when there's a `label` to associate; without one (e.g. inside a
4038
// `Field`, which manages labeling), pass the caller's `id` through untouched.
4139
const box = (
42-
<CheckboxElement id={label != null ? checkboxId : id} tone={tone} {...props}>
40+
<CheckboxElement id={label != null ? checkboxId : id} {...props}>
4341
<CheckboxIndicator>
4442
<Check aria-hidden />
4543
</CheckboxIndicator>

packages/propel/src/components/form/form.stories.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,9 @@ function ExampleForm({ onFormSubmit }: ExampleFormProps) {
103103
density="comfortable"
104104
defaultValue={["https"]}
105105
>
106-
<CheckboxGroupFieldOption tone="neutral" value="http" label="HTTP" />
107-
<CheckboxGroupFieldOption tone="neutral" value="https" label="HTTPS" />
108-
<CheckboxGroupFieldOption tone="neutral" value="ssh" label="SSH" />
106+
<CheckboxGroupFieldOption value="http" label="HTTP" />
107+
<CheckboxGroupFieldOption value="https" label="HTTPS" />
108+
<CheckboxGroupFieldOption value="ssh" label="SSH" />
109109
</CheckboxGroupField>
110110
<SwitchField
111111
name="restartOnFailure"

packages/propel/src/components/popover/popover.stories.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,11 @@ function ToggleFooter({ defaultToggles = {} }: { defaultToggles?: Record<string,
5858
return (
5959
<div className="flex flex-col">
6060
<Checkbox
61-
tone="neutral"
6261
checked={Boolean(toggles.sub)}
6362
onCheckedChange={(next) => setToggles((t) => ({ ...t, sub: Boolean(next) }))}
6463
label={<span className="flex-1">Show sub-work items</span>}
6564
/>
6665
<Checkbox
67-
tone="neutral"
6866
checked={Boolean(toggles.empty)}
6967
onCheckedChange={(next) => setToggles((t) => ({ ...t, empty: Boolean(next) }))}
7068
label={<span className="flex-1">Show empty groups</span>}

0 commit comments

Comments
 (0)