Repository navigation
Conversation
|
can you rebase the PR against main? |
There was a problem hiding this comment.
Pull request overview
This PR modernizes RadioButton for Material Design 3 by replacing the prior platform-specific implementations with a single unified component, introducing MD3 token-based sizing/color resolution, and aligning behavior (animation, error/disabled states, a11y, and group performance) with the library’s other MD3 selection controls.
Changes:
- Unifies
RadioButtoninto a single MD3 implementation (removingRadioButton.Android,RadioButton.IOS, and theRadioButton.Itemmodeprop) and migrates selection animation toreact-native-reanimated. - Adds MD3 radio tokens +
errorsupport, applies disabled opacity consistently, and updates selection-control color resolution to use tokenized theme roles. - Improves
RadioButton.Groupcontext stability and updates tests/examples/docs accordingly (including removing platform-specific docs pages).
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.tsx | Removes exported platform-specific RadioButton prop types. |
| src/components/RadioButton/utils.ts | Updates selection-control color resolution to use RadioButton tokens and removes iOS-specific helper. |
| src/components/RadioButton/tokens.ts | Introduces MD3 radio sizing + color-role tokens. |
| src/components/RadioButton/RadioButtonItem.tsx | Removes mode, adds error, switches to useContext, and adjusts a11y/testID behavior. |
| src/components/RadioButton/RadioButtonGroup.tsx | Memoizes context value and stabilizes onValueChange with use-latest-callback. |
| src/components/RadioButton/RadioButtonAndroid.tsx | Deletes old Android-specific implementation. |
| src/components/RadioButton/RadioButtonIOS.tsx | Deletes old iOS-specific implementation. |
| src/components/RadioButton/RadioButton.tsx | Implements unified MD3 RadioButton with Reanimated dot-scale animation, tokenized sizes/colors, and updated a11y behavior. |
| src/components/RadioButton/index.ts | Removes .Android/.IOS static exports from RadioButton. |
| src/components/tests/RadioButton/utils.test.tsx | Updates tests to cover getSelectionControlColor including error/checked/unchecked cases. |
| src/components/tests/RadioButton/RadioButtonItem.test.tsx | Removes platform-mode tests and adds a11y single-node assertion. |
| src/components/tests/RadioButton/RadioButtonGroup.test.tsx | Adds test asserting stable context object across unrelated parent rerenders. |
| src/components/tests/RadioButton/RadioButton.test.tsx | Simplifies snapshots to single unified RadioButton implementation. |
| src/components/tests/RadioButton/snapshots/RadioButtonItem.test.tsx.snap | Updates snapshots for new unified control + a11y changes. |
| src/components/tests/RadioButton/snapshots/RadioButtonGroup.test.tsx.snap | Updates snapshots for new unified control rendering. |
| src/components/tests/RadioButton/snapshots/RadioButton.test.tsx.snap | Updates snapshots for unified control rendering. |
| example/src/Examples/RadioButtonItemExample.tsx | Updates example to remove mode variants and demonstrate error/disabled states. |
| example/src/Examples/RadioButtonGroupExample.tsx | Updates example usage to RadioButton (no platform statics). |
| example/src/Examples/RadioButtonExample.tsx | Updates example states, adds error demos, and adjusts disabled label styling. |
| docs/versioned_docs/version-6.x/components/RadioButton/RadioButtonItem.mdx | Removes documentation for the deleted mode prop. |
| docs/versioned_docs/version-6.x/components/RadioButton/RadioButtonIOS.mdx | Removes iOS-specific docs page. |
| docs/versioned_docs/version-6.x/components/RadioButton/RadioButtonAndroid.mdx | Removes Android-specific docs page. |
| docs/docusaurus.config.js | Removes nav entries for the deleted platform-specific docs pages. |
| <Text | ||
| variant={labelVariant} | ||
| testID={`${testID}-text`} | ||
| style={[styles.label, computedStyle, labelStyle]} | ||
| maxFontSizeMultiplier={labelMaxFontSizeMultiplier} | ||
| > |
| /** | ||
| * Function to execute on press. | ||
| */ | ||
| onPress?: (e: GestureResponderEvent) => void; | ||
| onPress?: (param?: any) => void; | ||
| /** |
| return ( | ||
| <TouchableRipple | ||
| {...rest} | ||
| borderless | ||
| onPress={(event) => { | ||
| handlePress({ | ||
| onPress, | ||
| onValueChange: context?.onValueChange, | ||
| value, | ||
| event, | ||
| }); | ||
| }} | ||
| disabled={disabled} | ||
| {...accessibilityProps} | ||
| style={styles.container} | ||
| testID={testID} | ||
| theme={theme} | ||
| > |
cb74fc8 to
c0e1c18
Compare
- Fix accessibilityLabel variable reference (ariaLabel, not accessibilityLabel) - Make sync render() calls async in RadioButton and RadioButtonItem tests - Await rerender() in RadioButtonGroup stability test - Use screen.queryAllByRole instead of destructuring from render result - Update snapshots to reflect accessibilityRole instead of role prop
Nested ternary indentation left misaligned after conflict resolution.
RadioButtonAndroid and RadioButtonIOS were removed but the navigation index still referenced them, breaking the docs build.
RadioButtonAndroid and RadioButtonIOS source mappings caused the docs build to fail with ENOENT since the files no longer exist.
- Restore onPress type to (e: GestureResponderEvent) => void to match TouchableRipple - Merge consumer style with container style, handling function-style props - Guard RadioButtonItem label testID to avoid "undefined-text" when testID is not provided
| // The outer TouchableRipple is the interactable element + a11y radio; the | ||
| // inner control is purely visual, so exclude it from the a11y tree to | ||
| // avoid duplicate `checked` states. | ||
| const radioButton = <RadioButton {...radioButtonProps} accessible={false} />; |
There was a problem hiding this comment.
what about preventing visual-only radio from receiving keyboard focus?
accessible={false} removes its accessibility semantics, but updated snapshot still shows focusable={true} (source)
additionally, react-native-web defaults enabled Pressable to tabIndex={0} (RNW sourcecode), leaving unlabeled second tab stop inside RadioButton.Item
so what about this change?
| const radioButton = <RadioButton {...radioButtonProps} accessible={false} />; | |
| const radioButton = <RadioButton {...radioButtonProps} accessible={false} focusable={false} tabIndex={-1} />; |
| const sizes = { | ||
| ringSize: 20, | ||
| dotSize: 10, | ||
| outlineWidth: 2, | ||
| } as const; |
There was a problem hiding this comment.
what about adding an explicit state-layer size token & sizing touchable accordingly?
current 20dp ring with 8dp margins produces 36dp state layer, while MD3 defines radio state layer as 40dp.
modernized Checkbox component already uses the same 40dp state-layer token (source)
so could we change it to:
| const sizes = { | |
| ringSize: 20, | |
| dotSize: 10, | |
| outlineWidth: 2, | |
| } as const; | |
| const sizes = { | |
| ringSize: 20, | |
| dotSize: 10, | |
| outlineWidth: 2, | |
| stateLayerSize: 40, | |
| } as const; |
and use like that in RadioButton.tsx:
const {
ringSize,
dotSize,
outlineWidth: OUTLINE_WIDTH,
stateLayerSize,
} = RadioButtonTokens;
const styles = StyleSheet.create({
container: {
width: stateLayerSize,
height: stateLayerSize,
borderRadius: stateLayerSize / 2,
alignItems: 'center',
justifyContent: 'center',
},
radio: {
height: ringSize,
width: ringSize,
borderRadius: ringSize / 2,
borderWidth: OUTLINE_WIDTH,
},
});
…into feat/radio-button-md3 # Conflicts: # src/components/RadioButton/RadioButtonAndroid.tsx # src/components/RadioButton/RadioButtonIOS.tsx # src/components/RadioButton/RadioButtonItem.tsx
MikitasK
left a comment
MikitasK
left a comment
There was a problem hiding this comment.
- updated radio-button state layer to MD3 specified 40dp size while keeping visible ring centered at 20dp
- added reusable
stateLayerSizetoken & removed previous margin-based state-layer sizing - made nested visual control inside
RadioButton.Itemnon-focusable withfocusable={false}andtabIndex={-1}keeping outer item as the only interactive radio - added regression coverage for
40dpstate layer & updated affected accessibility & component snapshots
| const accessibilityProps = | ||
| rest.accessible === false | ||
| ? {} | ||
| : { | ||
| accessibilityRole: 'radio' as const, | ||
| accessibilityState: { disabled: !!disabled, checked }, | ||
| }; |
There was a problem hiding this comment.
- accessibilityRole: 'radio' as const,
- accessibilityState: { disabled: !!disabled, checked },
+ role: 'radio' as const,
+ 'aria-disabled': !!disabled,
+ 'aria-checked': checked,466d9c608 (#5005) moved the library to aria-* and src/ on main has none of these props left. Same revert at RadioButtonItem.tsx:207-212 (accessibilityLabel / Role / State) and RadioButtonGroup.tsx:69 (accessibilityRole="radiogroup"; main has role). Checkbox.tsx:249-256 is the shape to copy under the identical rest.accessible === false guard, and CheckboxItem.tsx:157-160 for the row. Snapshots need regenerating after.
| {checked ? ( | ||
| <View style={[StyleSheet.absoluteFill, styles.radioContainer]}> | ||
| <Animated.View | ||
| style={[ | ||
| styles.dot, | ||
| { backgroundColor: selectionControlColor }, | ||
| dotAnimatedStyle, | ||
| ]} | ||
| /> | ||
| </View> | ||
| ) : null} |
There was a problem hiding this comment.
The StyleSheet.absoluteFill + radioContainer wrapper only existed because the old code animated borderWidth. The outline is a constant now, so it can go: put alignItems: 'center' / justifyContent: 'center' on styles.radio, render the Animated.View dot directly as its child, and drop styles.radioContainer.
|
@MikitasK is this ready for another review? |
JKobrynski
left a comment
JKobrynski
left a comment
There was a problem hiding this comment.
Add a ### RadioButton entry to docs/6.x/docs/guides/migration.md. The iOS variant drew a checkmark, so anyone on it gets a visibly different control. #5059 has a ### Checkbox section covering the identical removals that would work as the template, but it skips RadioButton and still lists RadioButton.Android / RadioButton.IOS as live. Worth syncing with @mikolajadamowicz on which PR carries it.
done ✅ added |
JKobrynski
left a comment
JKobrynski
left a comment
There was a problem hiding this comment.
We've got a bunch of conflicts
|
|
||
| const styles = StyleSheet.create({ | ||
| container: { | ||
| width: stateLayerSize, |
There was a problem hiding this comment.
The spec's Measurements table lists a 48dp target next to the 40dp state layer, and the same page warns against dropping targets below 48x48. This sizes the whole touchable to 40. Would hitSlop cover the difference? Checkbox has the same 40dp target, so it may be one change for both.
…into feat/radio-button-md3 Conflicts resolved: - RadioButton.tsx: kept the unified MD3 implementation; adopted the upstream utility-type cleanup (`Omit<React.PropsWithoutRef< TouchableRippleProps>, 'children'>` instead of the removed `$RemoveChildren`) and the `ThemeProp` move to `theme/types`. - RadioButtonAndroid.tsx / RadioButtonIOS.tsx: stay deleted. - docs/6.x RadioButton mdx + _meta.json: stay deleted, superseded by the upstream generated-docs pipeline. - migration.md: kept both the upstream v6 sections and the RadioButton one.
The MD3 state layer for both controls is 40dp, but the same spec page requires touch targets of at least 48x48dp. Sizing the pressable to the state layer left an 8dp shortfall. Add a shared `getMinTouchTargetHitSlop` helper and apply it to both controls, so the target grows to 48dp while the visual bounds stay at 40dp. It is passed before the prop spread, so a caller-supplied `hitSlop` still wins. Also drop the now-dead `RadioButton.Android` / `RadioButton.IOS` screenshot entries and collapse the platform-split `RadioButton` ones, since the component renders the same control on every platform.
| * Note that on Android a child's `hitSlop` cannot extend past its parent's | ||
| * bounds, so a control placed in a tightly clipped container may still | ||
| * receive a smaller effective target. |
There was a problem hiding this comment.
I'm pretty sure this doesn't change much on the web. AFAIK react-native-web's Pressable never reads hitSlop. So the touchable should probably have padding on the web instead, or at least a note next to this Android caveat. Wdyt?
There was a problem hiding this comment.
nice catch!
documented this limitation next to Android caveat & noted that web consumers should use wrapper padding when 48dp target is required
updated |
|
Review by Claude The press and hover state layer covers the whole 48dp touch target. The MD3 radio state layer is 40dp, and the new 40dp iOSSteps: open Radio Button, press and hold the "Error (Checked)" radio.
(via Claude, verify if it makes sense) AndroidSteps: open Radio Button, press and hold the "Error (Checked)" radio.
(via Claude, verify if it makes sense) WebSteps: open
(via Claude, verify if it makes sense) |
satya164
left a comment
satya164
left a comment
There was a problem hiding this comment.
Review by Claude
See the inline comments. (via Claude, verify if it makes sense)
| // Single selection animation path: the dot scales in (with a slight | ||
| // overshoot) when the radio becomes checked. The ring outline stays a | ||
| // constant width. Keyed on `checked` (not `status`) so radios driven by a | ||
| // `RadioButton.Group` animate too. | ||
| const dotScale = useSharedValue(1); | ||
| const isFirstRendering = React.useRef<boolean>(true); | ||
|
|
||
| const Button = Platform.select({ | ||
| default: RadioButtonAndroid, | ||
| ios: RadioButtonIOS, | ||
| }); | ||
| const dotTimingConfig = React.useMemo( | ||
| () => ({ | ||
| duration: theme.motion.duration.short3, | ||
| easing: Easing.bezier(...theme.motion.easing.standard), | ||
| reduceMotion: reanimatedReduceMotion, | ||
| }), | ||
| [ | ||
| theme.motion.duration.short3, | ||
| theme.motion.easing.standard, | ||
| reanimatedReduceMotion, | ||
| ] | ||
| ); | ||
|
|
||
| return <Button {...props} theme={theme} />; | ||
| React.useEffect(() => { | ||
| // Do not run animation on very first rendering | ||
| if (isFirstRendering.current) { | ||
| isFirstRendering.current = false; | ||
| return; | ||
| } | ||
|
|
||
| if (checked) { | ||
| // Jump to the overshoot value, then settle back to 1. | ||
| dotScale.value = 1.2; | ||
| dotScale.value = withTiming(1, dotTimingConfig); | ||
| } | ||
| }, [checked, dotScale, dotTimingConfig]); | ||
|
|
||
| const dotAnimatedStyle = useAnimatedStyle(() => ({ | ||
| transform: [{ scale: dotScale.value }], | ||
| })); |
There was a problem hiding this comment.
The dot scale is a transition between the two values of checked, so it can use Reanimated CSS transitions like Checkbox and Badge on main, without a shared value, a ref and an effect. Right now the dot unmounts on deselect with no animation, and on select it appears at 1.2 and shrinks to 1, so it never scales in from 0.
Keep the dot mounted and transition transform. This removes dotScale, isFirstRendering, dotTimingConfig, the effect and dotAnimatedStyle:
import Animated, { cubicBezier, type CSSStyle } from 'react-native-reanimated';
const dotStyle: CSSStyle<ViewStyle> = {
backgroundColor: selectionControlColor,
transform: [{ scale: checked ? 1 : 0 }],
transitionProperty: 'transform',
transitionDuration: reduceMotion ? 0 : theme.motion.duration.short3,
transitionTimingFunction: cubicBezier(...theme.motion.easing.standard),
};
// inside styles.radio, in place of the conditional dot
<Animated.View style={[styles.dot, dotStyle]} />CSS transitions do not run on mount, so the first render needs no special case. The dot then grows on select and shrinks on deselect, and reduced motion makes both instant. (via Claude, verify if it makes sense)
| testID={testID} | ||
| theme={theme} | ||
| > | ||
| <View pointerEvents="none" style={styles.stateLayer}> |
There was a problem hiding this comment.
This 40dp view has no color, so it does not change anything on screen. The Android ripple, the iOS highlight and the web hover overlay all come from the borderless TouchableRipple, which is now the 48dp container. The visible state layer is 48dp, and the MD3 spec asks for 40dp. Paint the hover and press layers on this 40dp view and turn off the platform press on the outer touchable, as #5104 does for Checkbox.
(via Claude, verify if it makes sense)
|
|
||
| `RadioButton` renders one Material 3 circular radio control on every platform. The platform-specific components and their prop types are gone. On iOS checked controls no longer render a checkmark; they use the same ring-and-dot indicator as other platforms. `RadioButton` now also forwards `TouchableRipple` props (`accessible`, `onFocus`, `onBlur`, `borderless`, `rippleColor`, ...). | ||
|
|
||
| The control now occupies a 48×48dp layout box instead of 40×40dp, so rows containing it may become 8dp taller. The 20dp radio indicator is unchanged. The minimum touch target is part of the layout on every platform, rather than an invisible area added with `hitSlop`. |
There was a problem hiding this comment.
v5 RadioButton.Android is 36dp (a 20dp ring with an 8dp margin) and RadioButton.IOS is 36dp (a 24dp icon with 6dp padding). The box grows by 12dp, not 8dp, and a RadioButton.Item row goes from 52dp to 64dp. The hitSlop sentence describes an earlier revision of this PR. v5 never added hitSlop, so it gives readers nothing to compare with.
| The control now occupies a 48×48dp layout box instead of 40×40dp, so rows containing it may become 8dp taller. The 20dp radio indicator is unchanged. The minimum touch target is part of the layout on every platform, rather than an invisible area added with `hitSlop`. | |
| The control now occupies a 48×48dp layout box instead of 36×36dp, so rows containing it may become 12dp taller. The 20dp radio indicator is unchanged. |
(via Claude, verify if it makes sense)
| Android: 'screenshots/radio-button-group-android.gif', | ||
| iOS: 'screenshots/radio-button-group-ios.gif', | ||
| }, | ||
| 'RadioButton.IOS': { | ||
| enabled: 'screenshots/radio-enabled.ios.png', | ||
| disabled: 'screenshots/radio-disabled.ios.png', | ||
| }, | ||
| 'RadioButton.Item': 'screenshots/radio-item.ios.png', |
There was a problem hiding this comment.
RadioButton.Item still uses radio-item.ios.png and RadioButton.Group still uses radio-button-group-ios.gif. Both show the iOS checkmark that this PR removes, so the 6.x docs pages show a control that no longer exists. Replace them with screenshots of the new control, or remove the iOS entries. (via Claude, verify if it makes sense)
| it('keeps a stable context value when an unrelated parent prop changes', async () => { | ||
| const contexts: Array<RadioButtonContextType | null> = []; | ||
|
|
||
| const Capture = ({ extra }: { extra: number }) => { | ||
| contexts.push(React.useContext(RadioButtonContext)); | ||
| return <RadioButton value={`v${extra}`} />; | ||
| }; | ||
|
|
||
| // New inline `onValueChange` on every render — `use-latest-callback` + | ||
| // `useMemo` should keep the provided context object identical. | ||
| const Parent = ({ extra }: { extra: number }) => ( | ||
| <RadioButton.Group value="first" onValueChange={() => {}}> | ||
| <Capture extra={extra} /> | ||
| </RadioButton.Group> | ||
| ); | ||
|
|
||
| const { rerender } = await render(<Parent extra={1} />); | ||
| await rerender(<Parent extra={2} />); | ||
|
|
||
| expect(contexts.length).toBe(2); | ||
| expect(contexts[0]).not.toBeNull(); | ||
| expect(contexts[0]).toBe(contexts[1]); | ||
| }); |
There was a problem hiding this comment.
This checks the identity of the context object, which is an internal detail. The risk of memoizing the context is a stale onValueChange, and a test through the public API covers that:
it('calls the latest onValueChange after the parent re-renders', async () => {
const first = jest.fn();
const second = jest.fn();
const { rerender } = await render(
<RadioButton.Group value="first" onValueChange={first}>
<RadioButton.Item label="Second" value="second" />
</RadioButton.Group>
);
await rerender(
<RadioButton.Group value="first" onValueChange={second}>
<RadioButton.Item label="Second" value="second" />
</RadioButton.Group>
);
await userEvent.press(screen.getByRole('radio', { name: 'Second' }));
expect(first).not.toHaveBeenCalled();
expect(second).toHaveBeenCalledWith('second');
});It needs jest from @jest/globals and screen and userEvent from test-utils. (via Claude, verify if it makes sense)
Motivation
RadioButtonstill shipped as two platform-specific implementations (RadioButtonAndroid,RadioButtonIOS) with the iOS variant rendering a checkmark glyph rather than a Material radio. This is inconsistent with the rest of the v6 MD3 work (Checkbox, Switch, TextInput, FAB) and diverges from the M3 radio button spec.This PR unifies RadioButton into a single MD3 component that mirrors the modernized Checkbox structure (single component,
tokens.ts,errorprop,accessible={false}inner control), without pulling in Checkbox's reanimated/focus-ring machinery (out of scope — radio ships with none today).Related issue
Closes #4938
What changed
RadioButtonAndroid+RadioButtonIOSinto a single MD3RadioButton(Android impl as base). DroppedRadioButton.Android,RadioButton.IOS,RadioButtonIOS, and theRadioButton.Itemmodeprop.RadioButton/tokens.ts(MD3 dims20/10/2+ color roles); colors resolved via tokens inutils.errorprop — added toRadioButtonandRadioButton.Item(ring + dot usetheme.colors.error;disabled/custom colors take precedence).Animated.Values into one shared value and migrated toreact-native-reanimated(matching Checkbox): dot scales in with overshoot on selection, keyed oncheckedso it also animates inside aRadioButton.Group. Usestheme.motionMD3 duration/easing tokens and respects reduce-motion.selectionControlOpacitywas previously computed but never applied).onValueChangewithuse-latest-callback; consumers switched fromContext.ConsumertouseContext.accessible={false}+ containerimportantForAccessibility="no-hide-descendants", so a screen reader sees one radio per row; dropped per-radioaccessibilityLiveRegion;labelMaxFontSizeMultiplier=1.5+${testID}-textfor CheckboxItem parity.RadioButtonAndroid/RadioButtonIOSdoc pages.Breaking changes
RadioButton.Android,RadioButton.IOS,RadioButtonIOS(andRadioButtonAndroidProps/RadioButtonIOSPropsexports).RadioButton.Itemmodeprop.errorprop added (no visual change for existing usage).Test plan
yarn typescript,yarn lint,yarn test(snapshots updated) — green.yarn --cwd docs build— green.Videos
radio_ios.mp4
radio_android.mp4
radio_web.mp4