Add reduce motion check on animated counts#7924
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
There was a problem hiding this comment.
Code Review — Add reduce motion check on animated counts
The change is small, focused, and directionally correct — respecting prefers-reduced-motion is a meaningful accessibility improvement. The logic is sound: reduceMotion is properly added to the dependency array, which means the effect re-runs correctly if the user changes their OS preference at runtime (since usePrefersReducedMotion subscribes to MediaQueryList.change events).
Must Fix
return setValue(target) pattern (see inline comment on line 15)
The return setValue(target) pattern is misleading. React state setters return void, so this is equivalent to return undefined. In a useEffect callback, returning something implies a cleanup function — this looks like it's returning one but isn't. Should be:
setValue(target);
return;Suggestions
No test coverage for the new branch
There's no test file for useCountUp. The new reduceMotion path is the most important case to guard against regression — specifically that setValue(target) fires immediately and no requestAnimationFrame is scheduled. A renderHook test mocking window.matchMedia to return matches: true would cover this clearly.
First-render flash (pre-existing, but worth noting while touching the hook)
usePrefersReducedMotion initializes to false before the useEffect fires. On the first render, reduceMotion is always false, so the animation ticks for at least one frame before the hook snaps to the final value for users with reduced motion enabled. A common fix is to initialize synchronously if window is available:
const [prefersReducedMotion, setPrefersReducedMotion] = useState(
() => typeof window !== "undefined"
? window.matchMedia("(prefers-reduced-motion: reduce)").matches
: false
);This is pre-existing behavior in usePrefersReducedMotion, not introduced by this PR — flagging it as an informational note since the hook is being touched anyway.
🔬 Codegraph: connected (46711 nodes)
💡 Write /code-review in a comment to re-run this review.
| const startRef = useRef<number>(); | ||
| const reduceMotion = usePrefersReducedMotion(); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
clients/admin-ui/src/home/useCountUp.ts:15
return setValue(target) is misleading here. setValue is a React state setter that returns void. In a useEffect callback, the only meaningful return value is a cleanup function — returning the result of a side-effect call looks like it's returning a cleanup, but it's actually returning undefined.
Use the idiomatic form instead:
if (reduceMotion) {
setValue(target);
return;
}This makes the intent explicit: set state, then exit early.
a6da75a to
20a2153
Compare
simply adds a check for whether the user prefers reduced motion when doing the animated counts on the dashboard.