feat(gestures): Add propagate prop for tap event propagation control (#2277)

[mattgperry]

Summary

• Adds a propagate prop to control gesture event propagation between nested motion components
• propagate={{ tap: false }} prevents parent tap gesture handlers (onTap, onTapStart, whileTap) from firing when a child is tapped
• Fixes [BUG] Using the documented method of stopping propagation also prevents the onTap method from firing on the child element #2277 — the documented onPointerDownCapture + stopPropagation() workaround is broken in React 17+
• Uses a module-level WeakSet<Event> to track "claimed" pointerdown events, avoiding native stopPropagation() entirely so other non-Motion listeners are unaffected
• API is designed to be extensible for other gesture types (hover, drag, etc.)
• Also exposes stopPropagation option on the motion-dom press() function for framework-agnostic usage

Usage

<motion.div onTap={onParentTap}>
  <motion.div onTap={onChildTap} propagate={{ tap: false }} />
</motion.div>

Test plan

• propagate={{ tap: false }} fires child onTap but prevents parent onTap
• Without the prop, both parent and child onTap fire (default preserved)
• whileTap animation isolated to child when propagation disabled
• Three-level nesting: only child fires, parent and grandparent do not
• All 29 Jest press gesture tests pass
• Playwright E2E: stopPropagation option on press() prevents parent handler
• Playwright E2E: parent press still fires when clicking outside child
• All 14 Playwright press tests pass
• Full build succeeds

Generated with Claude Code

[claude]

Code Review for PR #3539: Add stopTapPropagation prop

Summary

This PR adds a stopTapPropagation prop to prevent parent tap gesture handlers from firing when a child element is tapped. The implementation is clean and well-tested.

---

✅ Strengths

1. Elegant Solution Design

• Using a module-level WeakSet<Event> to track claimed events is a smart approach that avoids calling native stopPropagation(), ensuring non-Motion listeners remain unaffected
• This aligns with the existing pattern in the codebase (see isPressing in packages/motion-dom/src/gestures/press/utils/state.ts:1)
• The WeakSet automatically handles memory cleanup when events are garbage collected

2. Excellent Test Coverage

• 4 comprehensive tests covering all critical scenarios:
  • Basic propagation blocking (packages/framer-motion/src/gestures/__tests__/press.test.tsx:770)
  • Default behavior preservation (packages/framer-motion/src/gestures/__tests__/press.test.tsx:794)
  • whileTap animation isolation (packages/framer-motion/src/gestures/__tests__/press.test.tsx:817)
  • Three-level nesting (packages/framer-motion/src/gestures/__tests__/press.test.tsx:866)
• All tests follow the repository's style (using nextFrame() helper for async operations)

3. Minimal API Surface

• Simple boolean prop that's easy to understand and use
• Clear documentation with usage example in packages/motion-dom/src/node/types.ts:542-553

4. Proper Integration

• Correctly added to validMotionProps set (packages/framer-motion/src/motion/utils/valid-prop.ts:38)
• Properly threaded through the layers: React component → PressGesture feature → motion-dom press function

---

🔍 Observations & Minor Considerations

1. Event Lifecycle & WeakSet Behavior
The claimedPointerDownEvents WeakSet stores the entire event object (packages/motion-dom/src/gestures/press/index.ts:21). Consider:

• Memory: PointerEvent objects hold references to targets and related objects. WeakSet will clean these up when the event is no longer referenced, but during rapid interactions, this could temporarily hold multiple event objects
• Edge Case: If the same event object is somehow reused (unlikely but theoretically possible in synthetic event systems), the check at line 61 would incorrectly block it
• Assessment: This is acceptable given browser event implementations, but worth documenting

2. Interaction with globalTapTarget
When useGlobalTarget: true, the pointerdown listener is on window (packages/motion-dom/src/gestures/press/index.ts:107). The current implementation should work correctly because:

• Events still bubble through the DOM
• The startEvent.currentTarget would be window, but the check happens before processing
• However, this interaction isn't explicitly tested

Suggestion: Add a test case combining globalTapTarget and stopTapPropagation to verify they work together correctly.

3. Performance Consideration
For rapid successive taps (e.g., button mashing), the WeakSet will accumulate event objects until they're garbage collected. This is unlikely to cause issues but:

• Consider if there are scenarios with thousands of rapid taps
• The WeakSet doesn't expose size or allow manual cleanup
• Assessment: Not a practical concern for typical UI interactions

4. Documentation Completeness
The JSDoc comment (packages/motion-dom/src/node/types.ts:542-553) could mention:

• That this doesn't affect non-Motion event listeners
• That this only blocks ancestor Motion tap handlers, not siblings
• The relationship with onTapStart and onTapCancel (are they also blocked?)

5. onTapCancel Behavior
Looking at the implementation in packages/framer-motion/src/gestures/press.ts:40-44, when a press is cancelled, handlePressEvent is called with "Cancel". The stopTapPropagation check happens at the pointerdown phase (packages/motion-dom/src/gestures/press/index.ts:61), so parent onTapCancel would also be prevented. This seems correct but isn't explicitly tested.

Suggestion: Add a test verifying that onTapCancel is also properly blocked on parents.

---

🐛 Potential Issues

1. Missing Type Export
The PointerEventOptions interface is defined in packages/motion-dom/src/gestures/press/index.ts:23-26 but not exported. While this may be intentional (internal API), if users need to type their own press handlers, they won't have access to the options type.

Assessment: Likely intentional. Only mention if this becomes a user request.

2. React Event System Compatibility ✅
The PR description mentions this fixes a React 17+ issue where onPointerDownCapture + stopPropagation() no longer works. The WeakSet approach correctly works around React's event delegation, but:

• The implementation uses native browser events (not synthetic React events)
• The startEvent parameter in startPress is typed as PointerEvent (native), not React.PointerEvent
• Assessment: Implementation is correct. React's event system delegates to a root container, but Motion uses native event listeners directly.

---

🔒 Security

No security concerns identified. The feature:

• Doesn't introduce XSS vulnerabilities
• Doesn't expose any sensitive data
• Uses standard DOM APIs safely
• WeakSet prevents memory leaks

---

⚡ Performance

The implementation is efficient:

• WeakSet lookups are O(1)
• No additional re-renders or state updates
• The check happens early in the event flow (line 61)
• Module-level WeakSet avoids per-instance overhead

Minor optimization opportunity: The WeakSet check could potentially be moved even earlier (before isValidPressEvent), but the current order is more logical and the performance difference is negligible.

---

📋 Recommendations

High Priority:

1. Add test case for globalTapTarget + stopTapPropagation interaction
2. Add test case verifying onTapCancel is blocked on parents

Low Priority:
3. Enhance JSDoc to mention non-Motion listener behavior
4. Consider adding a comment explaining why WeakSet is used instead of native stopPropagation()

Code snippet for additional test:

test("stopTapPropagation prevents parent onTapCancel from firing", async () => {
    const parentTapCancel = jest.fn()
    const childTapCancel = jest.fn()
    const Component = () => (
        <motion.div onTapCancel={() => parentTapCancel()}>
            <motion.div
                data-testid="child"
                onTapCancel={() => childTapCancel()}
                stopTapPropagation
            />
        </motion.div>
    )

    const { getByTestId, rerender } = render(<Component />)
    rerender(<Component />)

    pointerDown(getByTestId("child"))
    // Simulate a cancel by moving pointer away
    pointerCancel(getByTestId("child"))
    await nextFrame()

    expect(childTapCancel).toBeCalledTimes(1)
    expect(parentTapCancel).toBeCalledTimes(0)
})

---

✨ Overall Assessment

Verdict: Approve with minor suggestions ✅

This is a well-designed, well-tested feature that solves a real user pain point (issue #2277). The implementation is elegant and follows the repository's patterns and conventions. The test coverage is comprehensive, and the code quality is high.

The suggestions above are optional improvements that would make the feature even more robust, but the current implementation is production-ready.

Great work! 🎉

[claude]

PR Review: feat(gestures): Add stopTapPropagation prop (#2277)

Summary

This PR adds a stopTapPropagation prop to prevent parent tap gesture handlers from firing when a child element is tapped. The implementation uses a clever module-level WeakSet approach to track "claimed" events without interfering with native event propagation.

✅ Strengths

1. Excellent Architecture Decision

The use of WeakSet<Event> (packages/motion-dom/src/gestures/press/index.ts:21) is elegant:

• Avoids calling native event.stopPropagation(), which would break other non-Motion listeners
• Automatic garbage collection when events are no longer referenced
• Minimal memory overhead
• Thread-safe for the event coordination use case

2. Comprehensive Test Coverage

The four new tests cover all critical scenarios:

• Basic parent-child propagation prevention
• Default behavior preservation (without the prop)
• whileTap animation isolation
• Three-level nesting (grandparent, parent, child)

All tests follow the project's established patterns (using nextFrame() for async timing as per CLAUDE.md).

3. Minimal API Surface

The prop is boolean and opt-in, maintaining backward compatibility. The naming stopTapPropagation clearly conveys intent.

4. Clean Integration

• Properly added to validMotionProps (packages/framer-motion/src/motion/utils/valid-prop.ts:38)
• Threaded through the abstraction layers cleanly (React → framer-motion → motion-dom)
• No breaking changes to existing APIs

🔍 Observations & Minor Considerations

1. Early Return Position

In packages/motion-dom/src/gestures/press/index.ts:61, the check happens before adding to isPressing - this is correct.

2. Keyboard Accessibility

The press gesture also supports keyboard events (Enter key). The stopTapPropagation prop only affects pointer events.

Question for author: Should keyboard-triggered taps also respect stopTapPropagation? Currently, claimedPointerDownEvents only tracks PointerEvent objects, so keyboard events would not be blocked. This may be intentional, but worth documenting if it's a conscious design choice.

🔒 Security

No security concerns identified - event handling follows browser security model.

🚀 Performance

Excellent: WeakSet lookup is O(1), and the early return pattern means minimal overhead when the prop isn't used.

✨ Overall Assessment

This is a high-quality PR that:

• Solves a real user problem ([BUG] Using the documented method of stopping propagation also prevents the onTap method from firing on the child element #2277)
• Uses a clever, non-invasive implementation
• Maintains backward compatibility
• Includes thorough tests
• Follows project conventions

The WeakSet approach is particularly clever—it coordinates between Motion components without touching the native event system.

Recommendation: ✅ Approve

Minor suggestion: Document whether keyboard-triggered taps should also respect stopTapPropagation, or if the current pointer-only behavior is intentional.

---

Great work! This fixes a long-standing issue elegantly.

[claude]

PR Review: Tap Event Propagation Control

I've completed a thorough review of this PR. Overall, this is excellent work that solves a real problem (#2277) in an elegant, maintainable way. The implementation aligns perfectly with the codebase's existing patterns.

---

✅ Strengths

1. Architecture & Design

• WeakSet approach is brilliant: Using claimedPointerDownEvents = new WeakSet<Event>() at packages/motion-dom/src/gestures/press/index.ts:21 perfectly aligns with existing patterns (isPressing WeakSet, drag state management)
• Extensible API: propagate={{ tap: false }} provides a clean path for adding other gesture types (hover, drag, etc.)
• No native event pollution: Avoids event.stopPropagation() so non-Motion listeners remain unaffected - this is a major improvement over the broken React 17+ workaround

2. Code Quality

• Minimal footprint: Only 206 additions across 7 files - respects the library's size constraints
• Type safety: PropagateOptions interface properly typed at packages/motion-dom/src/node/types.ts:1047-1054
• Proper prop validation: Added to validMotionProps at packages/framer-motion/src/motion/utils/valid-prop.ts:38
• Clean integration: Feature system integration at packages/framer-motion/src/gestures/press.ts:35-51 follows existing patterns

3. Test Coverage

Comprehensive testing across both Jest and Playwright:

• ✅ Child tap fires, parent prevented (packages/framer-motion/src/gestures/tests/press.test.tsx:769-795)
• ✅ Default behavior preserved when prop omitted (lines 797-819)
• ✅ whileTap animation isolation (lines 821-867)
• ✅ Three-level nesting works correctly (lines 869-891)
• ✅ Framework-agnostic E2E tests (tests/gestures/press.spec.ts:248-270)

---

🔍 Observations & Suggestions

1. Memory Management ✅

The WeakSet automatically garbage collects when events are no longer referenced - no manual cleanup needed. This aligns with existing gesture patterns (isPressing, elementDragControls).

No action needed - implementation is correct.

2. Edge Case: Event Reuse

While highly unlikely in practice, if a browser reuses PointerEvent object references across multiple interactions, the WeakSet could theoretically block legitimate taps.

Risk assessment: Negligible - browsers create new event objects per interaction. The WeakSet's lifecycle is tied to event object lifetime, which is appropriate here.

3. Cross-Frame Scenarios

If events bubble across iframe boundaries, the module-level WeakSet won't track cross-frame events.

Impact: Minimal - cross-frame gesture handling is already complex and typically requires explicit coordination. Current implementation correctly scopes to single-context scenarios.

4. Documentation Clarity

The JSDoc at packages/motion-dom/src/node/types.ts:1069-1078 is excellent. Consider also mentioning in the PR description or docs that this doesn't affect:

• Native browser event propagation (other listeners still fire)
• React's synthetic event system (operates at Motion's gesture layer)

5. Performance ✅

• WeakSet operations are O(1)
• Early return at line 61 prevents unnecessary processing
• No measurable performance impact

---

🎯 Specific Code Review

packages/motion-dom/src/gestures/press/index.ts

// Line 61: Early exit is performant ✅
if (claimedPointerDownEvents.has(startEvent)) return

// Lines 65-67: Clean and minimal ✅
if (options.stopPropagation) {
    claimedPointerDownEvents.add(startEvent)
}

Suggestion: Consider adding a brief comment explaining why we check WeakSet before the isPressing check:

// Skip if another Motion element already claimed this pointer event
if (claimedPointerDownEvents.has(startEvent)) return

packages/framer-motion/src/gestures/press.ts

// Lines 49-52: Proper feature integration ✅
{
    useGlobalTarget: globalTapTarget,
    stopPropagation: propagate?.tap === false,
}

Clean optional chaining usage aligns with codebase style guidelines.

---

🔒 Security ✅

No security concerns. The implementation:

• Doesn't expose sensitive data
• Doesn't create injection vectors
• Uses browser-native WeakSet with automatic cleanup

---

🧪 Test Suggestions (Optional Enhancements)

While existing tests are comprehensive, consider adding:

1. Rapid-fire taps: Verify WeakSet handles rapid successive taps correctly
2. Mixed propagation: Parent with propagate={{ tap: false }} and child without
3. Dynamic prop changes: Toggle propagate.tap at runtime

These are nice-to-haves rather than blockers.

---

📊 Bundle Size Impact

Given the emphasis on file size in CLAUDE.md, the additions are minimal:

• WeakSet declaration: ~40 bytes
• Conditional logic: ~60 bytes
• Type definitions: 0 bytes (stripped in production)

Estimate: <100 bytes added to production bundle - excellent cost/benefit ratio.

---

✅ Verdict: Approved

This PR is production-ready. The implementation is:

• ✅ Well-architected and maintainable
• ✅ Properly tested (29 Jest + 14 Playwright tests passing)
• ✅ Aligned with codebase conventions
• ✅ Minimal performance/size impact
• ✅ Solves a real user pain point ([BUG] Using the documented method of stopping propagation also prevents the onTap method from firing on the child element #2277)

The API design (propagate={{ tap: false }}) is particularly elegant - it avoids double negatives and provides clear extensibility for future gesture types.

Recommendation: Merge with confidence. 🚀

---

📝 Minor Nits (Optional)

1. Consider adding the inline comment suggestion at motion-dom/src/gestures/press/index.ts:61
2. The trailing comma at packages/motion-dom/src/node/types.ts:540 is stylistically inconsistent (but likely auto-formatted)

---

Great work solving this long-standing issue! The React 17+ breaking change made the old workaround obsolete, and this Motion-native solution is the right approach.

🤖 Review conducted by Claude Code