Skip to content

refactor: modernize non standard components to the latest MD3 spec - #5016

Open
MrMuzyk wants to merge 6 commits into
callstack:mainfrom
MrMuzyk:refactor/non-standard-components
Open

refactor: modernize non standard components to the latest MD3 spec#5016
MrMuzyk wants to merge 6 commits into
callstack:mainfrom
MrMuzyk:refactor/non-standard-components

Conversation

@MrMuzyk

@MrMuzyk MrMuzyk commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Motivation

Modernizes the remaining non-standard components — Avatar, Banner, and DataTable — to the latest MD3 spec by adopting the recently added theme tokens (shape, motion, typography, colors).

Changes

Avatar (AvatarIcon, AvatarText, AvatarImage, new utils.ts)

  • Colors: default primary + getContrastingColor(white) → MD3 container pair primaryContainer / onPrimaryContainer. Custom backgrounds keep the getContrastingColor
  • luminance fallback (arbitrary per-user colors have no on- role).
  • Shape: borderRadius: size / 2 → cornerFull (9999) from src/theme/tokens/sys/shape.
  • Typography: AvatarText spreads theme.fonts.titleMedium (family/weight/letterSpacing) before its proportional fontSize/lineHeight.
  • Structure: new src/components/Avatar/utils.ts (DEFAULT_SIZE = 64, ICON_SIZE_RATIO = 0.6, resolveAvatarColors) shared by the three files; removes the duplicated
  • defaultSize constant.

Banner (Banner.tsx)

  • Motion: durations 250 * scale / 200 * scale → theme.motion.duration.medium1 / short4 (values unchanged), plus easing: Easing.bezier(...theme.motion.easing.standard). Dropped the legacy * scale multiplier to match how Switch/Checkbox adopted motion tokens.
  • Icon size: size={40} → const ICON_SIZE = 40.

DataTable (DataTableTitle, DataTableCell, DataTablePagination)

  • Typography: header → variant="labelMedium" (keeps 12px; lineHeight: 24 retained to stay vertically aligned with the 24dp sort icon); cell → variant="bodyMedium"; pagination labels → variant="bodySmall".
  • Motion: sort-arrow spin duration: 150 → theme.motion.duration.short3 (unchanged value) + easing.standard.

Visual / behavioral changes (no public API changed)

  • Avatar's default tone flips from solid primary + white content to the primaryContainer / onPrimaryContainer pair.
  • Banner & DataTable animations now use the MD3 standard easing curve (was RN's default ease-in-out); Banner no longer scales its duration by theme.animation.scale.
  • DataTable header gains MD3 label tracking (letterSpacing: 0.5).

Related issue

#4990

Test plan

  • yarn typescript
  • yarn lint
  • yarn test
  • Visual verification (yarn example web):
    • Avatar — Icon/Text/Image render circular with the new tonal default; custom-colored avatars stay legible
    • Banner — show/hide animates correctly via the FAB toggle
    • DataTable — column title sits vertically centered with the sort icon, cell/pagination text reads well, sort arrow still animates on header press
image
banner.mp4
image

@MikitasK MikitasK left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nicely done 👏
just one comment to consider before merge:

Comment thread src/components/Banner.tsx Outdated
// show
Animated.timing(position, {
duration: 250 * scale,
duration: 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.

could we keep using theme.animation.scale here? PaperProvider sets it to 0 here for reduced motion

Suggested change
duration: duration.medium1,
duration: duration.medium1 * scale,

Comment thread src/components/Banner.tsx Outdated
// hide
Animated.timing(position, {
duration: 200 * scale,
duration: duration.short4,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same here

Suggested change
duration: duration.short4,
duration: duration.short4 * scale,

oleksandrzavarzin-callstack added a commit to oleksandrzavarzin-callstack/react-native-paper that referenced this pull request Aug 21, 2026
Comment thread src/components/Banner.tsx Outdated
Comment on lines +152 to +179
@@ -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]);

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:

const reduceMotion = 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`

Comment thread src/components/Banner.tsx Outdated
Comment on lines +152 to +179
@@ -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]);

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`

Comment on lines +108 to +112
duration: duration.short3,
easing: Easing.bezier(...easing.standard),
useNativeDriver: true,
}).start();
}, [sortDirection, spinAnim]);
}, [sortDirection, spinAnim, duration, easing]);

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.

-      duration: duration.short3,
+      duration: duration.short3 * scale,

The latest commit restored * scale in Banner, but the sort-arrow rotation still runs at full duration when PaperProvider has zeroed theme.animation.scale for reduce-motion. Not a regression — it was a hardcoded 150 before — but you're already editing this exact Animated.timing call, and leaving the two halves of one PR on different reduce-motion stories invites the next reader to pick the wrong one. Needs const { scale } = theme.animation; and the extra dep, same as Banner.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

good point, updated 👌

@JKobrynski

Copy link
Copy Markdown
Collaborator

@MikitasK let me know when this is ready for another review!

@JKobrynski JKobrynski left a comment

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.

LGTM! @satya164 @artus9033 all yours

@JKobrynski

Copy link
Copy Markdown
Collaborator

@MikitasK we've got conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants