new(drag): add useDrag hook
#902
Conversation
| @@ -64,27 +64,25 @@ export default function DragI({ width, height }: DragIProps) { | |||
| setDraggingItems(raise(draggingItems, i)); | |||
| }} | |||
| > | |||
| {({ dragStart, dragEnd, dragMove, isDragging, dx, dy }) => | |||
| (false && isDragging && console.log(d.id, d.x, dx)) || ( | |||
There was a problem hiding this comment.
just removed this guy 😱
|
|
||
| <g> | ||
| {isDragging && ( | ||
| /* capture mouse events (note: <Drag /> does this for you) */ |
There was a problem hiding this comment.
💬 this is the key missing feature from useDrag
| ); | ||
|
|
||
| // if we use useEffect, some callback invocations are skipped | ||
| useLayoutEffect(() => { |
There was a problem hiding this comment.
the hook is the general approach I saw for trying to re-create the setState(state, callback) component api in hooks, except they all recommend useEffect. However, when I used useEffect I noticed in the /drag-ii demo that some of the callbacks were skipped (e.g., the useDragOptions.onDragStart [the callback] would be skipped sometimes, which resulted in the previous line being connected to the new line you were starting to draw [onDragStart creates a new line in /drag-ii]).
I don't have a ton of experience using useLayoutEffect vs useEffect, I just know that useLayoutEffect is blocking while useEffect isn't as documented in the react api and we get a warning in next now that useLayoutEffect cannot be used server-side (for this functionality I don't think it matters).
Would love any thoughts on this!
There was a problem hiding this comment.
Hm. Not ideal but this could be something we revisit in the future.
Pull Request Test Coverage Report for Build 344323691
💛 - Coveralls |
packages/visx-drag/src/useDrag.tsx
Outdated
| import { localPoint } from '@visx/event'; | ||
| import useStateWithCallback from './util/useStateWithCallback'; | ||
|
|
||
| type MouseOrTouchEvent = React.MouseEvent | React.TouchEvent; |
There was a problem hiding this comment.
Could add React.PointerEvent. Was added in React 16.4 https://reactjs.org/blog/2018/05/23/react-v-16-4.html#pointer-events
| ); | ||
|
|
||
| // if we use useEffect, some callback invocations are skipped | ||
| useLayoutEffect(() => { |
There was a problem hiding this comment.
Hm. Not ideal but this could be something we revisit in the future.
There was a problem hiding this comment.
Just remembered, need to bump react peerDep to ^16.8.0-0. similar to @visx/tooltip. (hooks were added in 16.8 https://reactjs.org/docs/hooks-intro.html).
https://github.com/airbnb/visx/blob/master/packages/visx-tooltip/package.json#L39
🚀 Enhancements
This PR
useDraghook to@visx/dragto complement the render props<Drag />component@visx/annotationwork)<Drag />to useuseDragreactpeerDependencyto^16.8.0-0to support hooksPointerEvents(in addition toMouseandTouchevents) in drag callbacks@visx/demo/drag-iito useuseDragso we have examples of both usages/docs/dragto includeuseDrag@visx/dragapisTested via
/drag-iiis still functional@hshoff @kristw