Implement useMove, use it in Slider & SplitView #1106

mischnic · 2020-09-29T09:38:28Z

Depends on #1083

Closes #1063, Closes #1018
Closes #1069 (useDrag1D is now unused, I've includes the tests in this PR)

adobe-bot · 2020-09-29T18:19:52Z

Build successful! 🎉

packages/@react-aria/interactions/src/useMove.ts

adobe-bot · 2020-09-30T10:37:06Z

Build successful! 🎉

adobe-bot · 2020-10-01T21:36:57Z

Build successful! 🎉

packages/@react-aria/interactions/src/useMove.ts

devongovett · 2020-10-02T01:11:50Z

packages/@react-aria/interactions/src/useMove.ts

+
+    if (typeof PointerEvent === 'undefined') {
+      let onMouseMove = (e: MouseEvent) => {
+        move('mouse', e.pageX - state.current.previousPosition.pageX, e.pageY - state.current.previousPosition.pageY);


I think you may need to ignore emulate mouse events on touch devices here. Haven't tested but my guess is that onMouseMove will be fired after onTouchMove and potentially in other cases. You can set a flag in touch events and ignore the following mouse events like we do in usePress.

I'd also add a check for e.button === 0 to only support dragging with the left mouse button as well. Same in pointer events below.

I think you may need to ignore emulate mouse events on touch devices here.

Works on Chrome Android, but maybe for iOS?

packages/@react-aria/interactions/src/useMove.ts

devongovett · 2020-10-02T01:54:14Z

packages/@react-aria/slider/src/useSlider.ts

-    })
+      onMouseDown(e: React.MouseEvent<HTMLElement>) { onDownTrack(e, e.clientX); },
+      onPointerDown(e: React.PointerEvent<HTMLElement>) { onDownTrack(e, e.clientX); },
+      onTouchStart(e: React.TouchEvent<HTMLElement>) { onDownTrack(e, e.touches[0].clientX); }


Will this cause onDownTrack to be called more than once on touch devices due to emulated mouse events/support for both touch and pointer events? Should we use usePress here?

I tried to use usePress but client/page X/Y aren't passed onto the handlers.

devongovett · 2020-10-02T02:04:35Z

packages/@react-stately/slider/src/useSliderState.ts

@@ -105,20 +110,22 @@ export function useSliderState(props: Omit<SliderProps, 'direction'>): SliderSta

    // Round value to multiple of step, clamp value between min and max
    value = clamp(getRoundedValue(value), thisMin, thisMax);
-    setValues(values => replaceIndex(values, index, value));
+    valuesRef.current = replaceIndex(valuesRef.current, index, value);
+    setValues(valuesRef.current);


What wasn't working here?

This test fails when changing it back:

FAIL component and hooks packages/@react-aria/slider/test/useSliderThumb.test.js ● useSliderThumb › interactions on thumbs, where track contains thumbs › using KeyEvents › can be moved with keys expect(jest.fn()).toHaveBeenLastCalledWith(...expected) - Expected + Received Array [ - 11, + 10, ], Number of calls: 1 358 | expect(onChangeSpy).toHaveBeenLastCalledWith([11]); 359 | expect(onChangeSpy).toHaveBeenCalledTimes(1); > 360 | expect(onChangeEndSpy).toHaveBeenLastCalledWith([11]); | ^ 361 | expect(onChangeEndSpy).toHaveBeenCalledTimes(1); 362 | expect(stateRef.current.values).toEqual([11]); 363 | }); at Object.toHaveBeenLastCalledWith (packages/@react-aria/slider/test/useSliderThumb.test.js:360:32)

mischnic · 2020-10-02T13:17:52Z

Most of the issues and TODO comments went away automatically after adding preventDefault() and using the touch/pointer id. There is now no global state at all anymore (that currentTarget).

Now multitouch on sliders works, I added two stories 🎉

adobe-bot · 2020-10-02T13:28:15Z

Build successful! 🎉

adobe-bot · 2020-10-02T17:18:56Z

Build successful! 🎉

adobe-bot · 2020-10-02T17:38:15Z

Build successful! 🎉

devongovett

Multi touch looking good!

packages/@react-aria/interactions/src/useMove.ts

adobe-bot · 2020-10-05T08:37:57Z

Build successful! 🎉

The base branch was changed.

adobe-bot · 2020-10-06T01:25:35Z

Build successful! 🎉

mischnic requested review from dannify, devongovett and snowystinger as code owners September 29, 2020 09:38

mischnic force-pushed the useMove branch from 807c8e9 to 782080d Compare September 29, 2020 12:20

mischnic commented Sep 29, 2020

View reviewed changes

packages/@react-aria/interactions/src/useMove.ts Outdated Show resolved Hide resolved

This was referenced Sep 30, 2020

Implement ColorArea component #1118

Closed

Implement ColorSlider component #1119

Closed

Implement ColorWheel component #1120

Closed

mischnic force-pushed the mischnic/slider branch from 31f3fc6 to 05d959a Compare September 30, 2020 10:21

mischnic force-pushed the useMove branch from 8bf3bdf to 0251d53 Compare September 30, 2020 10:21

mischnic force-pushed the mischnic/slider branch from 05d959a to 8b91785 Compare September 30, 2020 10:22

mischnic force-pushed the useMove branch from 0251d53 to 8aca436 Compare September 30, 2020 10:23

mischnic force-pushed the mischnic/slider branch from 8b91785 to 2e0fad6 Compare October 1, 2020 20:44

mischnic force-pushed the useMove branch from 8aca436 to 19750b1 Compare October 1, 2020 20:44

mischnic force-pushed the mischnic/slider branch from d5c8240 to ff5a10e Compare October 1, 2020 21:13

mischnic force-pushed the useMove branch from 19750b1 to 436ec42 Compare October 1, 2020 21:22

mischnic mentioned this pull request Oct 1, 2020

Implement Color Wheel #1129

Merged

devongovett reviewed Oct 2, 2020

View reviewed changes

mischnic force-pushed the mischnic/slider branch from ff5a10e to 168b011 Compare October 2, 2020 08:31

mischnic force-pushed the useMove branch 2 times, most recently from 6ffe7f8 to a951c0c Compare October 2, 2020 09:45

This comment has been minimized.

Sign in to view

mischnic force-pushed the useMove branch from ce1cef6 to adf356d Compare October 2, 2020 17:04

mischnic force-pushed the mischnic/slider branch from 893d8ba to 218e1ce Compare October 2, 2020 17:18

mischnic force-pushed the useMove branch from adf356d to cca1092 Compare October 2, 2020 17:23

devongovett previously approved these changes Oct 3, 2020

View reviewed changes

packages/@react-aria/interactions/src/useMove.ts Outdated Show resolved Hide resolved

mischnic force-pushed the mischnic/slider branch from 218e1ce to c0e4d59 Compare October 4, 2020 22:53

mischnic force-pushed the useMove branch from cca1092 to 62fd055 Compare October 4, 2020 22:53

Base automatically changed from mischnic/slider to main October 6, 2020 01:08

mischnic and others added 12 commits October 5, 2020 18:12

Implement useMove

a73cf94

Slider: use useMove

b7dc97e

Refactor testing utilities for Event shims

10477e5

Update React testing library

1fcbcfa

Pull in some more tests

504f449

Remove onMouseDown from SplitView

f3296e3

Remove SplitView V2 parity tests

574de21

Text selection, stop events

3a724de

Multitouch

cf84661

Cleanup global handlers on unmount

926c64c

Do setThumbDragging properly

d0d1da7

Fix onChangeEnd for key events

5d5756f

devongovett force-pushed the useMove branch from d96f8af to 5d5756f Compare October 6, 2020 01:12

devongovett approved these changes Oct 6, 2020

View reviewed changes

devongovett merged commit dc6d787 into main Oct 6, 2020

devongovett deleted the useMove branch October 6, 2020 01:48

devongovett mentioned this pull request Dec 19, 2020

useMove: dragging and releasing outside iframe doesn't end drag #1406

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement useMove, use it in Slider & SplitView #1106

Implement useMove, use it in Slider & SplitView #1106

mischnic commented Sep 29, 2020 •

edited

Loading

adobe-bot commented Sep 29, 2020

adobe-bot commented Sep 30, 2020

adobe-bot commented Oct 1, 2020

devongovett Oct 2, 2020

devongovett Oct 2, 2020

mischnic Oct 2, 2020

devongovett Oct 2, 2020

mischnic Oct 2, 2020

devongovett Oct 2, 2020

mischnic Oct 2, 2020

This comment has been minimized.

mischnic commented Oct 2, 2020 •

edited

Loading

adobe-bot commented Oct 2, 2020

adobe-bot commented Oct 2, 2020

adobe-bot commented Oct 2, 2020

devongovett left a comment

adobe-bot commented Oct 5, 2020

adobe-bot commented Oct 6, 2020

Implement useMove, use it in Slider & SplitView #1106

Implement useMove, use it in Slider & SplitView #1106

Conversation

mischnic commented Sep 29, 2020 • edited Loading

adobe-bot commented Sep 29, 2020

adobe-bot commented Sep 30, 2020

adobe-bot commented Oct 1, 2020

devongovett Oct 2, 2020

Choose a reason for hiding this comment

devongovett Oct 2, 2020

Choose a reason for hiding this comment

mischnic Oct 2, 2020

Choose a reason for hiding this comment

devongovett Oct 2, 2020

Choose a reason for hiding this comment

mischnic Oct 2, 2020

Choose a reason for hiding this comment

devongovett Oct 2, 2020

Choose a reason for hiding this comment

mischnic Oct 2, 2020

Choose a reason for hiding this comment

This comment has been minimized.

mischnic commented Oct 2, 2020 • edited Loading

adobe-bot commented Oct 2, 2020

adobe-bot commented Oct 2, 2020

adobe-bot commented Oct 2, 2020

devongovett left a comment

Choose a reason for hiding this comment

adobe-bot commented Oct 5, 2020

adobe-bot commented Oct 6, 2020

mischnic commented Sep 29, 2020 •

edited

Loading

mischnic commented Oct 2, 2020 •

edited

Loading