-
-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add drag pan type #256
Add drag pan type #256
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
✅ Deploy Preview for reaflow ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
src/Canvas.tsx
Outdated
@@ -244,11 +274,34 @@ const InternalCanvas: FC<CanvasProps & { ref?: Ref<CanvasRef> }> = forwardRef(({ | |||
} | |||
}, [createDragNodeChildren, dragNodeData, layout?.children]); | |||
|
|||
useEffect(() => { | |||
const zoomOnWheelScroll = (e: WheelEvent) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to add pinch too. Use the pinch in react-use-gesture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also combine the gestures into 1 and use the wheel event they provide - https://use-gesture.netlify.app/docs/gestures/#handling-multiple-gestures-at-once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah nice that's cleaner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the pinch zoom is already being applied here
Lines 52 to 67 in 85ec43c
useGesture( | |
{ | |
onPinch: ({ offset: [d], event }) => { | |
event.preventDefault(); | |
// TODO: Set X/Y on center of zoom | |
const next = limit(d / 100, minZoom, maxZoom); | |
setFactor(next); | |
onZoomChange(next + 1); | |
} | |
}, | |
{ | |
enabled: !disabled, | |
domTarget: svgRef, | |
eventOptions: { passive: false } | |
} | |
); |
@@ -313,7 +302,6 @@ const InternalCanvas: FC<CanvasProps & { ref?: Ref<CanvasRef> }> = forwardRef(({ | |||
}} | |||
onMouseEnter={onMouseEnter} | |||
onMouseLeave={onMouseLeave} | |||
{...bind()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need this because u set the domTarget
correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The only way to pan the chart is with scrolling
What is the new behavior?
You can pan charts by dragging the screen with
panType='drag'
. This will also enable zooming with the mouse wheelDoes this PR introduce a breaking change?
Other information
Screen.Recording.2024-07-03.at.2.16.11.PM.mov