Skip to content

Conversation

@mattgperry
Copy link
Collaborator

Summary

  • Adds reduceMotion option to animate() allowing developers to explicitly control transform/layout animation behavior
  • Makes useAnimate automatically respect MotionConfig reducedMotion setting
  • Fixes issues where animate() ignored MotionConfig and provided no way to override device preference

Fixes #2593, fixes #2771

Test plan

  • Verify animate(el, { x: 100 }, { reduceMotion: false }) animates transforms even with device reduced motion enabled
  • Verify animate(el, { x: 100 }, { reduceMotion: true }) skips transform animations
  • Verify useAnimate within <MotionConfig reducedMotion="never"> animates transforms
  • Verify useAnimate within <MotionConfig reducedMotion="always"> skips transform animations
  • Verify sequence animations respect reduceMotion option

🤖 Generated with Claude Code

@claude
Copy link

claude bot commented Jan 21, 2026

Code Review: Add reduceMotion option to animate() and make useAnimate respect MotionConfig

Summary

This PR successfully addresses issues #2593 and #2771 by adding explicit reduceMotion control to the animate() API and making useAnimate respect MotionConfig settings. The implementation is well-structured and follows the codebase conventions.


✅ Strengths

  1. Clear API Design: The reduceMotion option is well-documented with clear semantics:

    • true: Skip transform/layout animations
    • false: Always animate transforms/layout
    • undefined: Use device preference (default)
  2. Consistent Implementation: The option is properly threaded through the entire animation pipeline:

    • animateSequenceanimateSubjectanimateTarget → individual value animations
    • Type definitions are properly extended across packages
  3. React Integration: The useAnimate hook elegantly uses useReducedMotionConfig() and a ref to ensure the config value is always current without causing unnecessary re-renders.

  4. Backward Compatible: The changes are additive and maintain full backward compatibility.


⚠️ Issues & Recommendations

1. Missing Test Coverage (Critical)

The PR checklist has unchecked test items, and no test files were added/modified. Per CLAUDE.md:

IMPORTANT: Always write a failing test FIRST before implementing any bug fix or feature.

Recommendation: Add comprehensive tests covering:

// In packages/framer-motion/src/animation/animate/__tests__/animate.test.tsx
describe("animate: reduceMotion option", () => {
  test("reduceMotion: false animates transforms even with device preference", async () => {
    // Mock device preference
    // Verify transform animations work
  })
  
  test("reduceMotion: true skips transform animations", async () => {
    // Verify instant transition
  })
  
  test("reduceMotion: undefined respects device preference", async () => {
    // Test both scenarios
  })
  
  test("sequence animations respect reduceMotion option", async () => {
    // Test sequence behavior
  })
})

// In packages/framer-motion/src/animation/hooks/__tests__/use-animate.test.tsx
describe("useAnimate: MotionConfig integration", () => {
  test("respects MotionConfig reducedMotion='never'", async () => {
    // Wrap in MotionConfig, verify animations work
  })
  
  test("respects MotionConfig reducedMotion='always'", async () => {
    // Wrap in MotionConfig, verify animations skip
  })
  
  test("respects device preference when MotionConfig is 'user'", async () => {
    // Test default behavior
  })
})

2. Type Safety Issue in useAnimate (Medium)

In use-animate.ts:22-47, the wrapper function uses any types:

return ((
    subjectOrSequence: any,
    optionsOrKeyframes?: any,
    options?: any
) => {

Recommendation: Use proper typing to maintain type safety:

const animate = useConstant(() => {
    const baseAnimate = createScopedAnimate(scope)

    return (((...args: Parameters<typeof baseAnimate>) => {
        const [subjectOrSequence, optionsOrKeyframes, options] = args
        
        const isSequence = Array.isArray(subjectOrSequence) && 
                          subjectOrSequence.some(Array.isArray)

        if (isSequence) {
            return baseAnimate(
                subjectOrSequence,
                { reduceMotion: shouldReduceMotionRef.current, ...optionsOrKeyframes }
            )
        }

        return baseAnimate(
            subjectOrSequence,
            optionsOrKeyframes,
            { reduceMotion: shouldReduceMotionRef.current, ...options }
        )
    }) as typeof baseAnimate)
})

3. Potential Timing Issue with useRef (Low-Medium)

In use-animate.ts:12-13, the ref is updated on every render:

const shouldReduceMotionRef = useRef(shouldReduceMotion)
shouldReduceMotionRef.current = shouldReduceMotion

This is generally fine, but there's a subtle issue: if MotionConfig changes during an animation's execution, the behavior might be confusing. The ref ensures we use the latest value, but:

Recommendation: Document this behavior or consider if it's intentional. If animations should use the config value from when they started (not when they execute), you might need a different approach. However, the current approach seems reasonable for most use cases.

4. Documentation Gap (Low)

The TypeScript documentation is excellent, but there's no JSDoc on the functions that now accept reduceMotion.

Recommendation: Add JSDoc comments to key functions:

/**
 * Animates a sequence of animations with optional reduced motion support.
 * 
 * @param sequence - The animation sequence to execute
 * @param options - Sequence options including reduceMotion
 * @param scope - Optional animation scope
 */
export function animateSequence(...)

5. Edge Case: Sequence with Mixed reduceMotion (Low)

Currently, reduceMotion is extracted at the sequence level and applied to all animations. Consider if users might want per-animation control:

animate([
  [element1, { x: 100 }],  // Should this respect sequence reduceMotion?
  [element2, { opacity: 0 }],  // Or allow per-item override?
])

The current design applies it to all, which is probably correct, but worth documenting.


🔒 Security Considerations

No security concerns identified. The changes don't introduce any:

  • User input handling vulnerabilities
  • XSS vectors
  • Injection risks
  • Access control issues

⚡ Performance Considerations

Positive: The implementation is efficient:

  • No unnecessary re-renders (uses ref in useAnimate)
  • No extra object allocations in hot paths
  • Minimal overhead for the new option

Minor Note: The wrapper function in useAnimate creates a small closure, but this is negligible and only happens once per hook instance.


📋 Code Style & Best Practices

✅ Follows repository conventions:

  • Uses interface for type definitions
  • Named exports only
  • Proper destructuring
  • Consistent formatting

✅ The .claude/commands/fix.md change is unrelated but helpful.


🎯 Recommendations Summary

Before merging:

  1. Add comprehensive test coverage (Critical)
  2. Improve type safety in useAnimate wrapper (Medium)
  3. Consider documenting the ref update behavior (Low)

Nice to have:
4. Add JSDoc comments
5. Document sequence-level reduceMotion behavior


Final Verdict

The implementation is solid and solves the stated problems well. The main blocker is missing test coverage - this needs to be addressed before merging to ensure the feature works as expected and to prevent regressions.

Once tests are added, this will be a valuable addition to the API that gives developers proper control over reduced motion behavior.

Great work on the implementation! 🎉

@mattgperry mattgperry force-pushed the reduced-motion-animate branch from b587354 to 39a1b60 Compare January 21, 2026 16:00
@claude
Copy link

claude bot commented Jan 21, 2026

PR Review - Add reduceMotion option to animate() and make useAnimate respect MotionConfig

I've reviewed this PR and here's my feedback:

✅ Positives

  1. Clear Problem Resolution: The PR effectively addresses the issues raised in [FEATURE] Please allow reducedMotion from <MotionConfig> to be used in animate() #2593 and [BUG] animate() automatically disables all transform & layout animations when device has "reduced motion" preference #2771, providing developers control over reduced motion behavior in animate() and useAnimate.

  2. Type Safety: The ReduceMotionOption interface is well-documented with clear JSDoc comments explaining the three states (true/false/undefined).

  3. Consistent API Design: The implementation maintains consistency with existing patterns - using optional parameters and following the library's convention of respecting MotionConfig.

  4. Scope Management: The useAnimate implementation correctly uses useMemo with proper dependencies (scope and shouldReduceMotion) to avoid unnecessary re-creations.

🔍 Areas for Improvement

1. Potential Issue with useAnimate Option Merging (packages/framer-motion/src/animation/hooks/use-animate.ts:28-35)

The current implementation may not correctly handle cases where users explicitly pass reduceMotion in their animate calls:

const mergedOptions = {
    reduceMotion: shouldReduceMotion,  // This comes first
    ...options,                          // User options spread after
}

Problem: If a user explicitly passes reduceMotion: false in their options, it will correctly override the config value. However, the order might be confusing since typically we want config defaults first, then user overrides. The current implementation is actually correct (user options override), but this should be verified with tests.

Suggestion: Consider adding a comment explaining the precedence, or explicitly check for undefined:

const mergedOptions = {
    ...options,
    reduceMotion: options?.reduceMotion ?? shouldReduceMotion,
}

2. Missing Test Coverage

The PR checklist shows unchecked test items. For a feature this important, comprehensive tests are critical:

  • Test that animate(el, { x: 100 }, { reduceMotion: false }) works with device preference enabled
  • Test that animate(el, { x: 100 }, { reduceMotion: true }) skips animations
  • Test useAnimate behavior with different MotionConfig settings
  • Test sequence animations with reduceMotion option
  • Test that user-provided reduceMotion overrides MotionConfig in useAnimate

3. Sequence Animation Type Safety (packages/framer-motion/src/animation/hooks/use-animate.ts:15-24)

The sequence detection uses any types which could be more type-safe:

return ((
    subjectOrSequence: any,
    optionsOrKeyframes?: any,
    options?: any
) => {

Suggestion: Consider using proper type guards and conditional types to maintain type safety, or at minimum add a comment explaining why any is necessary here.

4. Documentation of Sequence Option Spreading (packages/framer-motion/src/animation/sequence/create.ts:388)

In create.ts, the sequenceTransition is spread at line 388, which would include the reduceMotion option. This is correct behavior, but worth verifying that this propagates correctly through all the sequence processing logic.

🎯 Performance Considerations

The use of useMemo in useAnimate is appropriate and prevents unnecessary function recreations. The implementation doesn't introduce any additional re-renders or performance penalties.

🔒 Security Considerations

No security concerns identified. The changes are purely behavioral and don't introduce any new attack vectors.

📝 Code Style

The code follows the repository conventions from CLAUDE.md:

  • Uses interface for type definitions ✅
  • Uses named exports ✅
  • Uses arrow callbacks ✅
  • Proper use of strict equality ✅

🎨 Suggestions

  1. Add tests before merging - This is critical for a feature that affects accessibility.

  2. Consider adding a migration guide - Developers who previously worked around the lack of this feature might benefit from documentation on the proper way to use it now.

  3. Document precedence clearly - The interaction between device preference, MotionConfig, and explicit reduceMotion parameter should be documented in the type definitions or README.

Overall Assessment

This is a well-designed solution that addresses a real pain point in the library. The implementation is clean and follows established patterns. The main concern is the lack of test coverage for the new functionality. Once tests are added and the minor issues above are addressed, this will be ready to merge.

Recommendation: Address test coverage before merging, consider the option merging precedence comment for clarity.

@mattgperry mattgperry force-pushed the reduced-motion-animate branch from 39a1b60 to f1d9714 Compare January 22, 2026 13:37
@claude
Copy link

claude bot commented Jan 22, 2026

PR Review: Add reduceMotion option to animate() and make useAnimate respect MotionConfig

Summary

This PR addresses issues #2593 and #2771 by adding explicit reduceMotion control to the animate() function and making useAnimate respect the MotionConfig context. The implementation allows developers to override device preferences and provides better control over reduced motion behavior.


✅ Strengths

  1. Solves Real User Pain Points: Addresses two legitimate issues where developers couldn't override device reduced motion preferences
  2. Clean API Design: The reduceMotion option follows a clear boolean pattern (true/false/undefined) that's well-documented
  3. Proper Context Integration: useAnimate now correctly respects MotionConfig via useReducedMotionConfig()
  4. Consistent Implementation: The option flows correctly through the animation pipeline (animateanimateSequenceanimateSubjectanimateTarget)
  5. Non-Breaking Change: The new option is optional and defaults to existing behavior

⚠️ Issues & Concerns

1. Critical: Missing Test Coverage

The PR has a test plan but no actual tests are included. According to CLAUDE.md:

"IMPORTANT: Always write a failing test FIRST before implementing any bug fix or feature."

Required tests:

  • animate(el, { x: 100 }, { reduceMotion: false }) animates transforms with device reduced motion enabled
  • animate(el, { x: 100 }, { reduceMotion: true }) skips transform animations
  • useAnimate within <MotionConfig reducedMotion="never"> animates transforms
  • useAnimate within <MotionConfig reducedMotion="always"> skips transform animations
  • Sequence animations respect reduceMotion option

Location for tests:

  • packages/framer-motion/src/animation/animate/__tests__/animate.test.tsx
  • packages/framer-motion/src/animation/hooks/__tests__/use-animate.test.tsx

2. Hook Dependency Issue (use-animate.ts:18-21)

const animate = useMemo(
    () => createScopedAnimate({ scope, reduceMotion }),
    [scope, reduceMotion]
)

Problem: scope is created with useConstant(), which returns a stable reference that never changes. Including it in the dependency array is misleading.

Recommendation: Remove scope from dependencies:

const animate = useMemo(
    () => createScopedAnimate({ scope, reduceMotion }),
    [reduceMotion]  // scope is stable, only reduceMotion changes
)

3. Type Safety Issue (visual-element-target.ts:42)

const reduceMotion = (transition as { reduceMotion?: boolean })?.reduceMotion

Problem: Type assertion used instead of proper typing. This bypasses TypeScript's type checking.

Better approach: Update the VisualElementAnimationOptions interface to include reduceMotion:

export interface VisualElementAnimationOptions {
    delay?: number
    transitionOverride?: TargetAndTransition["transition"]
    type?: AnimationType
    reduceMotion?: boolean  // Add this
}

Then access it directly:

export function animateTarget(
    visualElement: VisualElement,
    targetAndTransition: TargetAndTransition,
    { delay = 0, transitionOverride, type, reduceMotion }: VisualElementAnimationOptions = {}
)

4. Nullish Coalescing Logic (use-animate.ts:16)

const reduceMotion = useReducedMotionConfig() ?? undefined

Issue: This converts false to undefined, which is incorrect. If MotionConfig has reducedMotion="never", useReducedMotionConfig() returns false, but this line changes it to undefined.

Fix: Remove the nullish coalescing:

const reduceMotion = useReducedMotionConfig()

Or if you need to handle null:

const reduceMotion = useReducedMotionConfig() ?? undefined

But check what useReducedMotionConfig() actually returns (likely boolean | undefined, not boolean | null).

5. Potential Breaking Change in API

The createScopedAnimate signature changed from:

export function createScopedAnimate(scope?: AnimationScope)

to:

export function createScopedAnimate(options: ScopedAnimateOptions = {})

While this should be backward compatible (since AnimationScope is an object and options is an object with optional fields), this breaks type compatibility for existing callers passing scope directly.

Fix: Add function overload or update all call sites:

// Option 1: Overload
export function createScopedAnimate(scope?: AnimationScope): typeof scopedAnimate
export function createScopedAnimate(options?: ScopedAnimateOptions): typeof scopedAnimate
export function createScopedAnimate(scopeOrOptions?: AnimationScope | ScopedAnimateOptions) {
    // Implementation with type guard
}

// Option 2: Check for direct scope usage in codebase

📋 Minor Issues

  1. Documentation: JSDoc comments should be updated for createScopedAnimate to document the new options parameter
  2. Consistency: The ReduceMotionOption interface could be exported from a shared location rather than defined inline in types.ts

🔍 Performance Considerations

  • useMemo correctly prevents unnecessary re-creation of animate function
  • ✅ No performance regressions expected—the option is checked during animation setup, not per-frame

🔒 Security Concerns

  • ✅ No security issues identified

📊 Test Coverage Status

Current: ❌ No tests included
Required: As documented in the test plan


Recommendations Priority

Must fix before merge:

  1. Add comprehensive test coverage (all 5 test cases from the test plan)
  2. Fix the nullish coalescing logic in useAnimate (issue adding circleci config #4)
  3. Fix the type assertion in visual-element-target.ts (issue Add motionValue prop #3)

Should fix:
4. Remove scope from useMemo dependencies (issue #2)
5. Verify createScopedAnimate API compatibility (issue #5)

Nice to have:
6. Add JSDoc for new parameters
7. Consider exporting ReduceMotionOption interface


Overall Assessment

This is a valuable feature that solves real user problems. The implementation approach is sound, but the PR should not be merged without tests. The test coverage is critical for ensuring the feature works as expected and preventing regressions.

Once tests are added and the identified issues are fixed, this will be a solid addition to the API. 👍

…onConfig

This fixes two related issues:
- #2593: MotionConfig reducedMotion setting doesn't affect animate() function
- #2771: animate() automatically disables transform/layout animations based on
  device preference without developer control

Changes:
- Add reduceMotion option to AnimationOptions allowing explicit override
- useAnimate now automatically injects reduceMotion based on MotionConfig context
- Pass reduceMotion through animateTarget to override visualElement.shouldReduceMotion

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mattgperry mattgperry force-pushed the reduced-motion-animate branch from f1d9714 to b32b466 Compare January 22, 2026 13:49
@claude
Copy link

claude bot commented Jan 22, 2026

Pull Request Review

Thank you for addressing issues #2593 and #2771! This is a well-structured implementation that gives developers explicit control over reduced motion behavior. Here's my detailed review:


Strengths

  1. Clear API Design: The reduceMotion option with three states (true, false, undefined) provides intuitive control:

    • true: Skip transform/layout animations
    • false: Always animate
    • undefined: Use device preference (default)
  2. Proper Integration: The implementation correctly threads reduceMotion through the entire animation pipeline:

    • createScopedAnimate()animateSequence() / animateSubject()animateTarget()animateMotionValue()
  3. React Context Integration: useAnimate() now properly respects MotionConfig via useReducedMotionConfig(), solving a major pain point.

  4. Backwards Compatible: Uses optional parameter with default behavior unchanged, ensuring no breaking changes.

  5. Code Quality: Follows repository conventions (interfaces, no default exports, proper TypeScript usage).


🔍 Potential Issues & Questions

1. Sequence Animation Option Propagation (packages/framer-motion/src/animation/sequence/create.ts)

The reduceMotion option is added to SequenceOptions interface but I don't see it being extracted and passed down to individual element animations in createAnimationsFromSequence().

Current flow (lines 42-44):

export function createAnimationsFromSequence(
    sequence: AnimationSequence,
    { defaultTransition = {}, ...sequenceTransition }: SequenceOptions = {},

The reduceMotion is in sequenceTransition and gets spread at line 388:

definition.transition[key] = {
    ...remainingDefaultTransition,
    duration: totalDuration,
    ease: valueEasing,
    times: valueOffset,
    ...sequenceTransition,  // ← reduceMotion is here
}

Then in animateSequence() (packages/framer-motion/src/animation/animate/sequence.ts:24-25), it calls:

animations.push(...animateSubject(subject, keyframes, transition))

Question: Does the transition object preserve reduceMotion through to animateTarget()? Looking at animateSubject() (subject.ts:142), it seems to spread options into transition:

const transition = { ...options }

This should work, but I'd verify that reduceMotion successfully flows through sequence animations to the final animateTarget() call.

Test recommendation: Add a specific test for sequence animations with reduceMotion:

animate([
    [el1, { x: 100 }],
    [el2, { y: 100 }]
], { reduceMotion: false })

2. useMemo Dependency Stability (packages/framer-motion/src/animation/hooks/use-animate.ts:16-21)

const reduceMotion = useReducedMotionConfig() ?? undefined

const animate = useMemo(
    () => createScopedAnimate({ scope, reduceMotion }),
    [scope, reduceMotion]
)

Potential issue: The scope object is created once with useConstant(), so it's stable. However, reduceMotion comes from React context and could change. This is likely intentional, but:

Question: Should the animate function be recreated when reduceMotion changes? This seems correct for dynamic MotionConfig changes, but consider documenting this behavior since it means:

  • Existing running animations won't be affected by mid-flight MotionConfig changes
  • Only new animate() calls will use the updated setting

This is probably the desired behavior, but worth confirming.


3. Type Safety in animateTarget() (packages/motion-dom/src/animation/interfaces/visual-element-target.ts:42)

const reduceMotion = (transition as { reduceMotion?: boolean })?.reduceMotion

Issue: Using type assertion instead of proper typing. The transition parameter should include reduceMotion in its type definition.

Suggestion: Update the Transition type or the function signature to include reduceMotion:

interface TargetAndTransition {
    transition?: Transition & { reduceMotion?: boolean }
    // ...
}

Or add it to the Transition type itself in packages/motion-dom/src/animation/types.ts.


4. Missing Test Coverage

The PR includes a test plan in the description but no actual tests. Per CLAUDE.md:

IMPORTANT: Always write a failing test FIRST before implementing any bug fix or feature.

Critical: Please add tests covering:

  • animate(el, { x: 100 }, { reduceMotion: false }) with device preference enabled
  • animate(el, { x: 100 }, { reduceMotion: true })
  • useAnimate within <MotionConfig reducedMotion="never">
  • useAnimate within <MotionConfig reducedMotion="always">
  • ✅ Sequence animations with reduceMotion option
  • reduceMotion: false overrides device preference
  • reduceMotion: undefined respects device preference
  • ✅ Non-transform properties (like opacity) animate regardless of reduceMotion

Consider Playwright/E2E tests for gesture handling as suggested in CLAUDE.md.


5. Documentation

JSDoc improvements needed:

In packages/framer-motion/src/animation/animate/index.ts:

interface ScopedAnimateOptions {
    scope?: AnimationScope
    /**
     * Whether to reduce motion for transform/layout animations.
     * 
     * - `true`: Skip transform/layout animations (instant transition)
     * - `false`: Always animate transforms/layout
     * - `undefined`: Respect device preference (default)
     * 
     * @default undefined
     */
    reduceMotion?: boolean
}

The JSDoc in motion-dom/src/animation/types.ts:595-604 is good, but consider adding examples:

/**
 * @example
 * // Override device preference
 * animate(el, { x: 100 }, { reduceMotion: false })
 * 
 * @example  
 * // Skip transform animations
 * animate(el, { x: 100 }, { reduceMotion: true })
 */

🎯 Edge Cases to Consider

  1. Mixed animations: What happens with animate(el, { x: 100, opacity: 0.5 }, { reduceMotion: true })?

    • Expected: x is instant, opacity animates normally
    • Verify this works as intended (it should, based on the positionalKeys.has(key) check at visual-element-target.ts:119)
  2. Nested MotionConfig: If useAnimate is used within nested MotionConfig components, does it respect the nearest context? (Should work via React context behavior)

  3. Mid-animation changes: If MotionConfig.reducedMotion changes while an animation from useAnimate is running, the running animation continues with its original setting (as discussed in issue Fixing types and es references #2)


🔒 Security & Performance

  • ✅ No security concerns
  • ✅ Performance impact is minimal (one additional property check)
  • ✅ No unnecessary re-renders (proper use of useMemo)

📋 Recommendations

Before merge:

  1. ⚠️ Add comprehensive test coverage (blocker)
  2. ⚠️ Fix type assertion in visual-element-target.ts:42
  3. ⚠️ Verify sequence animation propagation with a test

Nice to have:
4. Add JSDoc examples
5. Consider adding migration guide for users affected by #2593 and #2771
6. Update documentation site with reduceMotion option examples


🎉 Conclusion

This is a solid implementation that solves real user pain points. The architecture is clean and the feature integrates well with the existing codebase. Once tests are added and the type safety issue is addressed, this will be ready to merge.

Estimated effort to address blockers: 1-2 hours for tests + 15 minutes for type fix.

Great work on addressing these long-standing issues! 🚀

@mattgperry mattgperry merged commit ea4b2e0 into main Jan 22, 2026
4 checks passed
@mattgperry mattgperry deleted the reduced-motion-animate branch January 22, 2026 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants