Skip to content
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 touch drag support to NumberInput/HorizontallyScrollableArea #490

Open
jo-chemla opened this issue May 14, 2024 · 4 comments
Open

Add touch drag support to NumberInput/HorizontallyScrollableArea #490

jo-chemla opened this issue May 14, 2024 · 4 comments

Comments

@jo-chemla
Copy link

It would be great if the BasicNumberInput could support dragging for touchscreen/touch events the same way we can drag these number inputs with the mouse to increase/decrease the prop like a slider.

A solution could be introducing touchstart and touchmove event listeners in r3f/src/extension/components/DragDetector.tsx#L23-L24

However, since there are equivalences between mousedown === touchstart, mousemove === touchmove, mouseup === touchend, it should not be mandatory. Might be because touch events can be multiple, so a target has to be selected first using glue-code like the following from this SO answer:

target = /touch/.test(event.type) ? event.targetTouches[0] : event.target;

Should this be introduced here in the HorizontallyScrollableArea?

@AriaMinaei
Copy link
Member

It would be great if the BasicNumberInput could support dragging for touchscreen/touch events

Agreed! The best way to do this would be to make useDrag() use touch events as well, so touch events will be supported in components other than BasicNumberInput too.

@jo-chemla
Copy link
Author

So simply switching the signature of both dragHandler and dragEndHandler to accept MouseEvent arrays, and selecting the first event of the array could do the trick?

- const dragHandler = (event: MouseEvent) => {
+ const dragHandler = (event: MouseEvent | MouseEvent[]) => {
+   const event_ = (Array.isArray(event)) ? event[0] : event;

- const dragEndHandler = (e: MouseEvent) => {
+ const dragEndHandler = (e: MouseEvent | MouseEvent[]) => {
+   const e_ = (Array.isArray(e)) ? e[0] : e;

@jo-chemla
Copy link
Author

jo-chemla commented Jul 25, 2024

I made an attempt at fixing the useDrag to also support touch. It seems that the easiest trick is to convert all mouse-* event handlers to pointer- events handlers. This is better than adding touch-* events in parallel to mouse-* because these handlers handle events differently - there might be multiple touch events happening at the same time, and event properties are renamed.

This is a bit out of my area of expertise, + these draghandlers are a bit heavy. At the moment, the touch event is almost working, although it seems to be captured differently - on a touch simulator in chrome dev tools, I can only do micro-increments of the slider. And once the pointer is released, at the moment the cursor always ends-up at the center of the screen. Hopefully this can make it into the 1.0 release, will try to investigate a bit more and report back!

- target.addEventListener('mousedown', onMouseDown as $FixMe)
+ // target.addEventListener('touchstart', onMouseDown as $FixMe)
+ target.addEventListener('pointerdown', onMouseDown as $FixMe)
+ // target.addEventListener('touchstart', onMouseDown as $FixMe)
+ ...
+ target.removeEventListener('pointerdown', onMouseDown as $FixMe)


const addDragListeners = () => {
-  document.addEventListener('mousemove', dragHandler)
-  document.addEventListener('mouseup', dragEndHandler)
+ // document.addEventListener('touchmove', dragHandler)
+ // document.addEventListener('touchend', dragEndHandler)
+ document.addEventListener('pointermove', dragHandler)
+ document.addEventListener('pointerup', dragEndHandler)
+ // document.addEventListener('touchmove', dragHandler)
+ // document.addEventListener('touchend', dragEndHandler)
}

I think the different handling of touch and mouse is now due to the DRAG_DETECTION_DISTANCE_THRESHOLD which is set up to differentiate between click and drag events to capture pointer for mouse. Might need to do something smarter than simply using pointer-* events, because pointer events in case of touch do increment by very little steps of a few pixels, often below that threshold - which might explain why it is less correctly captured.

@jo-chemla
Copy link
Author

jo-chemla commented Aug 14, 2024

I tested (via npm run playground on the dom example) the above described code modification, modifying the useDrag to convert all mouse-* event handlers to pointer-* handlers and this still does work on standard mouse. I also tested on a real touchscreen on chrome on a Surface Hub (rather than emulated touch because the cursor capture seem to break stuff) and it does work to some extent!

I can via touch modify all slideable controls by small increments (sliders, numbers inputs etc). For some reason I can't explain, when the drag exceeds a given amount of pixels, the onmove events stop firing although the pointerup event do not. I'll transfer it back to your team who knows that useDrag subtleties way better than I do - seems like a quite complex code design.

I've made a PR with my changes here #499 stilla wip to add touch support to useDrag for sliders, number inputs etc

  • Convert all MouseEvent to PointEvent
  • Edit all event listeners from mouse-* to pointer-* like pointerdown, pointermove, pointerup
  • Add logs to try to debug touch not working above a given threshold displacement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants