Enable selection of time ranges on touch devices

[parttimenerd]

Handles touchstart events like mouseclick events and touchmove like mousemove events thereby enabling the selection using touch events only. It does not support multi-touch and just takes the first touch.

Fixes #4208.

Deploy preview

[codecov[bot]]

Codecov Report

Merging #4210 (5c01bb0) into main (6c371c3) will increase coverage by 0.00%. The diff coverage is 95.45%.

:exclamation: Current head 5c01bb0 differs from pull request most recent head 0382aa1. Consider uploading reports for the commit 0382aa1 to get more accurate results

@@           Coverage Diff           @@
##             main    #4210   +/-   ##
=======================================
  Coverage   88.43%   88.43%           
=======================================
  Files         280      280           
  Lines       24564    24572    +8     
  Branches     6557     6560    +3     
=======================================
+ Hits        21722    21730    +8     
  Misses       2640     2640           
  Partials      202      202           

Impacted Files	Coverage Δ
src/components/timeline/Selection.js	80.15% <95.45%> (+1.29%)	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[whimboo]

Thanks a lot! The preview page for the profiler works perfectly with touch now. I can successfully set time ranges and also slide the divider up and down.

[julienw]

Without looking much closer, I wonder if this would be a good idea to move to pointer events instead of mouse/touch events. What do you think?

[parttimenerd]

I will evaluate it (I forgot that pointer events exist).

[parttimenerd]

Pointer events are too limited. We would have to distinguish between the different pointer types with these events too. Using them therefore does not help.

[julienw]

Pointer events are too limited. We would have to distinguish between the different pointer types with these events too. Using them therefore does not help.

Can you please be some more specific? What do you think that is missing from pointer events? From what I see they seem do the job just fine but I may be missing something :-)

[parttimenerd]

They are missing the position information. One has to distinguish between the different event types: https://developer.mozilla.org/en-US/docs/Web/API/Element/pointerdown_event

The only advantage is that we reduce the number of added event handler (just one for each pointer event, instead of one for mouse and touch events for each). The rest of the code should not really be affected.

[julienw]

They are missing the position information.

I believe they're here. MDN says that this interface inherits properties from [MouseEvent](https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent) and [Event](https://developer.mozilla.org/en-US/docs/Web/API/Event). So they should have pageX directly, and you shouldn't have to check for touches. Actually I made this code to double check this, this works both with touch and mouse: https://codepen.io/julienw/pen/vYRozmw

The only advantage is that we reduce the number of added event handler (just one for each pointer event, instead of one for mouse and touch events for each). The rest of the code should not really be affected.

I believe it's wrong: you wouldn't need all these new if checks.

That was the whole point of the pointer events when this was implemented some years ago: provide a common interface that works for both touch and mouse events.

[julienw]

You can use isPrimary to replace the test for the left button though :-)

[parttimenerd]

Thanks for the correction, I must have misread the paragraph. I'll alter the code accordingly tomorrow.

[parttimenerd]

Regarding your bug: I don't see this bug using a mouse. But this could be due to my latest fixes.

[parttimenerd]

There is still a single failing test suite related to hovering.

[julienw]

After testing and discussion, we agreed it was a good idea to move back to the first commit of this patch set. Indeed pointer events do not seem to work reliably in a crossbrowsing way, at least in this case.

To finish it, we need to remove the registering of "touchstart" in _installMoveAndClickHandlers, but also do the same change in Draggable.js to support the use case of moving and editing the selection after one is done.

[julienw]

I'm closing it for now to unclutter the list of PR and because nobody works on this at the moment. Feel free to reopen or use the commits here for a future fix.

[parttimenerd]

This isn't the a priority for me either. I could work on if there is a real use case with enough users.