feat: implement Toolbar component - #5043
Conversation
MikitasK
left a comment
There was a problem hiding this comment.
nice work! 👍 overall, this looks like a solid foundation for the new Toolbar components
just a few points to address before merge:
| React.Children.map(children, (child) => { | ||
| if (!React.isValidElement<RecolorableProps>(child)) { | ||
| return child; | ||
| } | ||
|
|
||
| // `React.Children.map` doesn't flatten a `Fragment`, so recurse into | ||
| // it manually. | ||
| if (child.type === React.Fragment) { | ||
| return React.cloneElement( | ||
| child, | ||
| undefined, | ||
| recolorChildren(child.props.children, theme, colorScheme) | ||
| ); | ||
| } | ||
|
|
||
| if (child.type === IconButton) { | ||
| // A `mode` or explicit color prop means it already has its own | ||
| // spec-defined coloring. | ||
| if ( | ||
| child.props.mode != null || | ||
| child.props.iconColor != null || | ||
| child.props.containerColor != null | ||
| ) { | ||
| return child; | ||
| } | ||
|
|
||
| const { iconColor, containerColor } = resolveIconColors({ | ||
| theme, | ||
| colorScheme, | ||
| selected: child.props.selected ?? false, | ||
| }); | ||
| return React.cloneElement(child, { | ||
| iconColor, | ||
| containerColor, | ||
| ...(child.props.selected ? { mode: 'contained-tonal' } : null), | ||
| }); | ||
| } | ||
|
|
||
| if (child.type === Button) { | ||
| // `text` is `Button`'s mode-less default; any other mode (its own | ||
| // spec-defined coloring) or an explicit color prop opts it out. | ||
| if ( | ||
| (child.props.mode != null && child.props.mode !== 'text') || | ||
| child.props.textColor != null || | ||
| child.props.buttonColor != null | ||
| ) { | ||
| return child; | ||
| } | ||
|
|
||
| const textColor = resolveLabelColor({ theme, colorScheme }); | ||
| return React.cloneElement(child, { textColor }); |
There was a problem hiding this comment.
React.Children and cloneElement are both off-limits in v6 — see #4989, which is open specifically to unwind them where they hurt composition, with the FAB Menu migration in #4963 as the reference. The occurrences still in src/ (Card, CardActions, Appbar/utils.ts, Dialog, ToggleButtonRow) are what that issue exists to remove, so they aren't precedent for new code.
Beyond the rule, the traversal has the failure mode context exists to avoid: it only matches Fragment, IconButton and Button, and returns everything else untouched. Write <Toolbar><MyBoldButton /></Toolbar>, or wrap two actions in a View to group them, and the recolouring silently stops — the children keep IconButton's default onSurfaceVariant on the toolbar's container.
There was a problem hiding this comment.
I added a ToolbarColorContext and modified Button and IconButton to pick up the Toolbar colors. Now they will work regardless of how deep they are nested inside any Views or custom components.
| containerColor, | ||
| style, | ||
| contentContainerStyle, | ||
| testID = 'toolbar', |
There was a problem hiding this comment.
No default testIDs in new components — Switch.tsx:58 is the v6 reference.
- testID = 'toolbar',
+ testID,:198 and :237 derive ${testID}-content / -container, so those need to become conditional too.
| iconColor: | ||
| 'theme.colors.onSecondaryFixedVariant (light) / theme.colors.onSecondaryContainer (dark)', | ||
| }, | ||
| }, | ||
| vibrant: { | ||
| unselected: { | ||
| backgroundColor: 'theme.colors.primaryContainer', | ||
| iconColor: | ||
| 'theme.colors.onPrimaryFixedVariant (light) / theme.colors.onPrimaryContainer (dark)', | ||
| textColor: | ||
| 'theme.colors.onPrimaryFixedVariant (light) / theme.colors.onPrimaryContainer (dark)', |
There was a problem hiding this comment.
Toolbar.standard.selected, lines 332-333:
- iconColor:
- 'theme.colors.onSecondaryFixedVariant (light) / theme.colors.onSecondaryContainer (dark)',
+ iconColor: 'theme.colors.onSecondaryContainer',Toolbar.vibrant.unselected, lines 339-342:
- iconColor:
- 'theme.colors.onPrimaryFixedVariant (light) / theme.colors.onPrimaryContainer (dark)',
- textColor:
- 'theme.colors.onPrimaryFixedVariant (light) / theme.colors.onPrimaryContainer (dark)',
+ iconColor: 'theme.colors.onPrimaryContainer',
+ textColor: 'theme.colors.onPrimaryContainer',tokens.ts:49,57,59 already resolve these unconditionally, and there's no *-fixed-variant token anywhere in md.comp.toolbar.*. Then yarn docs generate — the same strings are inlined at Toolbar.mdx:160 and need committing alongside. The other two rows in the block are already correct.
There was a problem hiding this comment.
Good catch, I forgot to udpate the docs when I changed tokens. Now they are correct.
| containerShape: 'none' as ShapeToken, | ||
| containerLeadingSpace: 16, | ||
| containerTrailingSpace: 16, | ||
| defaultSpacing: 32, | ||
| } as const; | ||
|
|
||
| const floating = { | ||
| containerHeight: 64, | ||
| containerShape: 'full' as ShapeToken, |
There was a problem hiding this comment.
as casts aren't used anywhere in src/ — and these two do nothing anyway. ShapeToken is keyof ThemeShapeCorners | 'none' | 'full', so the literal is already assignable; the cast only widens it and loses the literal under as const.
- containerShape: 'none' as ShapeToken,
+ containerShape: 'none',- containerShape: 'full' as ShapeToken,
+ containerShape: 'full',Deleting them is the whole fix — no satisfies needed, the values are checked at the usage site in resolveCornerRadius. Same at Toolbar.test.tsx:392: type the it.each table as a const instead of casting it.
Motivation
Introduce a new
Toolbarcomponent implementing the Material Design 3 toolbars spec. Reuse theme tokens (shape, color roles, elevation) and follow the same component-tokens pattern as FAB / Checkbox.Spec re-check (M3 toolbars)
Re-checked the M3 toolbars specs:
variants:floating(self-positioned pill,corner.full, elevation level 3) anddocked(full-width bar pinned to the bottom edge,corner.none, no elevation, extends into safe-area insets).floatingsupportshorizontal/verticalorientation;dockedis always horizontal per spec.standard) colors: container/unselected-buttonsurfaceContainer, icon/labelonSurfaceVariant, selected buttonsecondaryContainerwithonSecondaryFixedVariant(light) /onSecondaryContainer(dark) icon.vibrantcolorScheme: container/unselected-buttonprimaryContainer, icon/labelonPrimaryFixedVariant(light) /onPrimaryContainer(dark), selected button falls back tosurfaceContainerwithonSurfaceicon.IconButton/Buttonchildren are auto-recolored to matchcolorSchemeunless they already set their own color (amodeon either opts them out in favor of their own mode-based coloring).Changes
Toolbar/ tokens / utilssrc/components/Toolbar/{Toolbar.tsx,tokens.ts,utils.ts}:variant(floating/docked),orientation(horizontal/vertical, floating-only),colorScheme(standard/vibrant),containerColoroverride,style/contentContainerStyle,testID,aria-label,theme,refToolbarTokens;dockedextends into safe-area insets via margin outsideSurface's box, keeping the 64dp icon band untouchedwithToolbarChildColorsauto-recolors mode-lessIconButton/Buttonchildren percolorScheme, without touching children that already set their own color/modesrc/index.tsxExample / docs / tests
ToolbarExample.tsx) covering both variants, both orientations, both color schemes, over a scrollable listToolbar.test.tsx) covering shape/elevation per variant, color resolution across light/dark themes for both color schemes, and child auto-recoloring behaviorScope note
Per Satyajit Sahoo's recommendation, this implementation is split into a series of PRs for easier review, rather than landing as one large change. This PR is PR 1 of the series:
colorSchemestandard/vibrant. Component, tokens, tests, docs, example.Related issue
Related to #4988
Test plan
yarn typecheck/yarn lint/ Toolbar unit testsVisual verification
Docs screenshots:
docs/public/screenshots/toolbar_*.png.