Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions src/components/Avatar/AvatarIcon.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,13 +2,11 @@ import { StyleSheet, View } from 'react-native';
import type { StyleProp, ViewProps, ViewStyle } from 'react-native';

import { useInternalTheme } from '../../core/theming';
import { white } from '../../theme/colors';
import { cornerFull } from '../../theme/tokens/sys/shape';
import type { ThemeProp } from '../../types';
import getContrastingColor from '../../utils/getContrastingColor';
import Icon from '../Icon';
import type { IconSource } from '../Icon';

const defaultSize = 64;
import { DEFAULT_SIZE, ICON_SIZE_RATIO, resolveAvatarColors } from './utils';

export type Props = ViewProps & {
/**
Expand DownExpand Up@@ -45,33 +43,34 @@ export type Props = ViewProps & {
*/
const Avatar = ({
icon,
size = defaultSize,
size = DEFAULT_SIZE,
style,
theme: themeOverrides,
...rest
}: Props) => {
const theme = useInternalTheme(themeOverrides);
const { backgroundColor = theme.colors?.primary, ...restStyle } =
StyleSheet.flatten(style) || {};
const textColor =
rest.color ??
getContrastingColor(backgroundColor, white, 'rgba(0, 0, 0, .54)');
const { backgroundColor, ...restStyle } = StyleSheet.flatten(style) || {};
const { background, textColor } = resolveAvatarColors({
theme,
backgroundColor,
color: rest.color,
});

return (
<View
style={[
{
width: size,
height: size,
borderRadius: size / 2,
backgroundColor,
borderRadius: cornerFull,
backgroundColor: background,
},
styles.container,
restStyle,
]}
{...rest}
>
<Icon source={icon} color={textColor} size={size * 0.6} />
<Icon source={icon} color={textColor} size={size * ICON_SIZE_RATIO} />
</View>
);
};
Expand Down
17 changes: 9 additions & 8 deletions src/components/Avatar/AvatarImage.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,11 +8,11 @@ import type {
ViewStyle,
} from 'react-native';

import { DEFAULT_SIZE, resolveAvatarColors } from './utils';
import { useInternalTheme } from '../../core/theming';
import { cornerFull } from '../../theme/tokens/sys/shape';
import type { ThemeProp } from '../../types';

const defaultSize = 64;

export type AvatarImageSource =
| ImageSourcePropType
| ((props: { size: number }) => React.ReactNode);
Expand DownExpand Up@@ -74,7 +74,7 @@ export type Props = ViewProps & {
* ```
*/
const AvatarImage = ({
size = defaultSize,
size = DEFAULT_SIZE,
source,
style,
onError,
Expand All@@ -87,17 +87,18 @@ const AvatarImage = ({
testID,
...rest
}: Props) => {
const { colors } = useInternalTheme(themeOverrides);
const { backgroundColor = colors?.primary } = StyleSheet.flatten(style) || {};
const theme = useInternalTheme(themeOverrides);
const { backgroundColor } = StyleSheet.flatten(style) || {};
const { background } = resolveAvatarColors({ theme, backgroundColor });

return (
<View
style={[
{
width: size,
height: size,
borderRadius: size / 2,
backgroundColor,
borderRadius: cornerFull,
backgroundColor: background,
},
style,
]}
Expand All@@ -108,7 +109,7 @@ const AvatarImage = ({
<Image
testID={testID}
source={source}
style={{ width: size, height: size, borderRadius: size / 2 }}
style={{ width: size, height: size, borderRadius: cornerFull }}
onError={onError}
onLayout={onLayout}
onLoad={onLoad}
Expand Down
24 changes: 12 additions & 12 deletions src/components/Avatar/AvatarText.tsx
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
import { StyleSheet, useWindowDimensions, View } from 'react-native';
import type { StyleProp, TextStyle, ViewProps, ViewStyle } from 'react-native';

import { DEFAULT_SIZE, resolveAvatarColors } from './utils';
import { useInternalTheme } from '../../core/theming';
import { white } from '../../theme/colors';
import { cornerFull } from '../../theme/tokens/sys/shape';
import type { ThemeProp } from '../../types';
import getContrastingColor from '../../utils/getContrastingColor';
import Text from '../Typography/Text';

const defaultSize = 64;

export type Props = ViewProps & {
/**
* Initials to show as the text in the `Avatar`.
Expand DownExpand Up@@ -55,7 +53,7 @@ export type Props = ViewProps & {
*/
const AvatarText = ({
label,
size = defaultSize,
size = DEFAULT_SIZE,
style,
labelStyle,
color: customColor,
Expand All@@ -64,11 +62,12 @@ const AvatarText = ({
...rest
}: Props) => {
const theme = useInternalTheme(themeOverrides);
const { backgroundColor = theme.colors?.primary, ...restStyle } =
StyleSheet.flatten(style) || {};
const textColor =
customColor ??
getContrastingColor(backgroundColor, white, 'rgba(0, 0, 0, .54)');
const { backgroundColor, ...restStyle } = StyleSheet.flatten(style) || {};
const { background, textColor } = resolveAvatarColors({
theme,
backgroundColor,
color: customColor,
});
const { fontScale } = useWindowDimensions();

return (
Expand All@@ -77,8 +76,8 @@ const AvatarText = ({
{
width: size,
height: size,
borderRadius: size / 2,
backgroundColor,
borderRadius: cornerFull,
backgroundColor: background,
},
styles.container,
restStyle,
Expand All@@ -88,6 +87,7 @@ const AvatarText = ({
<Text
style={[
styles.text,
theme.fonts.titleMedium,
{
color: textColor,
fontSize: size / 2,
Expand Down
33 changes: 33 additions & 0 deletions src/components/Avatar/utils.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
import type { ColorValue } from 'react-native';

import { white } from '../../theme/colors';
import type { InternalTheme } from '../../types';
import getContrastingColor from '../../utils/getContrastingColor';

export const DEFAULT_SIZE = 64;
export const ICON_SIZE_RATIO = 0.6;

/**
* Resolve background and content colors for an avatar.
* - No custom background → MD3 container pair (primaryContainer / onPrimaryContainer).
* - Custom background → keep luminance-based contrast so arbitrary per-user
* colors stay legible (contentColorFor only handles known theme roles).
*/
export const resolveAvatarColors = ({
theme,
backgroundColor,
color,
}: {
theme: InternalTheme;
backgroundColor?: ColorValue;
color?: string;
}) => {
const usingDefault = backgroundColor == null;
const background = backgroundColor ?? theme.colors.primaryContainer;
const textColor =
color ??
(usingDefault
? theme.colors.onPrimaryContainer
: getContrastingColor(background, white, 'rgba(0, 0, 0, .54)'));
return { background, textColor };
};
14 changes: 9 additions & 5 deletions src/components/Banner.tsx
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { Animated, StyleSheet, View } from 'react-native';
import { Animated, Easing, StyleSheet, View } from 'react-native';
import type { StyleProp, ViewStyle } from 'react-native';
import type { LayoutChangeEvent } from 'react-native';

Expand All@@ -14,6 +14,7 @@ import { useInternalTheme } from '../core/theming';
import type { $Omit, $RemoveChildren, Theme, ThemeProp } from '../types';

const DEFAULT_MAX_WIDTH = 960;
const ICON_SIZE = 40;

export type Props = $Omit<$RemoveChildren<typeof Surface>, 'mode'> & {
/**
Expand DownExpand Up@@ -148,6 +149,7 @@ const Banner = ({
const showCallback = useLatestCallback(onShowAnimationFinished);
const hideCallback = useLatestCallback(onHideAnimationFinished);

const { duration, easing } = theme.motion;
const { scale } = theme.animation;

const opacity = position.interpolate({
Expand All@@ -159,20 +161,22 @@ const Banner = ({
if (visible) {
// show
Animated.timing(position, {
duration: 250 * scale,
duration: duration.medium1 * scale,
toValue: 1,
useNativeDriver: false,
easing: Easing.bezier(...easing.standard),
}).start(showCallback);
} else {
// hide
Animated.timing(position, {
duration: 200 * scale,
duration: duration.short4 * scale,
toValue: 0,
useNativeDriver: false,
easing: Easing.bezier(...easing.standard),
}).start(hideCallback);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [visible, position, scale]);
}, [visible, position, duration, easing, scale]);
Comment on lines 152 to +179

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- const { scale } = theme.animation;+ const { duration, easing } = theme.motion;
...
- duration: 250 * scale,+ duration: duration.medium1,

Dropping the * scale multiplier turns off reduce-motion for Banner entirely. scale isn't a legacy leftover — PaperProvider.tsx:35-37 sets it to 0 whenever useResolvedReduceMotion() resolves true, either from AccessibilityInfo.isReduceMotionEnabled or from the explicit reduceMotion prop. 250 * scale was therefore 0 for those users and the banner snapped in and out; after this change it always runs the full 250ms/200ms transition. The token adoption
itself is fine (medium1 = 250, short4 = 200, identical to the old numbers), it's only the multiplier that carries the a11y behaviour.

The PR description says this matches "how Switch/Checkbox adopted motion tokens", but those two didn't drop reduce-motion — they replaced the mechanism. Switch.tsx:169-171 and Checkbox.tsx both call useReduceMotion() from src/theme/accessibility/ReduceMotionContext and feed ReduceMotion.Always/Never into their Reanimated configs. Banner ends up honouring neither path, and it's now the only animated component in src/ that reads neither theme.animation.scale (still live in ProgressBar, Modal, Badge, Snackbar, ActivityIndicator, Chip, Card, CrossFadeIcon, DrawerCollapsedItem, RadioButtonAndroid) nor useReduceMotion().

RN's Animated.timing has no reduceMotion option, so either keep scale, or take the Switch/Checkbox route:

constreduceMotion=useReduceMotion();// ...duration: reduceMotion ? 0 : duration.medium1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for raising this up @JKobrynski 🙏

basically, this issue was addressed in recent ae2ecfc commit. both Banner durations now use theme.animation.scale & regression test covers scale: 0

as for PR description, it's stale & I can't update since I'm not the owner of this PR
but here's what it should say instead:

### Banner — Motion
- Durations `250 * scale` / `200 * scale` -> `theme.motion.duration.medium1 * scale` / `theme.motion.duration.short4 * scale` (values unchanged at the default scale) + `Easing.bezier(...theme.motion.easing.standard)`
- Banner continues to respect `theme.animation.scale` including `scale: 0` reduced-motion behavior
### Visual / behavioral changes
- Banner & DataTable animations now use MD3 standard easing curve. Banner continues to honor `theme.animation.scale`


const handleLayout = ({ nativeEvent }: LayoutChangeEvent) => {
const { height } = nativeEvent.layout;
Expand DownExpand Up@@ -221,7 +225,7 @@ const Banner = ({
<View style={styles.content}>
{icon ? (
<View style={styles.icon}>
<Icon source={icon} size={40} />
<Icon source={icon} size={ICON_SIZE} />
</View>
) : null}
<Text
Expand Down
1 change: 1 addition & 0 deletions src/components/DataTable/DataTableCell.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -105,6 +105,7 @@ const CellContent = ({

return (
<Text
variant="bodyMedium"
style={textStyle}
numberOfLines={1}
maxFontSizeMultiplier={maxFontSizeMultiplier}
Expand Down
3 changes: 2 additions & 1 deletion src/components/DataTable/DataTablePagination.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -298,6 +298,7 @@ const DataTablePagination = ({
style={styles.optionsContainer}
>
<Text
variant="bodySmall"
style={[styles.label, { color: labelColor }]}
numberOfLines={3}
testID="select-page-dropdown-label"
Expand All@@ -317,6 +318,7 @@ const DataTablePagination = ({
</View>
)}
<Text
variant="bodySmall"
style={[styles.label, { color: labelColor }]}
numberOfLines={3}
aria-label={accessibilityLabel || 'label'}
Expand DownExpand Up@@ -352,7 +354,6 @@ const styles = StyleSheet.create({
marginVertical: 6,
},
label: {
fontSize: 12,
marginRight: 16,
},
button: {
Expand Down
20 changes: 14 additions & 6 deletions src/components/DataTable/DataTableTitle.tsx
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
import * as React from 'react';
import { Animated, PixelRatio, Pressable, StyleSheet } from 'react-native';
import {
Animated,
Easing,
PixelRatio,
Pressable,
StyleSheet,
} from 'react-native';
import type {
GestureResponderEvent,
PressableProps,
Expand DownExpand Up@@ -94,13 +100,16 @@ const DataTableTitle = ({
new Animated.Value(sortDirection === 'ascending' ? 0 : 1)
);

const { duration, easing } = theme.motion;

React.useEffect(() => {
Animated.timing(spinAnim, {
toValue: sortDirection === 'ascending' ? 0 : 1,
duration: 150,
duration: duration.short3,
easing: Easing.bezier(...easing.standard),
useNativeDriver: true,
}).start();
}, [sortDirection, spinAnim]);
}, [sortDirection, spinAnim, duration, easing]);

const textColor = theme.colors.onSurface;

Expand DownExpand Up@@ -132,6 +141,7 @@ const DataTableTitle = ({
{icon}

<Text
variant="labelMedium"
style={[
styles.cell,
// height must scale with numberOfLines
Expand DownExpand Up@@ -183,10 +193,8 @@ const styles = StyleSheet.create({
},

cell: {
lineHeight: 24,
fontSize: 12,
fontWeight: '500',
alignItems: 'center',
lineHeight: 24,
},

sorted: {
Expand Down
27 changes: 27 additions & 0 deletions src/components/__tests__/Banner.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -145,6 +145,33 @@ describe('animations', () => {
hideCallback = undefined;
});

it('uses zero-duration animations when animation scale is disabled', async () => {
const timingSpy = jest.spyOn(Animated, 'timing');
const view = await render(
<Banner visible={false} theme={{ animation: { scale: 0 } }}>
Text
</Banner>
);

expect(timingSpy).toHaveBeenLastCalledWith(
expect.anything(),
expect.objectContaining({ duration: 0 })
);

await view.rerender(
<Banner visible theme={{ animation: { scale: 0 } }}>
Text
</Banner>
);

expect(timingSpy).toHaveBeenLastCalledWith(
expect.anything(),
expect.objectContaining({ duration: 0 })
);

timingSpy.mockRestore();
});

describe('when component is rendered hidden', () => {
// This behaviour is probably a bug. Needs triage before next version.
it('will fire onHideAnimationFinished on mount', async () => {
Expand Down
Loading