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

Replace touch and mouse events with pointer events #74

Closed
12 of 14 tasks
Tracked by #94
nerdyman opened this issue Oct 5, 2022 · 9 comments · Fixed by #94
Closed
12 of 14 tasks
Tracked by #94

Replace touch and mouse events with pointer events #74

nerdyman opened this issue Oct 5, 2022 · 9 comments · Fixed by #94
Labels
enhancement New feature or request
Milestone

Comments

@nerdyman
Copy link
Owner

nerdyman commented Oct 5, 2022

The library currently uses mouse and touch event bindings from V1 when IE11 was supported. All currently targeted browsers support pointer events so they should be used instead.

See 5c84c5c (full file here) for a reference implementation.

Requirements

  • Add touchAction: 'none' style property to root element
  • Replace mousedown event binding with pointerdown
  • Remove touchstart event binding
  • Update mouse and touch handlers to replace MouseEvent | TouchEvent with PointerEvent
  • Remove instanceof ternaries in mouse and touch handlers, we can use ev.pageX and ev.pageY

Testing

  • Test on iOS Safari
  • Test on iOS Chrome
  • Test on Firefox Android
  • Test on Android Chrome
  • Test on Desktop Safari
  • Test on Desktop Chrome
  • Test on Desktop Firefox

TODO

@nerdyman nerdyman added this to the v3 milestone Oct 5, 2022
@nerdyman nerdyman added enhancement New feature or request hacktoberfest labels Oct 5, 2022
@muditchoudhary
Copy link

hey @nerdyman I would like to work on this. But could you describe a more like
What are the files where I need to replace touch/mouse events with a pointer?

@nerdyman
Copy link
Owner Author

nerdyman commented Oct 5, 2022

Hey @muditchoudhary! I've updated the ticket to include a link to a reference implementation and have added some requirements. I still need to do #87 before any Hacktoberfest tickets can be merged, I'll be sorting it out later today.

@muditchoudhary
Copy link

okay, No issue.

@nerdyman
Copy link
Owner Author

nerdyman commented Oct 5, 2022

This is ready to tackle @muditchoudhary , let me know if you need any more info.

@muditchoudhary
Copy link

This is ready to tackle @muditchoudhary, let me know if you need any more info.

okay, that's awesome, I'll start working today If I need any help I'll surely reach you.
Thanks!!

@muditchoudhary
Copy link

Hello, @nerdyman I'm getting this error when I replace MouseEvent | TouchEvent with PointerEvent :

image

But If I Put it like this function moveCall(ev: PointerEvent | MouseEvent) { the error has gone. So what should I do? should I put both PointerEvent & MouseEvent?

@muditchoudhary
Copy link

One more thing, where will I find the touchAction: 'none' property? I did not see any style file and I've also checked other files where I could find this. can you please tell me in which file do I need to check this?

@nerdyman
Copy link
Owner Author

@muditchoudhary Those events need removed/replaced with pointerup and pointermove events. I've updated the description for this issue to include links to the full file and also to the previously closed PR. Note that the linked PR also includes a new transition feature, that feature is not part of this ticket, just the pointer events changes listed in the description of this issue.

Here's a link to the diff for the pointer events in that PR. The styles for the root element are in rootStyles in the main component.

The linked PR should have everything needed to check the requirements for this PR. This isn't the simplest issue so if you'd like something else for Hacktoberfest #75 is also open.

@muditchoudhary
Copy link

@nerdyman If this issue requires the work of just replacing the events in the code I could do that. I'll try to fix this issue with the help of your PR. If I will not able to do it I'll tell you.

@nerdyman nerdyman mentioned this issue Nov 12, 2022
Merged
21 tasks
@nerdyman nerdyman linked a pull request Dec 6, 2022 that will close this issue
Merged
21 tasks
@nerdyman nerdyman closed this as completed Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants