Skip to content

[added] Fast touch clicks #495

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

Closed
wants to merge 4 commits into from
Closed

[added] Fast touch clicks #495

wants to merge 4 commits into from

Conversation

kenwheeler
Copy link

  • Added functionality that allows Link components to respond to the TouchEnd event, and not also fire the resulting click event.

[DONT MERGE]

Because tests are done in Firefox, I can't simulate touch events in order to test this. Did you have a recommendation?

@mjackson
Copy link
Member

@kenwheeler Although I've never done it, I believe we can use React.addons.TestUtils.Simulate to simulate touch events in all browsers. You may need to use React.initializeTouchEvents somewhere in the test harness to enable support for touch events.

@kenwheeler
Copy link
Author

Ok I'll check that out and get this updated, thanks.

@mjackson
Copy link
Member

Just a thought: When this work is done it may be useful to include all the special click/touch handling in the Navigation mixin. Not sure tho. Would it be re-usable elsewhere? Or is this all very specific to <a> tags?

@kenwheeler
Copy link
Author

Ok looks like that worked for the tests. In regards to the Navigation mixin, how would you envision that working? I would imagine that onClick & onTouchEnd would have to be manually added to a component and point at a method with a required naming convention like handleClick.

@kenwheeler
Copy link
Author

Amended. If this looks ok to you, I'll squash these into a single commit

@mjackson
Copy link
Member

Thanks @kenwheeler, this looks great. Let's go ahead and squash them!

The only question left in my mind is: do we always want to treat touchend the same as click? I don't think we do. Semantically, they're different. A click in touch terms is actually a touchstart + touchend in sequence in the same spot. Is anyone making higher-level "tap", "pinch", etc. mixins for React components? That would be really useful here. Then we could just mixin Tappable and get a little more accurate.

@kenwheeler
Copy link
Author

@mjackson Looked into how the touchstart/touchend relationship is working in React and it looks like if you start on target but end off target it still fires, but if you start off target it won't. I've seen a couple wrappers like https://github.com/dogfessional/react-touchable and https://github.com/JedWatson/react-tappable.

@mjackson
Copy link
Member

it looks like if you start on target but end off target it still fires, but if you start off target it won't

Huh, that's interesting. So you're saying that touchend fires when you start the touch on the target and end it off the target? I would expect the opposite to be true... or maybe I just don't understand touch events...

@kenwheeler
Copy link
Author

@mjackson funny right? Try the hamburger on postocracy.com on mobile/tablet to see what I mean. Seems backwards.

@kenwheeler
Copy link
Author

@mjackson
Copy link
Member

@kenwheeler Wow, I see what you mean.

I'm honestly not sure how to proceed here. We could try and roll our own touch support based on the work in react-tap-event-plugin (or just make it a dep and have a separate build for touch?) or we could just suggest users use react-tap-event-plugin together with Router.Navigation and Router.State until React core has better touch support.

I guess a 3rd option would be to just merge this in the short term with a caveat that anyone who uses it may have sketchy touch support.

@rpflorence What do you think?

@jonasi
Copy link
Contributor

jonasi commented Nov 23, 2014

If you do pull this in, please at least make it configurable via some sort of options flag.

Not saying that this will cause any problems, but touch issues can be tedious to debug. I'd like to not have to worry about what's happening in my routing library if something becomes an issue in my app.

@ryanflorence
Copy link
Member

Our <Link/> is nothing more than an <a href/>.

Any behavior that doesn't match exactly what an anchor does, I'd rather not try to do with <Link/>.

We have the right mixins for folks to create their own <TappableLink/> or whatever they want.

👎

@kenwheeler
Copy link
Author

Agreed. I'll wait for React to address it.

@kenwheeler kenwheeler closed this Nov 23, 2014
@ryanflorence
Copy link
Member

Just FYI, with this implementation, if I start a scroll on a link, scroll 200 pixels with my finger, and then stop, the link will navigate.

It's all so tricky.

@ryanflorence
Copy link
Member

@kenwheeler also, thanks for being involved :)

@slorber
Copy link

slorber commented Apr 6, 2016

Just passing by, I'm using React-tappable and it works great.

I've not tested yet but I think it can be used this way:

<Tappable component={Link} to="route" onTap={...}>

So it should be pretty easy to build a "TappableLink" abstraction

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants