Use pointer events instead of mouse and touch events. Support multi touch #150
Conversation
| // Interrupt "mousedown" call on mobiles | ||
| if (!isValid(nativeEvent) || !el) return; | ||
| // testing-library doesn't have setPointerCapture implemented | ||
| el.setPointerCapture && el.setPointerCapture(nativeEvent.pointerId); |
There was a problem hiding this comment.
This simple line makes:
- Multi touch support
- All subsequent pointermove and pointerup events will be captured by this particular element, hence if the pointer is released outside the browser it still will be captured (see screencasts section in description)
More info: https://developer.mozilla.org/en-US/docs/Web/API/Element/setPointerCapture
| const handleMove = useCallback( | ||
| (event: MouseEvent | TouchEvent) => { | ||
| // Prevent text selection | ||
| preventDefaultMove(event); |
There was a problem hiding this comment.
Don't need it anymore, as there is no intervention for pointer events (as I know)
| el.addEventListener("pointermove", handleMove); | ||
| el.addEventListener("pointerup", handleMoveEnd); |
There was a problem hiding this comment.
Events are attached to the element 'cause it captures all subsequent calls
| // Saves incoming handler to the ref in order to avoid "useCallback hell" | ||
| function useEventCallback<T>(handler?: (value: T) => void): (value: T) => void { | ||
| const callbackRef = useRef(handler); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
There is no need in pause, when a new function was passed, it doesn't have any async side effects, therefore removing useEffect makes the performance better
There was a problem hiding this comment.
@jeetiss check this out. You might find this approach interesting.
src/hooks/useEventCallback.ts
Outdated
|
|
||
| return useCallback((value: T) => callbackRef.current && callbackRef.current(value), []); | ||
| return callbackRef.current || noop; |
There was a problem hiding this comment.
noop is to support backward compatibility
| // Fast clicks on mobile devices | ||
| // See https://github.com/omgovich/react-colorful/issues/55 | ||
| it("Doesn't react on mouse events after a touch interaction", () => { | ||
| const handleChange = jest.fn((hslString) => hslString); | ||
| const result = render(<HslStringColorPicker color="hsl(100, 0%, 0%)" onChange={handleChange} />); | ||
| const hue = result.container.querySelector(".react-colorful__hue .react-colorful__interactive"); | ||
|
|
||
| fireEvent.touchStart(hue, { touches: [{ pageX: 0, pageY: 0, bubbles: true }] }); // 1 | ||
| fireEvent.touchMove(hue, { touches: [{ pageX: 55, pageY: 0, bubbles: true }] }); // 2 | ||
|
|
||
| // Should be skipped | ||
| fireEvent(hue, new FakeMouseEvent("mousedown", { pageX: 35, pageY: 0 })); // 3 | ||
| fireEvent(hue, new FakeMouseEvent("mousemove", { pageX: 105, pageY: 0 })); // 4 | ||
|
|
||
| expect(handleChange).toHaveReturnedTimes(2); | ||
| expect(handleChange).toHaveReturnedWith("hsl(180, 0%, 0%)"); | ||
| }); |
There was a problem hiding this comment.
Pointer events make it unified
| it("Pointer doesn't follow the mouse if it was released outside of the document bounds", async () => { | ||
| const handleChange = jest.fn(); | ||
| const result = render(<RgbaColorPicker onChange={handleChange} />); | ||
| const saturation = result.container.querySelector( | ||
| ".react-colorful__saturation .react-colorful__interactive" | ||
| ); | ||
|
|
||
| // User presses and moves the cursor inside the window | ||
| fireEvent(saturation, new FakeMouseEvent("mousedown", { pageX: 20, pageY: 10 })); // 1 | ||
| fireEvent(saturation, new FakeMouseEvent("mousemove", { pageX: 10, pageY: 10 })); // 2 | ||
| // User releases the mouse button outside of the document bounds with no `mouseup` event fired | ||
| // User moves the cursor back to the document with no button pressed | ||
| fireEvent(saturation, new FakeMouseEvent("mousemove", { pageX: 1, pageY: 50, buttons: 0 })); // 3 | ||
|
|
||
| expect(handleChange).toHaveReturnedTimes(2); // the last `mousemove` has to be ignored | ||
| }); |
There was a problem hiding this comment.
fulfilled by using pointerevents
There was a problem hiding this comment.
Can we emulate the same test case (pointer outside of the window) with PointerEvent? 🤔
|
Added separate commit, which adds TouchEvent support only for browsers that don't support PointerEvents. The size gets bigger than after the first commit, but it is still smaller than the master state: Also, the second commit removes all The only problem I have atm (didn't figure out it yet) is that UPD: it's all working: |
Codecov Report
@@ Coverage Diff @@
## master #150 +/- ##
===========================================
- Coverage 100.00% 99.29% -0.71%
===========================================
Files 33 33
Lines 433 428 -5
Branches 64 61 -3
===========================================
- Hits 433 425 -8
- Misses 0 3 +3
Continue to review full report at Codecov.
|
|
Hey @xnimorz. I'm still not sure about migrating to
|
|
I'll update the diff this week later and return mouse events than |
|
I'm going to publish 2 separate diff |
Hey! Thanks for the library ;)
I was looking at the library after our Twitter discussion: https://twitter.com/Omgovich/status/1421136814954385418
Overview
This pull request introduces several changes:
Removed MouseEvent and TouchEvent in favor of PointerEvent.
It has several advantages:
We don't have browser hacks in code to preventDefault behavior and etc.
We don't need to resolve conflicts between touches and clicks.
Improved library performance
useState, which reduces component re-renders,useEffectsalong withuseCallbackswhich also optimizes comparisons and overall performanceMulti touch support
(sorry, didn't find a better way to attach the screencasts)
Unit tests support
Removed unit test which no more needed 🙈
Added
PointerEventssupportWhat about browsers support?
It should work in all browsers including IE11: (and partly IE10): https://caniuse.com/pointer
Some screencasts:
Chrome: https://user-images.githubusercontent.com/1178825/127753055-25ba0bb7-8dd0-481d-8776-4f94bd0898a9.mp4
Firefox: https://user-images.githubusercontent.com/1178825/127752834-2da54a31-7c57-47a6-bfc8-3be510af0ace.mov
IPhone safari: https://user-images.githubusercontent.com/1178825/127752561-12864f6e-7cad-4a56-b3de-dd6b069eb568.MP4