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

Added support for touch-based (mobile) events. #12

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

leopoldjoy
Copy link
Contributor

No description provided.

@leopoldjoy
Copy link
Contributor Author

@unclecheese Note, I added a note at the bottom of README.md linking to my extended-features fork. Please let me know if you would prefer for me to remove this.

@leopoldjoy
Copy link
Contributor Author

@unclecheese Please let me know if you would like any additional changes made to the touch-device functionality.

_openSelector (e) {
_openSelector (e) {
if(this.state.mouseMoveStarted) return;
this.state.mouseMoveStarted = true;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't the correct way to mutate state.

Does this really need to be in state? If it doesn't affect the UI, then probably not.

@leopoldjoy
Copy link
Contributor Author

@unclecheese Thanks for the feedback! I made the syntactic changes and moved the mouse event tracking booleans out of the state.

In regards to your question of why we need to track the events: some devices are both click and touch enabled. On such devices the event handlers could fire twice. For example:

ReactDOM.findDOMNode(this).addEventListener('mousedown', this._mouseDown);
ReactDOM.findDOMNode(this).addEventListener('touchstart', this._mouseDown);

Additionally, for the mousemove/touchmove handlers, they could be doubly firing. Due to these inconsistencies of which events fire or are supported across platforms, I thought it best to guarantee that the functions are always fired in the correct order and not duplicated. What are your thoughts on this? I am open to altering this if you feel differently about it.

@leopoldjoy
Copy link
Contributor Author

Sorry, I had forgot to rebuild the bundle. Committed that now. I'm now working on updating the gh-pages branch so that the example has the new touch functionality. I will push shortly.

@leopoldjoy
Copy link
Contributor Author

Ok, everything is cleaned up and the gh-pages branch is rebundled. Please let me know if you feel that any other changes should be made.

@leopoldjoy
Copy link
Contributor Author

@unclecheese Please let me know if there are any additional changes you feel should be made.

@unclecheese
Copy link
Owner

Thanks! I'll have a look at it in the next few days.

@leopoldjoy
Copy link
Contributor Author

@unclecheese Did you get a chance to look through the changes? Let me know if you feel any changes should be made.

@aldahick
Copy link

aldahick commented Jun 4, 2018

@unclecheese Any update on this? Love the library, but mobile users are out of luck without this.

@shubham-2710
Copy link

Any Update on this for mobile touch ?

@danielmockaitis
Copy link

Can we please mobile support with this?

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

Successfully merging this pull request may close these issues.

5 participants