Skip to content

Add reduce motion check on animated counts#7924

Open
gilluminate wants to merge 2 commits intomainfrom
gill/reduce-motion-counts
Open

Add reduce motion check on animated counts#7924
gilluminate wants to merge 2 commits intomainfrom
gill/reduce-motion-counts

Conversation

@gilluminate
Copy link
Copy Markdown
Contributor

simply adds a check for whether the user prefers reduced motion when doing the animated counts on the dashboard.

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Apr 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Apr 15, 2026 5:23pm
fides-privacy-center Ignored Ignored Apr 15, 2026 5:23pm

Request Review

@gilluminate gilluminate requested a review from kruulik April 14, 2026 21:32
@gilluminate gilluminate marked this pull request as ready for review April 14, 2026 21:32
@gilluminate gilluminate requested a review from a team as a code owner April 14, 2026 21:32
@vercel vercel bot temporarily deployed to Preview – fides-plus-nightly April 14, 2026 21:33 Inactive
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

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(() => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 14, 2026

Title Lines Statements Branches Functions
admin-ui Coverage: 8%
6.1% (2663/43634) 5.22% (1293/24740) 4.19% (542/12913)
fides-js Coverage: 78%
78.98% (1962/2484) 65.55% (1214/1852) 72.57% (336/463)
privacy-center Coverage: 88%
85.97% (331/385) 81.36% (179/220) 78.87% (56/71)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant