refactor: replace React.Children usage with context and typed props - #5076
Draft
adam-sajko wants to merge 16 commits into
Draft
refactor: replace React.Children usage with context and typed props#5076adam-sajko wants to merge 16 commits into
adam-sajko wants to merge 16 commits into
Conversation
…th context List.Accordion used React.Children.map + cloneElement to inject paddingLeft and theme into expanded children, which makes composition harder (children had to be direct, prop-mergeable elements). Replace it with ListAccordionContext: the accordion exposes whether descendants should indent (leftIndent), and List.Item consumes it, applying the indent to its own container so the ripple/background stays full-width. Behavior preserved for List.Item children; theme now flows via context. Adds characterization tests. Refs callstack#4989.
Dialog cloned its children with React.Children + cloneElement to inject the theme, and DialogActions inspected the child count/index to space the first and last action. Both couple the components to the exact shape of their children. Remove the cloning: Dialog.Title/Content/Actions/Icon resolve the theme themselves via useInternalTheme, and DialogActions spaces its actions with a container `gap` instead of per-child injection. BREAKING CHANGE: Dialog no longer clones its children to forward an injected theme. The subcomponents read the theme from context (PaperProvider) or an explicit `theme` prop, so pass `theme` directly to a Dialog subcomponent if you previously relied on Dialog forwarding it. Wrapping or conditionally rendering Dialog.Actions children no longer affects their spacing. Refs callstack#4989
Card used React.Children.count/map + cloneElement to inspect sibling position and inject props (the theme, and top padding for the first child) into Card.Content/Cover/Title/Actions. That makes composition fragile: children had to be direct, recognised elements. Render the children directly instead and let each subcomponent own its own theme resolution and padding. BREAKING CHANGE: Card no longer clones its children to inject a theme or sibling-derived padding. Card.Content/Cover/Title/Actions read the theme from context or an explicit `theme` prop and apply their own spacing, so wrapping or reordering them keeps working; pass `theme` directly if you previously relied on Card forwarding it. Refs callstack#4989
ToggleButton.Row used React.Children.count/map + cloneElement to inject a first/middle/last border radius into each button by position, which only works when the buttons are direct children. Replace it with ToggleButtonRowContext: the row flags its descendants as segmented, each ToggleButton reads the flag and renders the flat segment shape itself, and the row clips a single rounded container with hairline dividers between segments. BREAKING CHANGE: ToggleButton.Row no longer clones its children to inject per-segment border radii. Segment styling now comes from ToggleButtonRowContext, so the buttons may be wrapped or conditionally rendered and still pick up the segmented appearance. Refs callstack#4989
Use the MD3 segmented-button shape token (corner.largeIncreased) for the row container and secondaryContainer for the selected segment, matching the existing SegmentedButtons component. Full per-segment alignment (first/middle/last radius as in SegmentedButtons) is intentionally NOT done: it would require positional child inspection, the React.Children anti-pattern this refactor removes. The container-clip + divider approach keeps the segmented look composition-friendly. Refs callstack#4989
Replace the `cloneElement` prop injection and `displayName`-based filtering in Appbar with an `AppbarContext` that exposes the shared `isDark` and `mode` values. `Appbar.Action`, `Appbar.BackAction` and `Appbar.Content` now read those values from context and derive their own foreground color, instead of having `color`/`mode`/`theme` spliced into them. - `small` and `center-aligned` render their children directly in author order; the title's `flex: 1` keeps trailing actions right-aligned and the center-aligned title centers itself. - `medium` and `large` keep the two-row layout (controls row above a full-width title) by partitioning children for placement only — no props are injected. - Drops the `React.Children.forEach` count heuristic that conditionally centered the title in `center-aligned`. BREAKING CHANGE: Appbar children are now rendered in the order they are written rather than being reordered to put the back action first. Place `Appbar.BackAction` before `Appbar.Content` and trailing `Appbar.Action`s after it, as shown in the docs. Children no longer receive injected `color`/`mode`/`theme` props; wrapping `Appbar.Content`/`Appbar.Action` in another element keeps working because the values come from context.
The composition refactor's job is to remove React.Children/cloneElement
without changing what users see. A separate commit on this branch
("align segmented row to MD3 spec") went further: it changed
ToggleButton.Row's corner radius (extraSmall -> largeIncreased) and the
selected segment's fill (own color -> secondaryContainer), converging it
toward the MD3 SegmentedButtons component.
That redesign was scope creep. ToggleButton.Row was never styled to the
MD3 SegmentedButtons spec upstream (it used surfaceContainerHighest /
extraSmall corners, distinct from SegmentedButtons' secondaryContainer /
largeIncreased) -- it's a separate, legacy component identity, not an
under-spec implementation of SegmentedButtons. The acceptance criterion
that triggered the MD3 pass ("double-check any affected component still
follows the specs") is a non-regression check, not a redesign mandate.
Revert ToggleButton.Row's corner back to theme.shapes.corner.extraSmall
and ToggleButton's selected fill back to the unconditional
getToggleButtonColor(...) (no row-specific override), matching the
original upstream component exactly. The composition-safe technique from
the prior commit (ToggleButtonRowContext, container-clip + gap divider)
is untouched -- this only reverts the two MD3-redesign value changes on
top of it, restoring a visually transparent composition refactor.
Verified: tsc clean, eslint clean, ToggleButton suite 8/9 (the 1 failure
is the pre-existing baseline animation test, unchanged). Pixel diff
against the prior B+ screenshot confirms the change is real and scoped
to exactly the Row control (4156px, corner + selected-fill only) with
zero diff elsewhere on screen.
…factor - DialogActions: destructure `theme`/`style`/`children` and forward only the remaining ViewProps to the native View, so the custom `theme` prop is not spread onto it (avoids react-native-web warnings and prop leakage). - ToggleButton.Row: apply the `style` prop to the segmented row container instead of an extra wrapper View, so callers can override the row's background, border radius and padding again. - ToggleButton.Row: replace the flexbox `gap` divider with a per-segment hairline `marginLeft` (dropping the row's left padding), so segment dividers render on React Native versions without `gap` support; the peer dependency is `react-native: "*"`. Refs callstack#4989
CardActions/DialogActions still used flexbox `gap` for inter-action spacing, the same RN-version-unsafe pattern already flagged and fixed in ToggleButtonRow. Since react-native is declared as a `*` peer dependency, `gap` silently drops on older RN versions, removing spacing between actions. Replace with explicit layout-only spacer Views rendered between children — no gap dependency, no prop injection into the action elements. Appbar's medium/large title-row partition compared `child.type === AppbarContent` directly, so a React.memo-wrapped Appbar.Content was never recognized and landed in the controls row instead of the title row — contradicting the composition goal that wrapped children keep working. Unwrap memo's `.type` before comparing. Also fixes two existing Card/Dialog tests that asserted the styled `gap` value directly and indexed action children positionally; they now assert via testID, decoupled from spacer siblings changing child count.
adam-sajko
changed the title
4 tasks
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
2 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.
Continues #5018, rebased on
mainwith review feedback applied. Related to #4989.