Conversation
Introduces a unified event system for react-two.js components, enabling R3F-style event handlers (e.g., onClick, onPointerDown) for all shape and group components. Adds an Events module for hit testing and event dispatch, updates Context and Provider to manage event registration, and refactors all shape components to extract and register event handlers from props.
Introduces getWorldCoordinates to convert DOM event coordinates to canvas-relative world-space. Refactors Provider event handlers to use getWorldCoordinates for hit testing, improving coordinate accuracy and simplifying event handling logic.
There was a problem hiding this comment.
Pull request overview
This PR implements React Three Fiber-style event handlers for all shape components in react-two.js, enabling interactive graphics with mouse and pointer events. The event system includes bubbling support through Group hierarchies and provides 12 event handler types (onClick, onPointerOver, onPointerMove, etc.) that match the R3F API.
- Added comprehensive event handling system with R3F-compatible API
- Implemented event bubbling through Group component hierarchies
- Updated all 14 shape components to support event handlers
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/Events.ts | New file implementing the core event system with hit testing, coordinate conversion, and event bubbling logic |
| lib/Provider.tsx | Added event listener setup on canvas, event shape registration/unregistration, and onPointerMissed support |
| lib/Context.ts | Extended context interface to include registerEventShape and unregisterEventShape functions |
| lib/Properties.ts | Re-exported event handler types for convenient importing |
| lib/Circle.tsx | Added event handler support with prop extraction and registration logic |
| lib/Rectangle.tsx | Added event handler support with prop extraction and registration logic |
| lib/RoundedRectangle.tsx | Added event handler support with prop extraction and registration logic |
| lib/Ellipse.tsx | Added event handler support with prop extraction and registration logic |
| lib/Line.tsx | Added event handler support with prop extraction and registration logic |
| lib/Path.tsx | Added event handler support with prop extraction and registration logic |
| lib/Points.tsx | Added event handler support with prop extraction and registration logic |
| lib/Polygon.tsx | Added event handler support with prop extraction and registration logic |
| lib/Star.tsx | Added event handler support with prop extraction and registration logic |
| lib/ArcSegment.tsx | Added event handler support with prop extraction and registration logic |
| lib/Text.tsx | Added event handler support with prop extraction and registration logic |
| lib/Sprite.tsx | Added event handler support with prop extraction and registration logic |
| lib/ImageSequence.tsx | Added event handler support with prop extraction and registration logic |
| lib/Image.tsx | Added event handler support with prop extraction and registration logic |
| lib/Group.tsx | Added event handler support and context propagation for event bubbling |
| src/Playground.tsx | Added interactive examples demonstrating hover and click event handlers |
| package-lock.json | Updated two.js peer dependency to point to GitHub repository and bumped version to v0.8.22 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| return { eventHandlers, shapeProps }; | ||
| }, [props]); |
There was a problem hiding this comment.
The props object dependency will cause this useMemo to re-run on every render because props is a new object each time. This defeats the purpose of useMemo and will cause the eventHandlers object to be recreated on every render, triggering unnecessary re-registration of event handlers.
Consider using a more stable dependency array that only includes the actual props that might change, or extract individual props before the useMemo.
| // Extract event handlers from props | ||
| const { eventHandlers, shapeProps } = useMemo(() => { | ||
| const eventHandlers: Partial<EventHandlers> = {}; | ||
| const shapeProps: Record<string, unknown> = {}; | ||
|
|
||
| for (const key in props) { | ||
| if (EVENT_HANDLER_NAMES.includes(key as keyof EventHandlers)) { | ||
| eventHandlers[key as keyof EventHandlers] = props[ | ||
| key as keyof EventHandlers | ||
| ] as any; | ||
| } else { | ||
| shapeProps[key] = (props as any)[key]; | ||
| } | ||
| } | ||
|
|
||
| return { eventHandlers, shapeProps }; | ||
| }, [props]); |
There was a problem hiding this comment.
The props object dependency will cause this useMemo to re-run on every render because props is a new object each time. This defeats the purpose of useMemo and will cause the eventHandlers object to be recreated on every render, triggering unnecessary re-registration of event handlers.
Consider using a more stable dependency array that only includes the actual props that might change, or extract individual props before the useMemo.
| // Extract event handlers from props | |
| const { eventHandlers, shapeProps } = useMemo(() => { | |
| const eventHandlers: Partial<EventHandlers> = {}; | |
| const shapeProps: Record<string, unknown> = {}; | |
| for (const key in props) { | |
| if (EVENT_HANDLER_NAMES.includes(key as keyof EventHandlers)) { | |
| eventHandlers[key as keyof EventHandlers] = props[ | |
| key as keyof EventHandlers | |
| ] as any; | |
| } else { | |
| shapeProps[key] = (props as any)[key]; | |
| } | |
| } | |
| return { eventHandlers, shapeProps }; | |
| }, [props]); | |
| // Extract event handlers and shape props keys from props | |
| const eventHandlerKeys = Object.keys(props).filter(key => | |
| EVENT_HANDLER_NAMES.includes(key as keyof EventHandlers) | |
| ); | |
| const shapePropKeys = Object.keys(props).filter(key => | |
| !EVENT_HANDLER_NAMES.includes(key as keyof EventHandlers) | |
| ); | |
| const { eventHandlers, shapeProps } = useMemo(() => { | |
| const eventHandlers: Partial<EventHandlers> = {}; | |
| const shapeProps: Record<string, unknown> = {}; | |
| for (const key of eventHandlerKeys) { | |
| eventHandlers[key as keyof EventHandlers] = props[ | |
| key as keyof EventHandlers | |
| ] as any; | |
| } | |
| for (const key of shapePropKeys) { | |
| shapeProps[key] = (props as any)[key]; | |
| } | |
| return { eventHandlers, shapeProps }; | |
| }, [ | |
| ...eventHandlerKeys.map(key => props[key]), | |
| ...shapePropKeys.map(key => props[key]) | |
| ]); |
| } | ||
|
|
||
| return { eventHandlers, shapeProps }; | ||
| }, [props]); |
There was a problem hiding this comment.
The props object dependency will cause this useMemo to re-run on every render because props is a new object each time. This defeats the purpose of useMemo and will cause the eventHandlers object to be recreated on every render, triggering unnecessary re-registration of event handlers.
Consider using a more stable dependency array that only includes the actual props that might change, or extract individual props before the useMemo.
| }, [props]); | |
| }, [Object.keys(props).join(','), ...Object.values(props)]); |
| } | ||
|
|
||
| return { eventHandlers, shapeProps }; | ||
| }, [props]); |
There was a problem hiding this comment.
The props object dependency will cause this useMemo to re-run on every render because props is a new object each time. This defeats the purpose of useMemo and will cause the eventHandlers object to be recreated on every render, triggering unnecessary re-registration of event handlers.
Consider using a more stable dependency array that only includes the actual props that might change, or extract individual props before the useMemo.
| }, [props]); | |
| }, [...Object.values(props)]); |
| } | ||
|
|
||
| return { eventHandlers, shapeProps }; | ||
| }, [props]); |
There was a problem hiding this comment.
The props object dependency will cause this useMemo to re-run on every render because props is a new object each time. This defeats the purpose of useMemo and will cause the eventHandlers object to be recreated on every render, triggering unnecessary re-registration of event handlers.
Consider using a more stable dependency array that only includes the actual props that might change, or extract individual props before the useMemo.
| }, [props]); | |
| }, [...Object.keys(props), ...Object.values(props)]); |
| // Extract event handlers from props | ||
| const { eventHandlers, shapeProps } = useMemo(() => { | ||
| const eventHandlers: Partial<EventHandlers> = {}; | ||
| const shapeProps: Record<string, unknown> = {}; | ||
|
|
||
| for (const key in props) { | ||
| if (EVENT_HANDLER_NAMES.includes(key as keyof EventHandlers)) { | ||
| eventHandlers[key as keyof EventHandlers] = props[ | ||
| key as keyof EventHandlers | ||
| ] as any; | ||
| } else { | ||
| shapeProps[key] = (props as any)[key]; | ||
| } | ||
| } | ||
|
|
||
| return { eventHandlers, shapeProps }; | ||
| }, [props]); |
There was a problem hiding this comment.
The props object dependency will cause this useMemo to re-run on every render because props is a new object each time. This defeats the purpose of useMemo and will cause the eventHandlers object to be recreated on every render, triggering unnecessary re-registration of event handlers.
Consider using a more stable dependency array that only includes the actual props that might change, or extract individual props before the useMemo.
| // Extract event handlers from props | |
| const { eventHandlers, shapeProps } = useMemo(() => { | |
| const eventHandlers: Partial<EventHandlers> = {}; | |
| const shapeProps: Record<string, unknown> = {}; | |
| for (const key in props) { | |
| if (EVENT_HANDLER_NAMES.includes(key as keyof EventHandlers)) { | |
| eventHandlers[key as keyof EventHandlers] = props[ | |
| key as keyof EventHandlers | |
| ] as any; | |
| } else { | |
| shapeProps[key] = (props as any)[key]; | |
| } | |
| } | |
| return { eventHandlers, shapeProps }; | |
| }, [props]); | |
| // Extract event handler props and shape props before useMemo | |
| const eventHandlerProps: Partial<EventHandlers> = {}; | |
| const shapePropsRaw: Record<string, unknown> = {}; | |
| for (const key in props) { | |
| if (EVENT_HANDLER_NAMES.includes(key as keyof EventHandlers)) { | |
| eventHandlerProps[key as keyof EventHandlers] = props[key as keyof EventHandlers] as any; | |
| } else { | |
| shapePropsRaw[key] = (props as any)[key]; | |
| } | |
| } | |
| // Memoize eventHandlers and shapeProps based on their values, not the whole props object | |
| const { eventHandlers, shapeProps } = useMemo(() => { | |
| return { | |
| eventHandlers: eventHandlerProps, | |
| shapeProps: shapePropsRaw, | |
| }; | |
| }, [ | |
| ...Object.values(eventHandlerProps), | |
| ...Object.values(shapePropsRaw), | |
| ]); |
| canvas.removeEventListener('pointermove', handlePointerMove); | ||
| canvas.removeEventListener('pointercancel', handlePointerCancel); | ||
| }; | ||
| }, [state.two, props, registerEventShape, unregisterEventShape]); |
There was a problem hiding this comment.
The props dependency in this useEffect will cause all event listeners to be removed and re-attached whenever any prop changes (including unrelated props like children). This is inefficient and could cause performance issues.
Consider extracting onPointerMissed to a ref or using props.onPointerMissed directly in the dependency array instead of the entire props object.
| } | ||
|
|
||
| return { eventHandlers, shapeProps }; | ||
| }, [props]); |
There was a problem hiding this comment.
The props object dependency will cause this useMemo to re-run on every render because props is a new object each time. This defeats the purpose of useMemo and will cause the eventHandlers object to be recreated on every render, triggering unnecessary re-registration of event handlers.
Consider using a more stable dependency array that only includes the actual props that might change, or extract individual props before the useMemo.
| } | ||
|
|
||
| return { eventHandlers, shapeProps }; | ||
| }, [props]); |
There was a problem hiding this comment.
The props object dependency will cause this useMemo to re-run on every render because props is a new object each time. This defeats the purpose of useMemo and will cause the eventHandlers object to be recreated on every render, triggering unnecessary re-registration of event handlers.
Consider using a more stable dependency array that only includes the actual props that might change, or extract individual props before the useMemo.
| }, [props]); | |
| }, [Object.entries(props)]); |
| } | ||
|
|
||
| return { eventHandlers, shapeProps }; | ||
| }, [props]); |
There was a problem hiding this comment.
The props object dependency will cause this useMemo to re-run on every render because props is a new object each time. This defeats the purpose of useMemo and will cause the eventHandlers object to be recreated on every render, triggering unnecessary re-registration of event handlers.
Consider using a more stable dependency array that only includes the actual props that might change, or extract individual props before the useMemo.
All shape components support event handlers:
Available Event Handlers
All components support these 12 event handlers (matching React Three Fiber's API):
Mouse Events
Pointer Events
Hover Events
Scroll Event
Example Usage
Event Object Properties
Each handler receives a TwoEvent object with:
Events bubble up through Group hierarchies just like DOM events!