Skip to content

refactor(v6): modernize Button API with label and iconPosition props#4943

Open
azizbecha wants to merge 11 commits into
callstack:v6from
azizbecha:@azizbecha/refactor/button
Open

refactor(v6): modernize Button API with label and iconPosition props#4943
azizbecha wants to merge 11 commits into
callstack:v6from
azizbecha:@azizbecha/refactor/button

Conversation

@azizbecha
Copy link
Copy Markdown
Collaborator

@azizbecha azizbecha commented May 12, 2026

Motivation

Button set its label through children, which coupled the internal layout to arbitrary child structures, made uppercase unreliable (it couldn't transform React elements), and forced an extra Text wrapper. Icon placement on the trailing edge relied on the undocumented contentStyle={{ 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:

  • Adds a string label prop as the primary way to set the button text. children keeps working as a deprecated fallback (when both are set, label wins) and emits a dev-only warning, so existing code is unaffected.
  • Adds an iconPosition?: 'leading' | 'trailing' prop. The previous contentStyle row-reverse approach still works but is deprecated (dev-only warning) — the icon-margin matrix is extracted into a getButtonIconStyle helper.
  • Adds a rippleColor?: ColorValue prop; by default the ripple / state layer now uses the label color at the pressed-state opacity per MD3 (instead of TouchableRipple's onSurface-based default), with a graceful fallback when the label color is a PlatformColor.
  • Wraps the expensive derived values (getButtonColors, border-radius extraction, ripple color, icon style, touchable-ripple style, flattened
    styles) in useMemo, memoizes the press handlers, and drops the isMode useCallback.
  • Migrates internal consumers (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 test all pass (snapshots updated where the iconPosition restructure changed the rendered tree; reviewed —
    the only diffs are a short-circuit false slot in the content style array, the icon-container style going from a 3-element array to an equivalent single object, and row-reverse moving into a styles.contentReverse).
  • New unit tests cover: label renders / takes precedence over children; deprecated children still renders and warns; iconPosition="trailing" and the legacy contentStyle fallback (+ warning); getButtonRippleColor (custom color, default = label color @ pressed opacity, PlatformColor → undefined).
  • Manual: open the Button example screen on iOS / Android / web — all five modes, leading vs trailing icon (both iconPosition and the legacy contentStyle path), loading spinner placement, disabled appearance, compact, custom radius, elevated press-elevation animation, and the ripple color matching the label color.

@azizbecha azizbecha requested a review from satya164 May 12, 2026 22:34
@callstack-bot
Copy link
Copy Markdown

callstack-bot commented May 12, 2026

Hey @azizbecha, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

Copy link
Copy Markdown
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@satya164
Copy link
Copy Markdown
Member

To clarify removal of children patterns that we discussed, it's about the usage of React.Children API - which breaks composition, not about children props.

azizbecha added 11 commits May 13, 2026 23:11
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 azizbecha force-pushed the @azizbecha/refactor/button branch from 4a849e5 to 1ae9094 Compare May 13, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants