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

Prevented default in onPointerDown does not trigger onOutsideClick #16

Open
joux3 opened this issue Nov 14, 2018 · 7 comments
Open

Prevented default in onPointerDown does not trigger onOutsideClick #16

joux3 opened this issue Nov 14, 2018 · 7 comments

Comments

@joux3
Copy link

joux3 commented Nov 14, 2018

Repro: https://codesandbox.io/s/p9jl7jq627

Clicking on a div with mouseDown default preventer triggers onOutsideClick, while clicking on a div with pointerDown default preventer does not.

This is because mouseDown will not be fired if the default is prevented in pointerDown, see step 6 under 7.1 in Pointer Events Specification.

Is this something that this library should support? I tried switching mouseDown -> pointerDown and mouseUp -> pointerUp and everything seemed to work like it should.

@ljharb
Copy link
Collaborator

ljharb commented Nov 14, 2018

Pointer events don’t work in Safari or fully in IE 10, so I’m not sure it’s a good idea to use them at all yet.

If there’s a way to make this library work with them, however, it’d be great to fix that.

@joux3
Copy link
Author

joux3 commented Nov 19, 2018

Pointer events don’t work in Safari or fully in IE 10, so I’m not sure it’s a good idea to use them at all yet.

Good point, just changing the handler from mouseDown to pointerDown will not cut it. By the way in this case pointer events are used in a component we depend on.

If there’s a way to make this library work with them, however, it’d be great to fix that.

I can think of one solution: handle both mouseDown/mouseUp and pointerDown/pointerUp pairs. But on the first pointerDown unbind mouseDown handling as it is not needed. Would a patch doing that be accepted?

@ljharb
Copy link
Collaborator

ljharb commented Nov 19, 2018

Having the mouse event handlers be conditional seems like a bug farm; could instead we install both, and code it such that it both expects and handles either or both being called?

@joux3
Copy link
Author

joux3 commented Nov 19, 2018

That does sound like a safer approach. I have some code in my fork that adds handlers for pointerDown and pointerUp.

The easy cases are when only pointerDown/pointerUp or mouseDown/mouseUp get called, not both. Do you have any ideas for handling the case of both being called? I can think of a couple of ways but neither of them are very clean IMO:

  1. do some sort of timestamp comparing and avoid calling onOutsideClick again "too soon"
  2. setTimeout(..., 0) for detecting a event loop boundary and only firing onOutsideClick again after it has happened

The option 2 seems like a better option though. Any thoughts?

@ljharb
Copy link
Collaborator

ljharb commented Nov 19, 2018

I’m not sure, neither of those sound great.

@joux3
Copy link
Author

joux3 commented Nov 22, 2018

I did come up with a third option: use Event.timeStamp to detect when both onPointerUp and onMouseUp fire.

@majapw
Copy link
Collaborator

majapw commented Nov 30, 2018

Hmm, I'll take a look at this today. It looks like using mousedown instead of pointerdown is also causing some issues with newer chrome + iOS.

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

3 participants