Skip to content

Commit bc626c8

Browse files
committed
style state from aria/data attributes, not props (pagination current, checkbox label disabled)
Same redundancy as tone: a styling prop re-encoding a state already on the element for a11y/Base UI. - PaginationPageButton drops the current prop — the selected fill keys off aria-current=page (which the ready-made already sets to name the current page for assistive tech). - CheckboxLabel drops the disabled prop — the row dims off the wrapped checkbox's data-disabled via :has([data-disabled]) (Base UI sets it / Field propagates it). Verified via play tests (selected fill / disabled cursor render off the attribute, no prop).
1 parent 488daae commit bc626c8

10 files changed

Lines changed: 55 additions & 61 deletions

File tree

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,11 @@ export const DisabledDoesNotToggle: Story = {
249249
const checkbox = canvas.getByRole("checkbox");
250250
await expect(checkbox).toHaveAttribute("aria-checked", "false");
251251
await expect(checkbox).toHaveAttribute("aria-disabled", "true");
252+
// The label row dims itself off the disabled box (`label:has([data-disabled])`), no prop needed:
253+
// the not-allowed cursor renders even though `CheckboxLabel` takes no `disabled` prop.
254+
const labelRow = checkbox.closest("label");
255+
await expect(labelRow).not.toBeNull();
256+
if (labelRow) await expect(getComputedStyle(labelRow).cursor).toBe("not-allowed");
252257
// Clicking the disabled control must not flip its state.
253258
await userEvent.click(checkbox);
254259
await expect(checkbox).toHaveAttribute("aria-checked", "false");

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export function Checkbox({ label, inlineStartNode, id, ...props }: CheckboxProps
5050
if (label == null) return box;
5151

5252
return (
53-
<CheckboxLabel disabled={props.disabled ?? false} htmlFor={checkboxId}>
53+
<CheckboxLabel htmlFor={checkboxId}>
5454
{box}
5555
{inlineStartNode ? (
5656
<CheckboxInlineStartNode>{inlineStartNode}</CheckboxInlineStartNode>

packages/propel/src/components/pagination/pagination.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,6 @@ export function Pagination({
236236
<PaginationPageButton
237237
aria-label={l.page(token)}
238238
aria-current={isCurrent ? "page" : undefined}
239-
current={isCurrent}
240239
disabled={isCurrent}
241240
onClick={() => onPageChange(token)}
242241
>
Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,19 @@
11
import type * as React from "react";
22

3-
import { checkboxLabelVariants, type CheckboxLabelDisabled } from "./variants";
3+
import { checkboxLabelVariants } from "./variants";
44

55
export type CheckboxLabelProps = Omit<
66
React.ComponentPropsWithoutRef<"label">,
77
"className" | "style"
8-
> & {
9-
/** Whether the row reads as disabled (drops the pointer cursor and hover background). */
10-
disabled: CheckboxLabelDisabled;
11-
};
8+
>;
129

1310
/**
1411
* The clickable row chip that wraps a `Checkbox` box with an optional `CheckboxInlineStartNode` and
1512
* the label text, matching the Figma "Checkbox with label" component. Associate it with the box via
16-
* `htmlFor` so clicking anywhere in the row toggles the box.
13+
* `htmlFor` so clicking anywhere in the row toggles the box. The row reads its disabled look off
14+
* the wrapped `Checkbox` (which carries `data-disabled`) via `:has()`, so it takes no `disabled`
15+
* prop.
1716
*/
18-
export function CheckboxLabel({ disabled, ...props }: CheckboxLabelProps) {
19-
return <label className={checkboxLabelVariants({ disabled })} {...props} />;
17+
export function CheckboxLabel(props: CheckboxLabelProps) {
18+
return <label className={checkboxLabelVariants()} {...props} />;
2019
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export const States: Story = {
126126
export const Labeled: Story = {
127127
parameters: { controls: { disable: true } },
128128
render: () => (
129-
<CheckboxLabel disabled={false} htmlFor="ui-checkbox-labeled">
129+
<CheckboxLabel htmlFor="ui-checkbox-labeled">
130130
<Checkbox id="ui-checkbox-labeled">
131131
<CheckboxIndicator>
132132
<Check aria-hidden />

packages/propel/src/ui/checkbox/index.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,3 @@ export * from "./checkbox-indicator";
33
export * from "./checkbox-indeterminate-indicator";
44
export * from "./checkbox-label";
55
export * from "./checkbox-inline-start-node";
6-
export type { CheckboxLabelDisabled } from "./variants";

packages/propel/src/ui/checkbox/variants.ts

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { cva, cx, type VariantProps } from "class-variance-authority";
1+
import { cva, cx } from "class-variance-authority";
22

33
import { checkboxBoxVariants } from "../../internal/checkbox-box";
44
import { nodeSlotClass } from "../../internal/node-slot";
@@ -24,27 +24,18 @@ export const checkboxIndeterminateIndicatorVariants = cva(
2424
),
2525
);
2626

27-
// The clickable label row that wraps the box + optional icon + label text.
28-
// `disabled` mirrors the `disabled` prop; the cursor and hover background change.
27+
// The clickable label row that wraps the box + optional icon + label text. The row reads its
28+
// disabled state off the wrapped `Checkbox` (Base UI — and `Field.Root` — set `data-disabled` on
29+
// it) via `:has()`, so it needs no `disabled` prop: it cancels the hover background and shows the
30+
// not-allowed cursor whenever a descendant is disabled.
2931
export const checkboxLabelVariants = cva(
3032
cx(
3133
"inline-flex items-center gap-2 rounded-sm px-2 py-1",
3234
"text-13 text-secondary transition-colors",
35+
"cursor-pointer not-has-[[data-disabled]]:hover:bg-layer-transparent-hover has-[[data-disabled]]:cursor-not-allowed",
3336
),
34-
{
35-
variants: {
36-
disabled: {
37-
true: "cursor-not-allowed",
38-
false: "cursor-pointer hover:bg-layer-transparent-hover",
39-
},
40-
},
41-
},
4237
);
4338

44-
type CheckboxLabelVariantProps = VariantProps<typeof checkboxLabelVariants>;
45-
46-
export type CheckboxLabelDisabled = NonNullable<CheckboxLabelVariantProps["disabled"]>;
47-
4839
// The inline-start icon slot between the box and the label text.
4940
export const checkboxInlineStartNodeVariants = cva(
5041
cx(nodeSlotClass, "text-icon-secondary [--node-size:0.875rem]"),

packages/propel/src/ui/pagination/pagination-page-button.tsx

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,15 @@ import type * as React from "react";
33
import { paginationPageButtonVariants } from "./variants";
44

55
export type PaginationPageButtonProps = Omit<
6-
React.ComponentProps<"button">,
6+
React.ComponentPropsWithoutRef<"button">,
77
"className" | "style"
8-
> & {
9-
/** Whether this button represents the current page (renders the pressed/selected fill). */
10-
current: boolean;
11-
};
8+
>;
129

1310
/**
14-
* A styled page-number button. Applies `paginationPageButtonVariants({ current })`; pass the page
15-
* number (or a loading spinner) as `children` and wire `aria-current`/`onClick` through props.
11+
* A styled page-number button. The current page is marked `aria-current="page"`, which the cva keys
12+
* the pressed/selected fill off of; pass the page number (or a loading spinner) as `children` and
13+
* wire `aria-current`/`onClick` through props.
1614
*/
17-
export function PaginationPageButton({ current, ...props }: PaginationPageButtonProps) {
18-
return <button type="button" className={paginationPageButtonVariants({ current })} {...props} />;
15+
export function PaginationPageButton(props: PaginationPageButtonProps) {
16+
return <button type="button" className={paginationPageButtonVariants()} {...props} />;
1917
}

packages/propel/src/ui/pagination/pagination.stories.tsx

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ import {
1414
// UI-tier story: composes the ATOMIC pagination parts (each renders a single element).
1515
// The components-tier `Pagination` story owns the truncation logic, the per-page Menu and the
1616
// range label. Here you lay out the raw slots yourself: the `Pagination` nav wraps a
17-
// `PaginationList`, whose `PaginationItem`s hold prev/next arrows, page-number buttons (one
18-
// marked `current`), and the non-interactive ellipsis (an icon slot).
17+
// `PaginationList`, whose `PaginationItem`s hold prev/next arrows, page-number buttons (the
18+
// current one marked `aria-current="page"`, which drives the selected fill), and the
19+
// non-interactive ellipsis (an icon slot).
1920
const meta = {
2021
title: "UI/Pagination",
2122
component: PaginationPageButton,
@@ -26,7 +27,7 @@ const meta = {
2627
PaginationArrowButton,
2728
PaginationEllipsis,
2829
},
29-
args: { current: false, children: "1" },
30+
args: { children: "1" },
3031
} satisfies Meta<typeof PaginationPageButton>;
3132

3233
export default meta;
@@ -48,7 +49,6 @@ export const Default: Story = {
4849
{PAGES.map((n) => (
4950
<PaginationItem key={n}>
5051
<PaginationPageButton
51-
current={n === 1}
5252
aria-current={n === 1 ? "page" : undefined}
5353
aria-label={`Go to page ${n}`}
5454
disabled={n === 1}
@@ -70,6 +70,12 @@ export const Default: Story = {
7070
const page1 = canvas.getByRole("button", { name: "Go to page 1" });
7171
await expect(page1).toHaveAttribute("aria-current", "page");
7272
await expect(canvas.getByRole("button", { name: "Go to previous page" })).toBeDisabled();
73+
// The selected fill keys off `aria-current="page"`, not a prop: the current page's
74+
// background differs from a non-current page button.
75+
const page2 = canvas.getByRole("button", { name: "Go to page 2" });
76+
await expect(getComputedStyle(page1).backgroundColor).not.toBe(
77+
getComputedStyle(page2).backgroundColor,
78+
);
7379
},
7480
};
7581

@@ -87,7 +93,6 @@ export const WithEllipsis: Story = {
8793
{[1, 2, 3].map((n) => (
8894
<PaginationItem key={n}>
8995
<PaginationPageButton
90-
current={n === 2}
9196
aria-current={n === 2 ? "page" : undefined}
9297
aria-label={`Go to page ${n}`}
9398
disabled={n === 2}
@@ -102,9 +107,7 @@ export const WithEllipsis: Story = {
102107
</PaginationEllipsis>
103108
</PaginationItem>
104109
<PaginationItem>
105-
<PaginationPageButton current={false} aria-label="Go to page 100">
106-
100
107-
</PaginationPageButton>
110+
<PaginationPageButton aria-label="Go to page 100">100</PaginationPageButton>
108111
</PaginationItem>
109112
<PaginationItem>
110113
<PaginationArrowButton aria-label="Go to next page">
@@ -116,7 +119,11 @@ export const WithEllipsis: Story = {
116119
),
117120
};
118121

119-
/** The standalone page-number button, with its `current` (pressed) state toggleable via controls. */
122+
/**
123+
* The standalone page-number button. Its pressed/selected fill keys off `aria-current="page"` — the
124+
* current page is marked `aria-current` (and `disabled` to block re-navigation), so toggling that
125+
* attribute toggles the selected fill.
126+
*/
120127
export const PageButton: Story = {
121128
render: (args) => <PaginationPageButton {...args} aria-label="Go to page 1" />,
122129
};

packages/propel/src/ui/pagination/variants.ts

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ import { nodeSlotClass } from "../../internal/node-slot";
44

55
// Pagination is a structural navigation primitive: the Figma "variant" axis (all
66
// pages visible / near start / middle / near end) is a function of where the current
7-
// page sits in the total, not a styling axis — so the parts carry only static chrome
8-
// and the `current` page state. Every part renders ONE element, with its className held
9-
// in the cva below so no `className` crosses the part boundary. The Figma 24px slot is
10-
// the only defined size, so it is baked in (no size/magnitude axis to expose).
7+
// page sits in the total, not a styling axis — so the parts carry only static chrome.
8+
// The current page reads as selected purely from its `aria-current="page"` a11y marker
9+
// (no styling prop). Every part renders ONE element, with its className held in the cva
10+
// below so no `className` crosses the part boundary. The Figma 24px slot is the only
11+
// defined size, so it is baked in (no size/magnitude axis to expose).
1112

1213
// The `<nav>` landmark wrapping the whole control: the optional per-page region and
1314
// range label sit before the page-button list, laid out in a row.
@@ -37,19 +38,14 @@ export const paginationPageButtonVariants = cva(
3738
"hover:bg-layer-transparent-hover",
3839
"focus-visible:ring-2 focus-visible:ring-accent-strong",
3940
"disabled:pointer-events-none disabled:text-disabled",
41+
// The current page reads as pressed: it sits on the `transparent-active` fill
42+
// (Figma "Selected") and is not interactive. It's marked `disabled` to block
43+
// re-navigation AND `aria-current="page"` for a11y, so the selected styling keys
44+
// off `aria-current` — no separate prop. The Figma "Selected" state keeps
45+
// `text/primary`, so the doubled `aria-[current=page]:disabled:` specificity wins
46+
// over the plain `disabled:text-disabled` dim the base applies.
47+
"aria-[current=page]:bg-layer-transparent-active aria-[current=page]:disabled:text-primary",
4048
),
41-
{
42-
variants: {
43-
// The current page reads as pressed: it sits on the `transparent-active` fill
44-
// (Figma "Selected") and is not interactive. It's marked `disabled` to block
45-
// re-navigation, but the Figma "Selected" state keeps `text/primary` — so it
46-
// must override the `disabled:text-disabled` dim that the base applies.
47-
current: {
48-
true: "bg-layer-transparent-active disabled:text-primary",
49-
false: "",
50-
},
51-
},
52-
},
5349
);
5450

5551
export const paginationArrowButtonVariants = cva(

0 commit comments

Comments
 (0)