Repository navigation
feat(theme): add semantic container color prop to Surface #5121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,7 +66,8 @@ export type Props = Omit<ViewProps, 'style'> & { | |
| contentStyle?: StyleProp<ViewStyle>; | ||
| /** | ||
| * @supported Available in v5.x with theme version 3 | ||
| * Changes Banner shadow and background on iOS and Android. | ||
| * Changes Banner shadow on iOS and Android. The banner background is | ||
| * `theme.colors.surfaceContainerLow` and is not affected by `elevation`. | ||
| */ | ||
| elevation?: Elevation; | ||
| /** | ||
|
|
@@ -211,6 +212,7 @@ const Banner = ({ | |
| {...rest} | ||
| style={[surfaceStyle, style]} | ||
| theme={theme} | ||
| container="surfaceContainerLow" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This breaks the "Set custom theme" action in the Banner example on iOS, Android and web. Update the example to override the role Banner now reads: const customTheme = {
...defaultTheme,
colors: {
...defaultTheme.colors,
onSurface: Palette.tertiary100,
surfaceContainerLow: Palette.tertiary50,
primary: Palette.tertiary10,
},
};Any app that customized
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated |
||
| elevation={elevation} | ||
| > | ||
| <View style={[styles.wrapper, contentStyle]}> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ import Animated, { | |
|
|
||
| import { useInternalTheme } from '../core/theming'; | ||
| import { androidElevationLevels, shadow } from '../theme/tokens/sys/elevation'; | ||
| import type { Elevation, ThemeProp } from '../theme/types'; | ||
| import type { Elevation, SurfaceRole, ThemeProp } from '../theme/types'; | ||
|
|
||
| type AnimatedStyleProp<Key extends keyof ViewStyle> = Extract< | ||
| AnimatedStyle<Required<Pick<ViewStyle, Key>>>, | ||
|
|
@@ -22,8 +22,8 @@ type BorderRadius = AnimatedStyleProp<'borderRadius'>; | |
|
|
||
| type SurfaceVisualProps = { | ||
| /** | ||
| * Background color of the Surface. Overrides the color derived from | ||
| * `elevation`. | ||
| * Background color of the Surface. Overrides both `container` and the | ||
| * color derived from `elevation`. | ||
| */ | ||
| backgroundColor?: ColorValue; | ||
| /** | ||
|
|
@@ -104,22 +104,40 @@ export type Props = Omit<ViewProps, 'pointerEvents' | 'style'> & | |
| * To achieve the same effect, wrap the content in a child View with the overflow style. | ||
| */ | ||
| style?: StyleProp<SurfaceStyle>; | ||
| /** | ||
| * Semantic color role of the Surface background, resolved from the | ||
| * theme, e.g. `container="surfaceContainerLow"` renders with | ||
| * `theme.colors.surfaceContainerLow`. Accepted roles: `surface`, | ||
| * `surfaceDim`, `surfaceBright`, `surfaceVariant`, | ||
| * `surfaceContainerLowest`, `surfaceContainerLow`, `surfaceContainer`, | ||
| * `surfaceContainerHigh`, `surfaceContainerHighest` and `inverseSurface`. | ||
| * | ||
| * When `container` is set, `elevation` only controls the shadow (and | ||
| * z-order on Android) and no longer affects the background color. | ||
| * | ||
| * Precedence: `backgroundColor` > `container` > | ||
| * `theme.colors.elevation[levelN]` derived from `elevation`. | ||
| */ | ||
| container?: SurfaceRole; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The existing The name is also unclear. Suggested fix: drop
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kept |
||
| /** | ||
| * @supported Available in v5.x with theme version 3 | ||
| * Changes shadows and background on iOS and Android. | ||
| * Used to create UI hierarchy between components. | ||
| * | ||
| * Note: If `mode` is set to `flat`, Surface doesn't have a shadow. | ||
| * | ||
| * Note: If `backgroundColor` or `container` is set, `elevation` only | ||
| * affects the shadow. | ||
| * | ||
| * Note: In version 2 the `elevation` prop was accepted via `style` prop i.e. `style={{ elevation: 4 }}`. | ||
| * It's no longer supported with theme version 3 and you should use `elevation` property instead. | ||
| */ | ||
| elevation?: Elevation; | ||
| /** | ||
| * @supported Available in v5.x with theme version 3 | ||
| * Mode of the Surface. | ||
| * - `elevated` - Surface with a shadow and background color corresponding to set `elevation` value. | ||
| * - `flat` - Surface without a shadow, with the background color corresponding to set `elevation` value. | ||
| * - `elevated` - Surface with a shadow and background color corresponding to set `elevation` value (unless `backgroundColor` or `container` is set). | ||
| * - `flat` - Surface without a shadow, with the background color corresponding to set `elevation` value (unless `backgroundColor` or `container` is set). | ||
| */ | ||
| mode?: 'flat' | 'elevated'; | ||
| /** | ||
|
|
@@ -150,9 +168,19 @@ export type Props = Omit<ViewProps, 'pointerEvents' | 'style'> & | |
| * import { StyleSheet } from 'react-native'; | ||
| * | ||
| * const MyComponent = () => ( | ||
| * <Surface style={styles.surface} elevation={4} borderRadius={8}> | ||
| * <Text>Surface</Text> | ||
| * </Surface> | ||
| * <> | ||
| * <Surface style={styles.surface} elevation={4} borderRadius={8}> | ||
| * <Text>Surface</Text> | ||
| * </Surface> | ||
| * <Surface | ||
| * style={styles.surface} | ||
| * container="surfaceContainerLow" | ||
| * elevation={1} | ||
| * borderRadius={8} | ||
| * > | ||
| * <Text>Semantic container color</Text> | ||
| * </Surface> | ||
| * </> | ||
| * ); | ||
| * | ||
| * export default MyComponent; | ||
|
|
@@ -170,6 +198,7 @@ export type Props = Omit<ViewProps, 'pointerEvents' | 'style'> & | |
| */ | ||
| const Surface = ({ | ||
| elevation = 1, | ||
| container, | ||
| children, | ||
| theme: overriddenTheme, | ||
| style, | ||
|
|
@@ -199,7 +228,8 @@ const Surface = ({ | |
| const { colors } = theme; | ||
|
|
||
| const backgroundColor = | ||
| customBackgroundColor ?? colors.elevation?.[`level${elevation}`]; | ||
| customBackgroundColor ?? | ||
| (container ? colors[container] : colors.elevation?.[`level${elevation}`]); | ||
|
|
||
| const backgroundStyle = { backgroundColor }; | ||
|
|
||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two problems with this row on iOS and Android:
surfaceandsurfaceBrighttiles have the same color as the screen, so they look like plain text with no tile.surfaceContainerLowestandsurfaceContainerLowwrap mid-word ("surfaceContainerLo / west").An elevated tile makes every role visible, and it also shows that
elevationonly sets the shadow here. For the wrapping, use wider tiles for this row or a smaller text variant.(via Claude, verify if it makes sense)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The elevated tile does not fix this in dark theme, because
surfaceandsurfaceDimare bothneutral6there and the shadow does not show on the dark screen (screenshots). A hairlineoutlineVariantborder shows every tile in light and dark theme:(via Claude, verify if it makes sense)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated