fix(web): honor reduced-motion preferences across product UI - #4855
fix(web): honor reduced-motion preferences across product UI#4855FaisalMisbah23 wants to merge 3 commits into
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ); | ||
| } | ||
|
|
||
| const shouldReduce = useReducedMotion(); |
There was a problem hiding this comment.
CRITICAL: React Rules of Hooks violation - useReducedMotion() is called after an early return
useReducedMotion() is invoked here, but the component has an early return for the isLoading branch a few lines above (before this line). When isLoading is true, this hook is never called; when isLoading is false, it is called. Because the number of hooks invoked differs between renders of the same component instance whenever isLoading toggles, React will throw "Rendered fewer hooks than expected" / hook-order-mismatch errors and crash the component.
Move const shouldReduce = useReducedMotion(); to the top of the function body, before the if (isLoading) { ... } early return, so it is called unconditionally on every render.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| <div className="h-2 w-2 animate-bounce rounded-full bg-gray-400 [animation-delay:-0.3s]" /> | ||
| <div className="h-2 w-2 animate-bounce rounded-full bg-gray-400 [animation-delay:-0.15s]" /> | ||
| <div className="h-2 w-2 animate-bounce rounded-full bg-gray-400" /> | ||
| <div className="motion-reduce:bg-gray-400 motion-reduce:opacity-40 h-2 w-2 animate-bounce rounded-full bg-gray-400 [animation-delay:-0.3s]" /> |
There was a problem hiding this comment.
WARNING: motion-reduce:opacity-40 does not stop the bounce animation, and motion-reduce:bg-gray-400 is a no-op
Two issues with the added utility classes on these three dots:
motion-reduce:bg-gray-400sets the same background color that's already applied unconditionally (bg-gray-400), so it has no effect under reduced motion.animate-bounceitself is never disabled here (nomotion-reduce:animate-none), so this change alone does not stop the animation. The bounce is only neutralized because of the unrelated global reset inapps/web/src/app/globals.css(animation-iteration-count: 1 !important/animation-duration: 0.01ms !important). If that global rule is ever scoped down or removed, these dots will keep bouncing despite this "fix". Consider addingmotion-reduce:animate-nonedirectly on these elements so the intent is explicit and self-contained.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| *::before, | ||
| *::after { | ||
| animation-duration: 0.01ms !important; | ||
| animation-iteration-count: 1 !important; |
There was a problem hiding this comment.
SUGGESTION: Global animation-iteration-count: 1 !important will freeze infinite/looping animations mid-frame rather than resolving them cleanly
This blanket reset forces every animation (including unbounded/looping ones like animate-spin loaders or animate-bounce indicators elsewhere in the app) to run once for 0.01ms and then stop on whatever frame that iteration lands on. For simple 360° rotations this happens to look static, but for any animation whose start/end keyframes differ, elements can appear to freeze on an intermediate/incorrect frame instead of settling into a clean resting state — which can look broken for reduced-motion users, especially for anything communicating an ongoing loading state. The PR already shows awareness of this trade-off by adding a dedicated .animate-pulse-once override; consider auditing other unbounded animations (loading spinners in particular) for the same treatment rather than relying on the global reset alone.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| @@ -0,0 +1,18 @@ | |||
| import type { StoryObj } from '@storybook/react'; | |||
There was a problem hiding this comment.
CRITICAL: New story file is in the wrong directory and missing the required default export - it will not be picked up by Storybook
Two problems:
- Storybook's config (
apps/storybook/.storybook/main.ts) only globs../stories/**/*.stories.@(js|jsx|mjs|ts|tsx), i.e.apps/storybook/stories/. This file lives inapps/web/src/stories/instead, so Storybook will never discover it. - Even if it were in the right location, Storybook's Component Story Format requires a default export with at least a
title(see other files inapps/storybook/stories/*.stories.tsx, e.g.export default { title: '...' }). This file has no default export, so it would fail to register as a story module.
Move this file to apps/storybook/stories/ReducedMotion.stories.tsx and add a default export with a title (and typically a Meta type import), following the pattern used by the other story files in that directory.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe final commit removes the duplicate, non-functional Files Reviewed (1 file)
Previous Review Summaries (2 snapshots, latest commit 946c350)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 946c350)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe follow-up commit correctly fixes the Rules-of-Hooks violation, the typing-indicator animation, and the global CSS reset, but it leaves a duplicate, still-broken story file behind at Overview
Issue Details (click to expand)CRITICAL
Resolved Since Last Review
Files Reviewed in This Update (5 files)
Fix these issues in Kilo Cloud Previous review (commit 79b4d28)Status: 4 Issues Found | Recommendation: Address before merge Executive SummaryHighest-risk issue: Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (7 files)
Reviewed by claude-sonnet-5 · Input: 26 · Output: 4.8K · Cached: 515.8K Review guidance: REVIEW.md from base branch |
1 participant
Summary
Adds a global @media (prefers-reduced-motion: reduce) CSS reset that kills all CSS animations and transitions when users enable reduced motion in their OS settings. Additionally, apply useReducedMotion() to Framer Motion animated components (HeaderLogo, BeadBoard, and OnboardingStepModel) to conditionally skip JS-driven animations.
Closes #4174
Reviewer Notes
The global CSS reset (*{ animation-duration: 0.01ms !important }) covers all CSS-based animations product-wide. Framer Motion components use the library's useReducedMotion() hook to conditionally skip animated props. Per-component changes are minimal the safety net does most of the work.