zilverline / react-tap-event-plugin

Instant TapEvents for React
http://facebook.github.io/react/
MIT License
1.07k stars 110 forks source link

Right click firing tap event on windows #70

Open nathanmarks opened 8 years ago

nathanmarks commented 8 years ago

On windows the behaviour is not consistent with onClick. With a left click, both the onTouchTap and onClick fire, but with a right click the onTouchTap fires. This issue is not present on OS X.

Using the demo provided in this repo: (Tested on Windows 10 & Windows 8.1 in chrome & IE)

img

It is definitely a right click, and the onClick does not fire:

img

nathanmarks commented 8 years ago

@gaearon Is this used internally at Facebook anymore? Is this issue something that is unlikely to be fixed given the current situation with tap delay on iOS Safari finally being addressed?

nathanmarks commented 8 years ago

I suppose the answer is not used:

https://github.com/facebook/react/issues/6221#issuecomment-204371455

We make no guarantees about it not being broken. I think we should remove it from the core source code. But yeah, this means that its maintainers will need to track changes to internal APIs. We don't have a better solution right now.

madjam002 commented 8 years ago

@nathanmarks Was this ever used internally at Facebook? This is a community plugin derived from the original tap event plugin that is in the React source code.

I'd be happy to merge a pull request for this.

nathanmarks commented 8 years ago

@madjam002 I was referring to the source doe this is derived from, but great point! Even if that's the case it doesn't hinge on the FB source being updated. I just remembered the last PR was from gaearon to add react 15 compatibility and made some assumption that they had updated the code in FB core too.

I haven't looked into the cause yet -- any inkling?

madjam002 commented 8 years ago

Okay sure 😄 The code here is now quite different to what it is in the React repo.

This will be happening because we're not looking at the button that was pressed, it's just listening on mouse events for all buttons.

We could add something around here https://github.com/zilverline/react-tap-event-plugin/blob/master/src/TapEventPlugin.js#L138 to look at the native event button and reject the click if it's not a left mouse button press.

But I'm not sure if this is even desired behaviour, I imagine most people would want this..?

nathanmarks commented 8 years ago

@madjam002

Thanks for the brief -- Admittedly I hadn't really looked closely at the source. 😄

Are you saying most people would want the right click to trigger? On OS X it only fires for the left click, this feels most natural as it makes onTouchTap a drop-in replacement for onClick. I was under the impression that this was the desired behaviour. Otherwise one would have to add boilerplate code to all of their event handlers for touch taps.

In the example found in this repo, there is inconsistent behaviour across operating systems so whichever way around it is, I believe that it should be consistent.

madjam002 commented 8 years ago

Yeah if it's only left click on Mac we should make it consistent with Windows too.

nathanmarks commented 8 years ago

@madjam002 cool -- i'll try find the time to have a quick fiddle

Ralle commented 7 years ago

+1

roryokane commented 6 years ago

This issue caused an annoying bug in my app. In my app, pressing the right mouse button over certain elements opens a Material UI context menu, in which you are meant to left-click the menu item you want to activate. The bug is that releasing the right mouse button would trigger an onTouchTap event, immediately triggering any menu item under the cursor.

This only happened if the cursor didn't move or moved only a little during the right-click. If you're wondering how a context menu item could be under the cursor without it moving, it happens when the cursor is near the edge of the window and the menu is shifted to stay onscreen.

I worked around this in my app by changing the onTouchTap prop to onClick, since my app doesn't need touch support that badly.