JedWatson / react-tappable

Tappable component for React
http://jedwatson.github.io/react-tappable/
MIT License
863 stars 90 forks source link

Question: onTap vs onClick. How to support both correctly? #41

Closed slorber closed 7 years ago

slorber commented 9 years ago

Hello

It seems that mobile browsers are able to handle click events. (However, iOS does some strange things see https://github.com/facebook/react/issues/2055)

So what I don't understand is what's the advantage of tap over click exactly?

In an application that should handle both mouse and touch events (responsive websites, or websites for laptops with touch screens), what should I do? use both onClick and onTap at the same time?

slorber commented 9 years ago
<Tappable onTap={this.handleClick}/>
   <MyComponent onClick={this.handleClick}/>
</Tappable>

Does this make any sens to you?

slorber commented 9 years ago

@JedWatson I have better insights in this now so I hope to answer some questions others might ask too.

Coming from many docs like

Maybe unlike you with TouchstoneJS, I have been building a desktop app that must support both touch and mouse events because we use the same code and responsive design and some new laptops does have both mouse/touch screens.

I have found some difficulties using Tappable with:

<Tappable onTap={this.handleClick}/>
   <MyComponent onClick={this.handleClick}/>
</Tappable>

This works but is not ideal because it always creates an intermediate span node, which can seriously mess up with our layout in some cases. Using it this way, many devs will tend to use the default span component, even if the children is a div (inline elements should only contain other inline elements).

I guess your initial idea is that the Tappable component is not supposed to be used as an intermediate component but as the component itself, as you forward all the props to the component provided.

This messes up a lot less with layout when used like that:

<Touchable component="div" className={toggleNotificationClasses} onTap={this.props.onToggleNotification} onClick={this.props.onToggleNotification}>
    <NotificationIcon/>
    {this.renderUnreadNum()}
</Touchable>

The click listener works fine. The problem is that onTap is also triggered on mouseUp (see https://github.com/JedWatson/react-tappable/issues/48).

The problem is that in some circonstances, I must be able to handle click and tap differently. For example I may use a different handler for touch and click. I may also need to let touch propagate but not click. This makes it more difficult because I now have to discriminate the event in the onTap handler while I think it would be more easy to deal with 2 separate handlers.

nightlyop commented 8 years ago

How about using onTouchTab of https://github.com/zilverline/react-tap-event-plugin for clicks and touches. I think the differentiation of a click and a touch is similar to knowing the device. So you could get the kind of device and use that for the differentiation.

slorber commented 8 years ago

Don't really understand what you mean @nightlyop but remember there are devices that have both touch and mouse support.

I think the correct way with Tappable is:

<Tappable component={MyComponent} className="class" onTap={this.handleTap} onClick={this.handleClick}>
    MyComponentChildren (optional)
</Tappable>

The problem is that when onTap is triggered on mouseUp. I'm currently running a fork of Tappable with this trigger removed and it works fine for me.

FaridSafi commented 8 years ago

I recommend https://github.com/zilverline/react-tap-event-plugin

dcousens commented 7 years ago

Just use onTap, you don't need onClick at all

slorber commented 7 years ago

@dcousens there are cases where you may want to differentiate between a click and a tap and not always handle the 2 the same way.

However I think the easiest would probably be to check the event type my callback receive on such case, so I can be fine if the lib triggers onTap for both events