feat: fix residual md3 deviation - checkbox component - #5104
Open
likevy wants to merge 5 commits into
Open
Conversation
The state-layer tokens were declared but never read, so hover and focus had no tint of their own. The layer now follows the spec -- `primary` when selected, `onSurface` when not, `error` throughout an error checkbox, `color` and `uncheckedColor` standing in for the role they override on the box -- and fades by opacity alone, so a dynamic theme's `PlatformColor` roles are never interpolated. Press handlers are only attached when something can handle a press, since TouchableRipple keys its own disabled state off that.
MD3 paints the press as a ripple bounded to the 40dp state layer in the inverted role -- `onSurface` when selected, `primary` when not. The platform press could not deliver that: Android's ripple rejects the `PlatformColor` a dynamic theme resolves roles to, iOS and Android only offered the neutral default, and web's hover overlay doubled up with the layer. The checkbox now draws the ripple itself and holds it for a minimum press so a quick tap still reads. `rippleColor` or `underlayColor` hands the press back to the platform; `rippleEffectEnabled: false` disables it as everywhere else.
The pressable was sized to the 40dp state layer, 8dp short of the minimum interactive area, with no hitSlop making up the difference. Only the pressable grows; the 40dp layers it centres stay where they were.
The ring sat on the state-layer boundary because the pressable clips overflow to the tap-target shape and would crop the spec's outer offset. Rendered as a sibling of the pressable it takes the spec geometry, which also covers Android P+, where a foreground ripple forces the clip regardless of `borderless`.
A standalone checkbox renders no visible label, and `aria-label` reached it only by inheritance, so it appeared in no prop table and nothing flagged one that shipped unnamed. `Checkbox.Item` names the row and is exempt.
There was a problem hiding this comment.
🟡 Changes recommended
The implementation can change an active ripple’s color, double-render customized Android ripples, and break existing style layout semantics.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates Checkbox behavior to align with MD3 interaction, sizing, focus, and accessibility requirements.
Changes:
- Adds MD3 state-layer and custom ripple behavior.
- Expands touch targets and adjusts focus-ring geometry.
- Adds accessibility guidance and comprehensive tests.
File summaries
| File | Description |
|---|---|
src/components/Checkbox/utils.ts |
Resolves interaction colors and opacity. |
src/components/Checkbox/tokens.ts |
Adds sizing and state-layer tokens. |
src/components/Checkbox/Checkbox.tsx |
Implements interaction, focus, sizing, and accessibility behavior. |
src/components/__tests__/Checkbox/utils.test.tsx |
Tests state-layer resolution. |
src/components/__tests__/Checkbox/Checkbox.test.tsx |
Tests interactions, sizing, focus, and accessibility. |
src/components/__tests__/Checkbox/__snapshots__/CheckboxItem.test.tsx.snap |
Updates Checkbox.Item snapshots. |
src/components/__tests__/Checkbox/__snapshots__/Checkbox.test.tsx.snap |
Updates Checkbox snapshots. |
docs/6.x/docs/guides/migration.md |
Documents behavioral and sizing changes. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+217
to
+218
| const platformOwnsPress = | ||
| rest.rippleColor != null || rest.underlayColor != null; |
Comment on lines
+451
to
+453
| // The ring is a sibling of the pressable, not a child: a foreground ripple | ||
| // forces `overflow: hidden` on it regardless of `borderless`. | ||
| <View style={styles.root}> |
Comment on lines
+484
to
+487
| style={[ | ||
| styles.stateLayer, | ||
| { backgroundColor: pressRipple.color }, | ||
| rippleStyle, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Checkboxis one of the MD3 reference components, but a review against the spec found residual deviations: the state-layer tokens were dead code, so every interaction fell back toTouchableRipple's neutral ripple; the touch target was 40dp; the focus ring sat on the state-layer edge; and a standalone checkbox had no accessible-name contract.Things worth mentioning:
Checkbox.kt:ripple(bounded = false, radius = StateLayerSize / 2); material-web_checkbox.scss:md-ripplesized to the state layer).TouchableRipplecan't get there: Android's ripple only accepts a numeric colour, so a dynamic theme'sPlatformColorroles can never tint it; the ripple and iOS highlight fill the whole pressable, now 48dp; and on web its hover overlay doubles up with the layer. So the platform press is turned off (rippleColor: 'transparent', asSwitchandBottomNavigationalready do) and a 40dp ripple is drawn with Reanimated shared values — static colour, onlyopacity/transformanimate, 200 ms grow with a 200 ms minimum hold so a quick tap still reads (ComposeRippleAnimation, material-webMINIMUM_PRESS_MS). An explicitrippleColororunderlayColorhands the press back to the platform;rippleEffectEnabled: falsestill disables it.PlatformColorVisible behaviour changes, both in the migration guide: the checkbox occupies 48dp instead of 40dp, so rows get ~8dp taller; interaction tints follow selection instead of being neutral. A standalone
Checkboxwith no accessible name now warns.Related issue
Checkbox review checklist:
hitSlop); MD3 requires 48dp (the 40dp state layer itself already matches).primary/ unselectedonSurface; pressed: selectedonSurface/ unselectedprimary. The lib applies one colour per selection state across all three interaction states (src/components/Checkbox/tokens.ts:24-25), so hover/focus are correct and pressed is wrong in both directions. Do not simply swap the two constants. Error states already match (errorthroughout).Checkbox.Itemis the labeled wrapper) — document and guarantee the standalone accessible-name path.Test plan
yarn lint,yarn typecheckandyarn testpass — 738 tests, 168 snapshots. Each commit is independently green, so the series bisects cleanly.60 new tests in
Checkbox.test.tsxandutils.test.tsxcover the state-layer colour per interaction (custom colours and the pressed inversion included), the ripple on all threePlatform.OSvalues, the minimum-press hold, the fade-out duration, the scale reset on a second press, reduce motion, therippleColor/underlayColorescape hatches,rippleEffectEnabled: false,PlatformColorroles, the 48×48 target, the 50dp ring geometry and the accessible-name warning. Mutation-tested: removing the inversion, the hold, the scale reset, the fade duration or the custom-colour path each fails its test.Manual, on the Checkbox example screen:
primarywhen unchecked,onSurfacewhen checked. On Android with dynamic colour on it takes the system accent.errorripple. "Custom color (tertiary)" - tertiary ripple, noprimaryhalo.secondaryring with a 2dp gap around the 40dp layer; a mouse click must not light the ring.primary.aria-labelannounces its name and state;Checkbox.Itemannounces once for the row; one without a name logs the warning.Screen.Recording.2026-09-04.at.16.50.46.mov
Screen.Recording.2026-09-04.at.16.51.11.mov
Screen.Recording.2026-09-04.at.16.51.40.mov