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

Right click firing tap event on windows #70

Open
nathanmarks opened this issue May 3, 2016 · 10 comments
Open

Right click firing tap event on windows #70

nathanmarks opened this issue May 3, 2016 · 10 comments

Comments

@nathanmarks
Copy link

nathanmarks commented May 3, 2016

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
Copy link
Author

nathanmarks commented May 5, 2016

@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
Copy link
Author

nathanmarks commented May 5, 2016

I suppose the answer is not used:

facebook#6221 (comment)

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
Copy link

@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
Copy link
Author

@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
Copy link

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
Copy link
Author

@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
Copy link

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

@nathanmarks
Copy link
Author

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

@Ralle
Copy link

Ralle commented Feb 15, 2017

+1

@roryokane
Copy link

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.

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

No branches or pull requests

4 participants