diff --git a/packages/propel/src/components/autocomplete-field/autocomplete-field.tsx b/packages/propel/src/components/autocomplete-field/autocomplete-field.tsx index 4824bd41..722030c5 100644 --- a/packages/propel/src/components/autocomplete-field/autocomplete-field.tsx +++ b/packages/propel/src/components/autocomplete-field/autocomplete-field.tsx @@ -20,7 +20,7 @@ export type AutocompleteFieldProps = Omit< AutocompleteProps, "children" | "items" > & { - /** Supporting text shown below the input. */ + /** Supporting text shown below the input. Replaced by `error` when an error is set. */ description?: React.ReactNode; /** The clear control (e.g. an `IconButton`) carrying its own localizable `aria-label`. */ clear: React.ReactElement; @@ -76,7 +76,7 @@ export function AutocompleteField({ - {description != null ? ( + {error == null && description != null ? ( {description} ) : null} diff --git a/packages/propel/src/components/breadcrumb/breadcrumb-menu-trigger.tsx b/packages/propel/src/components/breadcrumb/breadcrumb-menu-trigger.tsx index 04317ade..755e5a72 100644 --- a/packages/propel/src/components/breadcrumb/breadcrumb-menu-trigger.tsx +++ b/packages/propel/src/components/breadcrumb/breadcrumb-menu-trigger.tsx @@ -21,7 +21,7 @@ export function BreadcrumbMenuTrigger({ icon, label, ...props }: BreadcrumbMenuT } {...props}> {icon} {label} - + diff --git a/packages/propel/src/components/checkbox-field/checkbox-field.stories.tsx b/packages/propel/src/components/checkbox-field/checkbox-field.stories.tsx index 63b5da66..d0c6a427 100644 --- a/packages/propel/src/components/checkbox-field/checkbox-field.stories.tsx +++ b/packages/propel/src/components/checkbox-field/checkbox-field.stories.tsx @@ -97,7 +97,6 @@ export const InvalidInteraction: Story = { await expect(resting).not.toHaveAttribute("data-invalid"); // The invalid field propagates `data-invalid` onto the box (Base UI Field -> Checkbox.Root). await expect(invalid).toHaveAttribute("data-invalid"); - await expect(invalid).toHaveClass("data-invalid:border-danger-strong"); // ...and the danger border actually renders: its color differs from the resting box's border. await expect(getComputedStyle(invalid).borderColor).not.toBe( getComputedStyle(resting).borderColor, diff --git a/packages/propel/src/components/checkbox-field/checkbox-field.tsx b/packages/propel/src/components/checkbox-field/checkbox-field.tsx index 2e4428b1..3e5c11e4 100644 --- a/packages/propel/src/components/checkbox-field/checkbox-field.tsx +++ b/packages/propel/src/components/checkbox-field/checkbox-field.tsx @@ -1,5 +1,6 @@ import type * as React from "react"; +import { FieldItemControlGroup } from "../../elements/field/field-item-control-group"; import type { FieldMagnitude } from "../../elements/field/variants"; import { CheckboxFieldControl, @@ -40,7 +41,9 @@ export function CheckboxField({ return ( - + + + {label} diff --git a/packages/propel/src/components/checkbox-group-field/checkbox-group-field-option.tsx b/packages/propel/src/components/checkbox-group-field/checkbox-group-field-option.tsx index 73d7c847..4096edec 100644 --- a/packages/propel/src/components/checkbox-group-field/checkbox-group-field-option.tsx +++ b/packages/propel/src/components/checkbox-group-field/checkbox-group-field-option.tsx @@ -1,5 +1,6 @@ import type * as React from "react"; +import { FieldItemControlGroup } from "../../elements/field/field-item-control-group"; import type { FieldMagnitude } from "../../elements/field/variants"; import { CheckboxFieldControl, @@ -31,7 +32,9 @@ export function CheckboxGroupFieldOption({ return ( - + + + {label} diff --git a/packages/propel/src/components/checkbox-group-field/checkbox-group-field.stories.tsx b/packages/propel/src/components/checkbox-group-field/checkbox-group-field.stories.tsx index cdebf142..fb65e1ee 100644 --- a/packages/propel/src/components/checkbox-group-field/checkbox-group-field.stories.tsx +++ b/packages/propel/src/components/checkbox-group-field/checkbox-group-field.stories.tsx @@ -92,16 +92,26 @@ export const Invalid: Story = { }; /** - * Interaction test: every option's box propagates `data-invalid` and the danger border class. - * Tagged out of the sidebar/docs/manifest while still running under the default `test` tag. + * Interaction test: every option's box propagates `data-invalid`. The still-unchecked options show + * the danger border; "Email" starts `defaultValue`-checked, and a checked box keeps its transparent + * border (the accent fill alone communicates the value) even while the group is invalid. Tagged out + * of the sidebar/docs/manifest while still running under the default `test` tag. */ export const InvalidInteraction: Story = { ...Invalid, tags: ["!dev", "!autodocs", "!manifest"], play: async ({ canvas }) => { + const dangerBorder = getComputedStyle(document.documentElement) + .getPropertyValue("--border-danger-strong") + .trim(); for (const box of canvas.getAllByRole("checkbox")) { await expect(box).toHaveAttribute("data-invalid"); - await expect(box).toHaveClass("data-invalid:border-danger-strong"); + const borderColor = getComputedStyle(box).borderColor; + if (box.getAttribute("aria-checked") === "true") { + await expect(borderColor).not.toBe(dangerBorder); + } else { + await expect(borderColor).toBe(dangerBorder); + } } }, }; diff --git a/packages/propel/src/components/checkbox-group/checkbox-group.stories.tsx b/packages/propel/src/components/checkbox-group/checkbox-group.stories.tsx index 0bd58b03..5d03429e 100644 --- a/packages/propel/src/components/checkbox-group/checkbox-group.stories.tsx +++ b/packages/propel/src/components/checkbox-group/checkbox-group.stories.tsx @@ -1,5 +1,4 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import * as React from "react"; import { expect } from "storybook/test"; import { Checkbox } from "../checkbox/index"; @@ -18,8 +17,8 @@ const meta = { export default meta; type Story = StoryObj; -// Every child value in the parent-checkbox stories; the group's `allValues` prop -// is what lets a `parent` checkbox derive its checked/indeterminate state. +// Every value in the select-all story; the group's `allValues` prop is what lets the `parent` +// checkbox derive its checked/indeterminate state from the flat list of rows. const PROTOCOLS = ["http", "https", "ssh"]; /** Labeled checkbox rows; the first is selected by default. */ @@ -50,176 +49,160 @@ export const DefaultInteraction: Story = { }; /** - * A `parent` checkbox controls the whole group: pass the group `allValues` and mark one row - * `parent`. With only some values selected it shows the indeterminate dash; toggling it checks or - * clears every row. Explicit child ids + `aria-controls` keep the parent's controls references - * valid (propel rows always carry their own label id). + * A **select-all** control (Base UI's `parent` checkbox): pass the group `allValues` and mark one + * row `parent`. Base UI derives that row's state from the flat list — checked when every row is on, + * the indeterminate dash when only some are, unchecked when none — and toggling it checks or clears + * every row at once. The control is an affordance over the whole group, not a value of its own, so + * label it plainly ("Select all"). Rows carry no explicit `id`: Base UI derives each child input id + * as `${groupId}-${value}` and auto-wires the parent's `aria-controls` to match (a custom `id` on a + * grouped child is ignored — base-ui#2691). */ -export const ParentCheckbox: Story = { +export const SelectAll: Story = { args: { density: "comfortable", defaultValue: ["https"], allValues: PROTOCOLS }, render: (args) => ( - - - - + + + + ), }; /** - * Interaction test: the parent reads `mixed` for a partial selection, checks every row when + * Interaction test: the select-all row reads `mixed` for a partial selection, checks every row when * clicked, clears them on the next click, and returns to `mixed` when a single row is checked. * Tagged out of the sidebar/docs/manifest while still running under the default `test` tag. */ -export const ParentCheckboxInteraction: Story = { - ...ParentCheckbox, +export const SelectAllInteraction: Story = { + ...SelectAll, tags: ["!dev", "!autodocs", "!manifest"], play: async ({ canvas, userEvent }) => { - const parent = canvas.getByRole("checkbox", { name: "All protocols" }); + const selectAll = canvas.getByRole("checkbox", { name: "Select all" }); const http = canvas.getByRole("checkbox", { name: "HTTP" }); const https = canvas.getByRole("checkbox", { name: "HTTPS" }); const ssh = canvas.getByRole("checkbox", { name: "SSH" }); - // one of three values selected → the parent reports the indeterminate (mixed) state - await expect(parent).toHaveAttribute("aria-checked", "mixed"); + // one of three values selected → the select-all reports the indeterminate (mixed) state + await expect(selectAll).toHaveAttribute("aria-checked", "mixed"); - // clicking a mixed parent selects every value - await userEvent.click(parent); - await expect(parent).toHaveAttribute("aria-checked", "true"); + // clicking a mixed select-all selects every value + await userEvent.click(selectAll); + await expect(selectAll).toHaveAttribute("aria-checked", "true"); await expect(http).toHaveAttribute("aria-checked", "true"); await expect(https).toHaveAttribute("aria-checked", "true"); await expect(ssh).toHaveAttribute("aria-checked", "true"); - // clicking a fully-checked parent clears the group - await userEvent.click(parent); - await expect(parent).toHaveAttribute("aria-checked", "false"); + // clicking a fully-checked select-all clears the group + await userEvent.click(selectAll); + await expect(selectAll).toHaveAttribute("aria-checked", "false"); await expect(http).toHaveAttribute("aria-checked", "false"); await expect(https).toHaveAttribute("aria-checked", "false"); await expect(ssh).toHaveAttribute("aria-checked", "false"); - // checking a single row returns the parent to mixed + // checking a single row returns the select-all to mixed await userEvent.click(http); - await expect(parent).toHaveAttribute("aria-checked", "mixed"); + await expect(selectAll).toHaveAttribute("aria-checked", "mixed"); }, }; -// Values for the nested-group stories: the outer group tracks the top-level permissions while -// the nested group tracks the user-management subset behind the "manage-users" value. -const MAIN_PERMISSIONS = ["view-dashboard", "manage-users", "access-reports"]; -const MANAGEMENT_PERMISSIONS = ["create-user", "edit-user", "delete-user", "assign-roles"]; +// The nested story models a real permission tree as two INDEPENDENT categories. Each category is its +// own group whose `allValues` are exactly that category's children, so its `parent` rolls up only +// its own children — never the sibling category. +const MANAGE_USERS_PERMISSIONS = ["create-user", "edit-user", "delete-user", "assign-roles"]; +const MANAGE_CONTENT_PERMISSIONS = ["create-content", "edit-content", "publish-content"]; /** - * Groups nest: a nested `CheckboxGroup` (with its own `parent` row) manages a subset of values - * while the outer group tracks the top level. The two controlled selections stay in sync through - * `onValueChange` — completing the nested group checks its value into the outer one — and the outer - * parent's explicit `indeterminate` shows the dash while the nested selection is partial. + * A genuine parent/child hierarchy — distinct from the flat `SelectAll` control above. Each + * category ("Manage users", "Manage content") is its OWN `CheckboxGroup` whose `allValues` are + * exactly that category's children, so its `parent` row rolls up ONLY its own children: it checks + * when all of them are on, shows the indeterminate dash when only some are, and toggling it checks + * or clears its own children. The categories are independent — a row under "Manage users" never + * affects "Manage content". Both groups are uncontrolled (`defaultValue`), so Base UI owns every + * parent/child state with no manual syncing. "User permissions" is a plain heading, not a control. */ export const NestedParentCheckbox: Story = { - args: { density: "comfortable", allValues: MAIN_PERMISSIONS }, - render: function Render(args) { - const [mainValue, setMainValue] = React.useState([]); - const [managementValue, setManagementValue] = React.useState([]); - return ( + parameters: { controls: { disable: true } }, + args: { density: "comfortable" }, + render: (args) => ( +
+

User permissions

{ - if (value.includes("manage-users")) { - setManagementValue(MANAGEMENT_PERMISSIONS); - } else if (managementValue.length === MANAGEMENT_PERMISSIONS.length) { - setManagementValue([]); - } - setMainValue(value); - }} + density={args.density} + aria-label="Manage users" + allValues={MANAGE_USERS_PERMISSIONS} + defaultValue={[]} > - 0 && managementValue.length !== MANAGEMENT_PERMISSIONS.length - } - /> - - -
- { - if (value.length === MANAGEMENT_PERMISSIONS.length) { - setMainValue((prev) => - prev.includes("manage-users") ? prev : [...prev, "manage-users"], - ); - } else { - setMainValue((prev) => prev.filter((v) => v !== "manage-users")); - } - setManagementValue(value); - }} - > - - - - - - + +
+ + + +
- ); - }, + + +
+ + + +
+ +
+ ), }; /** - * Interaction test: one nested row turns the nested parent `mixed` and puts the indeterminate dash - * on the outer parent (its explicit `indeterminate` surfaces as `data-indeterminate`; - * `aria-checked` stays group-derived). Completing the nested group checks "manage-users" into the - * outer group, and the outer parent then selects and clears every value across both groups. Tagged - * out of the sidebar/docs/manifest while still running under the default `test` tag. + * Interaction test for the parent/child hierarchy: selecting one child turns ONLY its own parent + * `mixed` (the sibling category is untouched), completing every child checks that parent, toggling + * a parent checks/clears only its own children, and toggling one category never leaks into the + * other. Tagged out of the sidebar/docs/manifest while still running under the default `test` tag. */ export const NestedParentCheckboxInteraction: Story = { ...NestedParentCheckbox, tags: ["!dev", "!autodocs", "!manifest"], play: async ({ canvas, userEvent }) => { - const userPermissions = canvas.getByRole("checkbox", { name: "User permissions" }); const manageUsers = canvas.getByRole("checkbox", { name: "Manage users" }); const createUser = canvas.getByRole("checkbox", { name: "Create user" }); - const viewDashboard = canvas.getByRole("checkbox", { name: "View dashboard" }); + const editUser = canvas.getByRole("checkbox", { name: "Edit user" }); + const deleteUser = canvas.getByRole("checkbox", { name: "Delete user" }); + const assignRoles = canvas.getByRole("checkbox", { name: "Assign roles" }); + const manageContent = canvas.getByRole("checkbox", { name: "Manage content" }); + const createContent = canvas.getByRole("checkbox", { name: "Create content" }); // nothing selected anywhere → both parents read clear - await expect(userPermissions).toHaveAttribute("aria-checked", "false"); await expect(manageUsers).toHaveAttribute("aria-checked", "false"); + await expect(manageContent).toHaveAttribute("aria-checked", "false"); - // one nested row → the nested parent turns mixed and the outer parent shows the dash - // through its explicit `indeterminate` wiring + // selecting one child turns ONLY its own parent mixed; the sibling category is untouched await userEvent.click(createUser); await expect(manageUsers).toHaveAttribute("aria-checked", "mixed"); - await expect(userPermissions).toHaveAttribute("data-indeterminate"); + await expect(manageContent).toHaveAttribute("aria-checked", "false"); + await expect(createContent).toHaveAttribute("aria-checked", "false"); - // completing the nested group syncs "manage-users" into the outer group → outer parent mixed - await userEvent.click(manageUsers); + // completing every child of a category checks its parent — the sibling stays clear + await userEvent.click(editUser); + await userEvent.click(deleteUser); + await userEvent.click(assignRoles); await expect(manageUsers).toHaveAttribute("aria-checked", "true"); - await expect(createUser).toHaveAttribute("aria-checked", "true"); - await expect(userPermissions).toHaveAttribute("aria-checked", "mixed"); + await expect(manageContent).toHaveAttribute("aria-checked", "false"); - // checking the outer parent selects every value in both groups - await userEvent.click(userPermissions); - await expect(userPermissions).toHaveAttribute("aria-checked", "true"); - await expect(viewDashboard).toHaveAttribute("aria-checked", "true"); - await expect(manageUsers).toHaveAttribute("aria-checked", "true"); + // toggling a checked parent clears only its own children + await userEvent.click(manageUsers); + await expect(manageUsers).toHaveAttribute("aria-checked", "false"); + await expect(createUser).toHaveAttribute("aria-checked", "false"); + await expect(assignRoles).toHaveAttribute("aria-checked", "false"); - // clearing the outer parent empties both groups - await userEvent.click(userPermissions); - await expect(userPermissions).toHaveAttribute("aria-checked", "false"); - await expect(viewDashboard).toHaveAttribute("aria-checked", "false"); + // toggling the OTHER parent checks only its own children — no leak into the first category + await userEvent.click(manageContent); + await expect(manageContent).toHaveAttribute("aria-checked", "true"); + await expect(createContent).toHaveAttribute("aria-checked", "true"); await expect(manageUsers).toHaveAttribute("aria-checked", "false"); await expect(createUser).toHaveAttribute("aria-checked", "false"); }, diff --git a/packages/propel/src/components/checkbox/checkbox.stories.tsx b/packages/propel/src/components/checkbox/checkbox.stories.tsx index ca5d01a6..3fb3b202 100644 --- a/packages/propel/src/components/checkbox/checkbox.stories.tsx +++ b/packages/propel/src/components/checkbox/checkbox.stories.tsx @@ -4,6 +4,7 @@ import * as React from "react"; import { expect, userEvent } from "storybook/test"; import { Button } from "../button"; +import { CheckboxField } from "../checkbox-field"; import { Field } from "../field"; import { Form, FormActions, FormBody } from "../form"; import { Icon } from "../icon"; @@ -17,10 +18,14 @@ import { const meta = { title: "Components/Checkbox", component: Checkbox, + // The ready-made Checkbox is the bare box (+ optional inline label); CheckboxField is the + // labeled-row composition that owns the Field name for form submission — add its tab to the + // args table and record the relationship in the manifest (mirrors Switch/RadioGroup). subcomponents: { CheckboxLabel, CheckboxIndicator, CheckboxIndeterminateIndicator, + CheckboxField, }, args: { "aria-label": "Example", @@ -164,9 +169,10 @@ export const Invalid: Story = { }; /** - * Interaction test: the invalid `Field` propagates `data-invalid` and the danger border, while the - * checked box keeps the accent fill. Tagged out of the sidebar/docs/manifest while still running - * under the default `test` tag. + * Interaction test: the invalid `Field` propagates `data-invalid` and the danger border on the + * RESTING box only — once checked, the danger border clears back to transparent (the accent fill + * alone communicates the value; a required-and-now-satisfied checkbox shouldn't still ring red). + * Tagged out of the sidebar/docs/manifest while still running under the default `test` tag. */ export const InvalidInteraction: Story = { ...Invalid, @@ -182,17 +188,29 @@ export const InvalidInteraction: Story = { await expect(getComputedStyle(unchecked).borderColor).not.toBe( getComputedStyle(resting).borderColor, ); - // Checked invalid box: accent-blue fill, like every other state. + // Checked invalid box: accent-blue fill, like every other state... await expect(checked).toHaveAttribute("aria-checked", "true"); - await expect(checked).toHaveClass("data-checked:bg-accent-primary"); + await expect(checked).toHaveAttribute("data-invalid"); + await expect(getComputedStyle(checked).backgroundColor).not.toBe( + getComputedStyle(resting).backgroundColor, + ); + // ...and, despite still being `data-invalid`, the danger border does NOT persist through the + // checked fill: `data-checked:border-transparent` takes over, same as a checked box anywhere + // else — it must NOT still show the unchecked invalid box's red border. + await expect(getComputedStyle(checked).borderColor).not.toBe( + getComputedStyle(unchecked).borderColor, + ); }, }; /** - * Form integration: wrap the checkbox in a `Field` with a `name` and Base UI wires the rest — the - * field name flows onto the box, a hidden input serializes it with the form, and `Form`'s - * `onFormSubmit` receives the checked state as a boolean. The checkbox itself needs no extra - * wiring. Submit to see the captured value. + * Form integration: a sign-in form where "Stay logged in" needs to submit alongside the rest of the + * fields — e.g. to extend the session server-side once the user authenticates. `CheckboxField` (the + * labeled-row ready-made) already owns the `Field` name, so inside a `Form` its checked state + * serializes with the submission — `onFormSubmit` receives it under the field's `name` as a + * boolean, with no extra wiring on the checkbox. Reach for the bare `Checkbox` + a hand-wired + * `Field` (as in the `Invalid` story above) only when you need a custom row layout `CheckboxField` + * doesn't offer. Submit to see the captured value. */ export const FormIntegration: Story = { parameters: { controls: { disable: true } }, @@ -202,9 +220,7 @@ export const FormIntegration: Story = {
onFormSubmit={(values) => setSubmitted(values)}> - - - +