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

Implement touch handling on web target(s) #1673

Closed
alvinhochun opened this issue Aug 22, 2020 · 5 comments · Fixed by #2188
Closed

Implement touch handling on web target(s) #1673

alvinhochun opened this issue Aug 22, 2020 · 5 comments · Fixed by #2188
Labels
DS - web S - platform parity Unintended platform differences

Comments

@alvinhochun
Copy link
Contributor

alvinhochun commented Aug 22, 2020

Adding touch event handling shouldn't be too hard, but it is tightly coupled with mouse handling due to the web using a unified Pointer Event model. Here are my thoughts:

  • Grabbing touch input from PointerEvent is simple enough. However, I would be concerned about correctly mapping the pointerType :
    • "mouse" - obviously should be mapped to CursorMove, CursorEntered, CursorLeft and MouseInput. In addition, I believe that only pointers with isPrimary === true should be reported.
    • "touch" - obviously should be mapped to Touch.
    • "pen" - this is tricky, as winit does not have an event for pen/stylus. I suggest that for now we just map pen input the same way as mouse input until winit finally supports pen events properly.
    • Empty strings or unknown values - the same as mouse.
  • Supporting browsers without PointerEvent - I suggest we do not add support for TouchEvent, because:
    • PointerEvent is getting widespread support.
    • Adding TouchEvent handling will likely make the code quite complicated.
    • If the user really care about supporting older browsers, they can use a JavaScript polyfill. Turns out the polyfill does not provide offsetX/offsetY which winit is currently using...
  • Should touch input also synthesize mouse input?
    • My idea would be that, since some users may not want to go through the trouble of implementing explicit touch handling there should be an option to allow user code to disable touch support and automatically treat the primary touch input as mouse input. Otherwise, touch input will only generate touch events and not mouse events.
    • However, it seems that this is not an option for other platforms. Perhaps it makes more sense to just provide one behaviour, which is for touch input to only generate touch events and not mouse events.
@ryanisaacg
Copy link
Contributor

I agree with your points on how to handle PointerEvent, and that we should avoid TouchEvent. I think there's an open meta issue somewhere for pen / styluses?

I think touch events from the browser should only generate touch events within winit. This is (I think?) consistent with other platforms, and allows the user to combine them if they want.

@ryanisaacg ryanisaacg added DS - web S - platform parity Unintended platform differences labels Aug 22, 2020
@alvinhochun
Copy link
Contributor Author

I think there's an open meta issue somewhere for pen / styluses?

Both #336 and #99 are related. (Somehow the latter is unsearchable on GitHub.) I would hope to push for changing to a unified pointer event model and add pen support as part of it, but that would be outside the scope of this issue.

@alvinhochun
Copy link
Contributor Author

It turns out the PEP polyfill doesn't work because it does not provide offsetX/offsetY which winit is currently using. (jquery-archive/PEP#217)

Still, I don't think it's really worth the effort to add TouchEvent support.

@ryo33
Copy link
Contributor

ryo33 commented May 25, 2022

Can anyone review #2188 ?

@ryo33
Copy link
Contributor

ryo33 commented May 27, 2022

Thanks for the review Liamolucko!
Then, when will this be merged? Does it need more review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DS - web S - platform parity Unintended platform differences
Development

Successfully merging a pull request may close this issue.

3 participants