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 · 3 comments
Open

Add touch drag support to NumberInput/HorizontallyScrollableArea #490

jo-chemla opened this issue May 14, 2024 · 3 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.

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