refactor(v6): modernize Button API with label and iconPosition props#4943
Open
azizbecha wants to merge 11 commits into
Open
refactor(v6): modernize Button API with label and iconPosition props#4943azizbecha wants to merge 11 commits into
azizbecha wants to merge 11 commits into
Conversation
|
Hey @azizbecha, thank you for your pull request 🤗. The documentation from this branch can be viewed here. |
satya164
requested changes
May 13, 2026
Member
satya164
left a comment
satya164
left a comment
There was a problem hiding this comment.
Current API relies heavily on children composition in ways that are difficult to optimize
@ruben-rebelo can you clarify what the optimization problem is?
I'm against changing the API from children to label: string. It's practically the same except for the type, which limits usage of other components. I don't see the benefit of this restriction, and it's also a big breaking change as Button is a common component.
Member
|
To clarify removal of children patterns that we discussed, it's about the usage of |
Introduce a `label?: string` prop as the primary way to set the button text. The `children` prop keeps working as a deprecated fallback (when both are set, `label` wins) and emits a dev-only warning. This decouples the button layout from arbitrary child structures and makes `uppercase` work reliably, since the label is always a string.
Update the components that compose Button (Banner, Snackbar, DataTablePagination) to pass the new `label` prop instead of children, and update the `## Usage` / `@example` JSDoc blocks (and the test files) accordingly so nothing relies on the deprecated `children` prop.
Add an `iconPosition?: 'leading' | 'trailing'` prop to control where the
icon sits relative to the label. The previous approach of setting
`contentStyle={{ flexDirection: 'row-reverse' }}` still works but is now
deprecated and emits a dev-only warning.
The icon margins are extracted into a `getButtonIconStyle` helper,
replacing the previous matrix of computed StyleSheet keys, and
DataTablePagination is updated to use the new prop.
Add a `rippleColor?: ColorValue` prop and, by default, drive the ripple / state layer with the label color at the pressed-state opacity (per Material Design 3) instead of TouchableRipple's onSurface-based default. The color is computed by a new `getButtonRippleColor` helper, which falls back to `undefined` (TouchableRipple's own default) when the label color is not a plain string, e.g. an Android Material You PlatformColor.
Wrap the expensive derived values (color computation, border-radius extraction, ripple color, icon style, touchable ripple style, and the flattened style objects) in `useMemo`, memoize the press handlers with `useCallback`, and replace the `isMode` `useCallback` with a plain local function. No behavior or render-output change.
Update the example screens to use the new `label` prop instead of
children, and the `iconPosition="trailing"` prop instead of the
`contentStyle={{ flexDirection: 'row-reverse' }}` hack.
Update the hand-written guide snippets (icons, react-navigation, ripple effect) and the docs-site example components to use the new `label` prop instead of children. The generated component reference pages are derived from the JSDoc and will be regenerated by the docs build.
Add a `size?: 'extra-small' | 'small' | 'medium' | 'large' | 'extra-large'` prop. When omitted, the Button keeps its current visuals; when set, the per-size MD3 metrics (minHeight, horizontal padding, icon size, icon/label gap, label typescale) are applied via a new `getButtonSizeStyle` helper.
Add a `shape?: 'round' | 'square'` prop. When omitted, the button keeps its legacy corner radius. When set, `'round'` uses the full-pill radius and `'square'` uses a per-size smaller corner; the mapping comes from a new `getButtonShapeRadius` helper. An explicit `borderRadius` in `style` still wins.
Add a `selected?: boolean` prop. When `true`, the button flips its `shape` (round ↔ square) so the selected/unselected pair contrasts, and for `outlined`/`text` modes adopts a filled tonal-selected appearance (`secondaryContainer` background, `onSecondaryContainer` label, no border). `accessibilityState.selected` is set so screen readers announce the toggle state. Other modes keep their colors and only flip the shape. The `selected` flag is threaded through `getButtonColors` and its sub-helpers.
Showcase the new expressive props in the example app: one button per size in the Size section, a round and a square row across sizes in the Shape section, and stateful selected/unselected toggles in the Toggle section.
azizbecha
force-pushed
the
@azizbecha/refactor/button
branch
from
4a849e5 to
1ae9094
Compare
adrcotfas
requested changes
May 15, 2026
Collaborator
There was a problem hiding this comment.
Hey @azizbecha, thanks for the PR!
I've updated #4928 with a link to a component vs specs review we did internally. You'll find more details about Button there.
Some general comments:
- It would be a great moment to revamp the example page for Buttons with a way to toggle all possible states (sizes, configurations, shapes, toggle etc) in a more compact way; now it's huge and messy IMHO. This will also help a lot with testing the modernization.
- please make sure you're following the specs in terms of colors, spacing etc; For example, the outlined variant should use
onSurfaceVariantas label color, not primary; - use the same naming as in the specs (filled instead of contained, tonal etc)
- default mode is text now but specs hierarchy implies filled as default
- we're using the legacy icon size of 18dp when no size is set: "extra-small" should have the iconSize of 20, not 16.
- we're using the legacy horizontal padding of 24dp instead of 16dp
- it would be nice to extract all "component specific tokens" in the same place for easier access/update if needed;
- use the theme shape tokens instead of magic numbers in
BUTTON_SHAPE_RADIUS, for example here are the values for thesmallbutton variant; I can share the other values if you want to; observe that there's a separate field forpressed:
// extract from the native Jetpack Compose library
internal object ButtonSmallTokens {
val ContainerHeight = 40.0.dp
val ContainerShapeRound = ShapeKeyTokens.CornerFull
val ContainerShapeSquare = ShapeKeyTokens.CornerMedium
val IconLabelSpace = 8.0.dp
val IconSize = 20.0.dp
val LeadingSpace = 16.0.dp
val OutlinedOutlineWidth = 1.0.dp
val PressedContainerShape = ShapeKeyTokens.CornerSmall
val SelectedContainerShapeRound = ShapeKeyTokens.CornerFull
val SelectedContainerShapeSquare = ShapeKeyTokens.CornerMedium
val TrailingSpace = 16.0.dp
}- I'm glad you added shapes but we're missing the shape morph animation on press and on toggle; we have motion tokens for that BTW
- drop the
md3prefix from styles - make sure we respect "Extra small and small icon buttons must have a target size of 48x48dp or larger to be accessible." as mentioned in the specs
- we should make use of
useLocale()for toggling the icon position - consolidate the naming; we're currently using
text(textColor, textOpacity etc) andlabelto refer to the same thing; Let's use label as in the MD specs - @satya164 do you think we should look into
hovered/focusedstates too? something relevant for screen readers, pointing device attached to device - It would be nice to consolidate all buttons under src/components/Buttons; See https://m3.material.io/components/all-buttons; Also, I believe we could reuse code between button types as they have a lot in common
Member
yes. we also support web where this is much more important. |
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
4 participants
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
Buttonset its label throughchildren, which coupled the internal layout to arbitrary child structures, madeuppercaseunreliable (it couldn't transform React elements), and forced an extraTextwrapper. Icon placement on the trailing edge relied on the undocumentedcontentStyle={{ flexDirection: 'row-reverse' }}hack, the ripple/state-layer color didn't follow the label color as MD3 specifies, and the render derived a number of style objects on every render.This PR (part of the v6 work) modernizes the Button API and rendering without removing anything:
labelprop as the primary way to set the button text.childrenkeeps working as a deprecated fallback (when both are set,labelwins) and emits a dev-only warning, so existing code is unaffected.iconPosition?: 'leading' | 'trailing'prop. The previouscontentStylerow-reverse approach still works but is deprecated (dev-only warning) — the icon-margin matrix is extracted into agetButtonIconStylehelper.rippleColor?: ColorValueprop; by default the ripple / state layer now uses the label color at the pressed-state opacity per MD3 (instead ofTouchableRipple'sonSurface-based default), with a graceful fallback when the label color is aPlatformColor.getButtonColors, border-radius extraction, ripple color, icon style, touchable-ripple style, flattenedstyles) in
useMemo, memoizes the press handlers, and drops theisMode useCallback.Banner,Snackbar,DataTablePagination), the example app, and the guide/docs-site snippets to the new props.Migration:
<Button>Text</Button>→<Button label="Text" />;contentStyle={{ flexDirection: 'row-reverse' }}→iconPosition="trailing".Related issue
closes #4928
Test plan
yarn typescript,yarn lint,yarn testall pass (snapshots updated where theiconPositionrestructure changed the rendered tree; reviewed —the only diffs are a short-circuit
falseslot in the content style array, the icon-container style going from a 3-element array to an equivalent single object, androw-reversemoving into astyles.contentReverse).labelrenders / takes precedence overchildren; deprecated children still renders and warns;iconPosition="trailing"and the legacycontentStylefallback (+ warning);getButtonRippleColor(custom color, default = label color @ pressed opacity,PlatformColor→ undefined).iconPositionand the legacycontentStylepath), loading spinner placement, disabled appearance, compact, custom radius, elevated press-elevation animation, and the ripple color matching the label color.