feat: add elevation tokens and fix Android dp spec bug#4923
feat: add elevation tokens and fix Android dp spec bug#4923adrcotfas wants to merge 1 commit into@adrcotfas/refactor/tokens_motionfrom
Conversation
|
The mobile version of example app from this branch is ready! You can see it here. |
| * @deprecated Pass `theme.colors.shadow` as the second argument and import | ||
| * `shadow` from `react-native-paper/theme/tokens/sys/elevation`. Will be removed in a future version. |
There was a problem hiding this comment.
Users shouldn't be importing things using deep imports. Either make a clean breaking change or keep the API backward-compatible.
| inputRange: [...elevationInputRange], | ||
| outputRange: [...shadowLayers[layer].height], | ||
| }), | ||
| }, | ||
| shadowRadius: elevation.interpolate({ | ||
| inputRange, | ||
| outputRange: iOSShadowOutputRanges[layer].shadowRadius, | ||
| inputRange: [...elevationInputRange], | ||
| outputRange: [...shadowLayers[layer].shadowRadius], |
There was a problem hiding this comment.
why are these spread instead of using directly?
| backgroundColor: | ||
| md3Colors.elevation[ELEVATION_LEVELS_MAP[elevation]], | ||
| md3Colors.elevation[ | ||
| `level${elevation}` as ElevationLevel |
| inputRange: [...elevationInputRange], | ||
| outputRange: [...elevationInputRange].map((elevation) => { |
|
|
||
| export const elevationInputRange = [0, 1, 2, 3, 4, 5] as const; | ||
|
|
||
| export const androidElevationLevels = [0, 1, 3, 6, 8, 12] as const; |
There was a problem hiding this comment.
if these are the actual values, why not expose directly instead of having 0-5 range and translating to a different format? avoiding the indirection would keep things simpler.
| inputRange: [...elevationInputRange], | ||
| outputRange: [...shadowLayers[0].height], | ||
| }), | ||
| }, | ||
| shadowOpacity: elevation.interpolate({ | ||
| inputRange: [0, 1], | ||
| outputRange: [0, 0.3], | ||
| extrapolate: 'clamp', | ||
| }), | ||
| shadowRadius: elevation.interpolate({ | ||
| inputRange: [...elevationInputRange], | ||
| outputRange: [...shadowLayers[0].shadowRadius], |
There was a problem hiding this comment.
the spreading seems unnecessary
| export type Elevation = 0 | 1 | 2 | 3 | 4 | 5; | ||
|
|
||
| export enum ElevationLevels { | ||
| 'level0', | ||
| 'level1', | ||
| 'level2', | ||
| 'level3', | ||
| 'level4', | ||
| 'level5', | ||
| } | ||
| export type ElevationLevel = | ||
| | 'level0' | ||
| | 'level1' | ||
| | 'level2' | ||
| | 'level3' | ||
| | 'level4' | ||
| | 'level5'; |
There was a problem hiding this comment.
There is an opportunity to reduce repetition and simplify these. The same things don't need to be define as enum, union, a numeric union, then later as object in src/theme/tokens/sys/elevation.ts, and a separate range - it could be derived from the map with Object.values, types could be derived from other types, redundant enum could be removed, etc.
| elevation: number | Animated.Value = 0, | ||
| shadowColor: string | ||
| ) { | ||
| if (elevation instanceof Animated.Value) { |
There was a problem hiding this comment.
does this work for animated interpolations etc.? i also saw there was a isAnimatedValue helper in another PR
| const elevationLevel = [...androidElevationLevels]; | ||
|
|
||
| const getElevationAndroid = () => { | ||
| if (isAnimatedValue(elevation)) { | ||
| return elevation.interpolate({ | ||
| inputRange, | ||
| inputRange: [...elevationInputRange], |
2 participants
Motivation
Part of the foundation of adding tokens and structure.
Add theme.elevation sys token and clean up elevation types
Replace the
ElevationLevelsnumeric enum with anElevationLevelstring union, addThemeElevationandtheme.elevation.level0-5as a proper sys token (matchingmd.sys.elevation.*spec naming), exportelevationInputRangeas the single source of truth for the [0-5] interpolation range, fix iOS shadow color to read fromtheme.colors.shadowinstead of a hardcoded #000, and removeELEVATION_LEVELS_MAPfromMenuin favor of the template literal pattern already used inSurface.Related issue
See https://www.notion.so/callstack/React-Native-Paper-Foundation-for-MD3-Expressive-34c5d027c0f880edba3df107cd35946f?source=copy_link
Merge order:
refactor: remove MD3 prefix and use deprecated aliases #4910
refactor: move styles/ into src/theme/ tree #4915
refactor: add color role table and buildScheme #4916
feat: add theme.state opacity tokens and split theme types #4917
feat: add theme.shapes and cornersToStyle #4919
feat: add emphasized typescale and fix letter-spacing spec bugs #4920
feat: add motion tokens (spring, easing, duration) #4922
feat: add elevation tokens and fix Android dp spec bug #4923
feat: add accessibility adaptation layer #4924
feat: add dynamicColor prop and DynamicTheme refactor #4925
feat(v6): remove deprecated public API #4926
docs: update documentation #4927
Test plan
yarn typescript-- no new type errorsyarn test-- all tests pass