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:
JKobrynski
left a comment
There was a problem hiding this comment.
I think it's the last one, and it's minor
|
LGTM! @satya164 do you want to take a look? |
87be2b8 to
d4845c7
Compare
| @@ -0,0 +1,3388 @@ | |||
| // Jest Snapshot v1, https://goo.gl/fbAQLP | |||
There was a problem hiding this comment.
I think you need to rerun these with -u after rebasing, because default testIDs were removed
There was a problem hiding this comment.
You were right, the PR was still behind main branch. I rebased it and now it is up to date.
| * Style for positioning `floating`'s pill, or overriding `docked`'s | ||
| * default anchoring. | ||
| */ | ||
| style?: StyleProp<ViewStyle>; |
There was a problem hiding this comment.
What happens when you pass backgroundColor|borderRadius through this prop? I'm afraid they'd silently do nothing, other components type style as styleProp<SurfaceStyle>
There was a problem hiding this comment.
You are correct, fixed it by changing style type to StyleProp<SurfaceStyle>
| import { resolveCornerRadius } from '../../theme/utils/shape'; | ||
| import Surface from '../Surface'; | ||
|
|
||
| export type Props = { |
There was a problem hiding this comment.
Other components use Omit<ViewProps, 'style'> to allow props like onLayout`, does that work here?
There was a problem hiding this comment.
Fixed it, changed Props to Omit<ViewProps, 'style'>
| ? [styles.verticalAnchor, { right: insets.right + 16 }] | ||
| : [styles.horizontalAnchor, { bottom: insets.bottom + 24 }] |
There was a problem hiding this comment.
Have you checked the edge offsets against the toolbar specs? I'm afraid the two are swapped
There was a problem hiding this comment.
The horizontal edge offset is incorrect, it should be 16. As for the vertical one the only spec in MD3 is that the minimum offset from screen edge should be equal to 16 so I left it as it is.
d4845c7 to
d24d47c
Compare
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.