feat(table): add onCheckedChange to Table.CheckCell and Table.CheckHead by mattrothenberg · Pull Request #437 · cloudflare/kumo

mattrothenberg · 2026-04-22T18:35:32Z

Summary

Align `Table.CheckCell` and `Table.CheckHead` with the `Checkbox` component's signature by adding an `onCheckedChange` prop that takes an optional second argument exposing event details. The existing `onValueChange` prop is retained as a deprecated alias — this is a backwards-compatible, additive change (minor bump).

Context

The v2 Checkbox breaking change removed `onValueChange` in favor of `onCheckedChange` (with optional event details). Table's `Table.CheckCell` / `Table.CheckHead` happened to use the same name (`onValueChange`) for their own consumer-facing prop — it's a Table prop, not a Checkbox passthrough, so v2 doesn't break Table consumers. But the naming collision would be confusing going forward: the Table prop would share a name with a prop Checkbox explicitly deleted.

Change

New prop: `onCheckedChange: (checked: boolean, eventDetails?: CheckboxChangeEventDetails) => void` on `Table.CheckCell` and `Table.CheckHead`.
Deprecated alias: `onValueChange` retained with `@deprecated` JSDoc. Picked up automatically by the `no-deprecated-props` lint rule via component registry codegen.
Internal wiring: Both handlers fire when set, so code passing either prop (or both, during migration) works identically.
Docs: updated usage examples, added migration snippet on the `/components/table` page.
Demos: existing `TableDemo.tsx` demos migrated to the new name as an example for readers.
Tests: new `table.test.tsx` locks in three invariants — new prop works with event details, deprecated prop still fires, both can coexist.

Migration (optional)

// Before (still works, but now emits a lint warning)
<Table.CheckCell onValueChange={(checked) => toggleRow(id)} />

// After
<Table.CheckCell onCheckedChange={(checked) => toggleRow(id)} />

// With event details
<Table.CheckCell
  onCheckedChange={(checked, eventDetails) => {
    toggleRow(id);
    eventDetails?.event.stopPropagation();
  }}
/>

Why additive instead of breaking

Considered as part of the v2 major (synchronize with Checkbox) but landed as additive because:

Every prior removed-prop transition in Kumo (`Checkbox.onChange`, `Select.hideLabel`, `Banner.text`, `DropdownMenu.Item.href`, …) went through a deprecation cycle first. Consistency with project convention.
v2's narrative is "Checkbox signature change"; bundling a separate Table rename would muddy the changelog and force consumer migrations they didn't sign up for.
Alias cost is ~5 lines and the lint rule nudges migration for free.

Verification

`pnpm --filter @cloudflare/kumo test -- --run table` → 3/3 new tests pass.
`pnpm --filter @cloudflare/kumo tsc --noEmit` → no Table-related type errors.

Checklist

Reviews
bonk has reviewed the change
automated review not possible because: waiting on bonk
Tests
Tests included/updated
Automated tests not possible - manual testing has been completed as follows:
Additional testing not necessary because:

Align Table.CheckCell and Table.CheckHead with the Checkbox component's signature by adding an `onCheckedChange` prop that takes an optional second argument exposing event details from the underlying Checkbox. The existing `onValueChange` prop is retained as a deprecated alias — it still works and continues to be called, so no consumer code needs to change immediately. The `no-deprecated-props` lint rule now flags `onValueChange` usage on these components, pointing migrators to the new name. This is a backwards-compatible change and ships as a minor bump. The deprecated alias will be removed in a future major version.

mattrothenberg · 2026-04-22T18:37:29Z

/bonk what say you?

pkg-pr-new · 2026-04-22T18:38:04Z

npm i https://pkg.pr.new/@cloudflare/kumo@437

commit: d9ab23c

ask-bonk

Review: LGTM ✅

This is a well-structured, backwards-compatible change that aligns Table.CheckCell and Table.CheckHead with the Checkbox component's signature.

What I verified:

Implementation (table.tsx):
- New onCheckedChange prop correctly accepts (checked: boolean, eventDetails?: CheckboxChangeEventDetails) => void
- Deprecated onValueChange retained with @deprecated JSDoc for lint rule integration
- Both handlers fire when set, enabling smooth migration
- Event details properly passed through from underlying Checkbox
Tests (table.test.tsx) - All 3 pass:
- ✅ New onCheckedChange prop works with event details
- ✅ Deprecated onValueChange still fires for backward compatibility
- ✅ Both props can coexist during migration
Documentation (table.mdx):
- Clear migration examples showing before/after
- Event details usage documented
- Deprecation notice appropriately placed
Demos (TableDemo.tsx):
- All checkbox demos migrated to new onCheckedChange prop
- Consistent pattern across TableWithCheckboxDemo, TableSelectedRowDemo, and TableFullDemo
Changeset:
- Correctly marked as minor bump for additive change
- Clear explanation of migration path

Code quality notes:

Clean implementation with proper TypeScript types
Follows the project's deprecation pattern (matches Checkbox, Select, Banner migrations)
displayName already set correctly for both components
Uses semantic token styling conventions appropriately

The unrelated test failures (missing component-registry.json) are pre-existing and not caused by this PR.

Approved for merge.

ask-bonk · 2026-04-22T18:39:09Z

I've completed my review of PR #437 and posted an approving review.

Summary

The PR adds onCheckedChange to Table.CheckCell and Table.CheckHead as a new prop, while keeping onValueChange as a deprecated alias for backward compatibility. This aligns the Table checkbox components with the Checkbox component's signature.

Key points verified:

✅ All 3 new table tests pass
✅ Implementation correctly passes event details through from the underlying Checkbox
✅ Backward compatibility maintained - both old and new props work
✅ Documentation updated with migration examples
✅ Demo code migrated to use the new prop
✅ Changeset correctly marks this as a minor (additive) change

The implementation follows Kumo's established deprecation patterns and the code is clean and well-tested. The unrelated test failures in the CI output are pre-existing issues with missing component-registry.json (auto-generated at build time), not caused by this PR.

github run