Improve "outside click" behaviour in combination with 3rd party libraries #2572
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is necessary for calculating the target where the focus will eventually move to. Some other libraries will use an `event.preventDefault()` and if we are not listening for all "down" events then we might not capture the necessary target. We already tried to ensure this was always captured by using the `capture` phase of the event but that's not enough. This change won't be enough on its own, but this will improve the experience with certain 3rd party libraries already.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
thecrypticace
approved these changes
Jul 3, 2023
|
Hey Robin. I was on 1.7.13 (vue) before and now I have a bug with 1.7.16. I have multiple slide-overs, two Dialogs to be specific. When I touch/click in the last opened, the first one gets closed. Going back to 1.7.13 for now but anyhow I have to say my thanks for this awesome library. It saved me ton of work and I love it. |
RobinMalfait
added a commit
that referenced
this pull request
Jan 9, 2024
When using a `Dialog`, we should prevent the default behaviour of the event that triggered the "close" in the `useOutsideClick` call. We recently made improvements to improve outside click behaviour on touch devices (#2572) but due to the `touchend` event, the touch is still forwarded and therefore a potential button _behind_ the "backdrop" will also be clicked. This is not what we want. Added the `event.preventDefault()` for the Dialog specifically because there are other places where we use `useOutsideClick` and where we _do_ want the behaviour where the click just continues. A concrete example of this is 2 `Menu`'s next to eachother where you open the first one, and then click on the second one. This should close first one (outside click) and open the second one (by not preventing the event)
RobinMalfait
added a commit
that referenced
this pull request
Jan 9, 2024
…2919) * use `event.preventDefault()` in the `useOutsideClick` on Dialog's When using a `Dialog`, we should prevent the default behaviour of the event that triggered the "close" in the `useOutsideClick` call. We recently made improvements to improve outside click behaviour on touch devices (#2572) but due to the `touchend` event, the touch is still forwarded and therefore a potential button _behind_ the "backdrop" will also be clicked. This is not what we want. Added the `event.preventDefault()` for the Dialog specifically because there are other places where we use `useOutsideClick` and where we _do_ want the behaviour where the click just continues. A concrete example of this is 2 `Menu`'s next to eachother where you open the first one, and then click on the second one. This should close first one (outside click) and open the second one (by not preventing the event) * update changelog
RobinMalfait
added a commit
that referenced
this pull request
Apr 15, 2024
…2919) * use `event.preventDefault()` in the `useOutsideClick` on Dialog's When using a `Dialog`, we should prevent the default behaviour of the event that triggered the "close" in the `useOutsideClick` call. We recently made improvements to improve outside click behaviour on touch devices (#2572) but due to the `touchend` event, the touch is still forwarded and therefore a potential button _behind_ the "backdrop" will also be clicked. This is not what we want. Added the `event.preventDefault()` for the Dialog specifically because there are other places where we use `useOutsideClick` and where we _do_ want the behaviour where the click just continues. A concrete example of this is 2 `Menu`'s next to eachother where you open the first one, and then click on the second one. This should close first one (outside click) and open the second one (by not preventing the event) * update changelog
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves the outside click behaviour when using our components in combination with 3rd party libraries. It also improves the outside click behaviour on touch devices.
We already use some logic to try and capture the target element outside of the main
clicklistener. We do this by listening to amousedownon thedocumentwhile using thecapturephase (instead of the bubble phase). Some 3rd party tools already use enevent.preventDefault()in these scenarios.To try and solve this, we will also listen for
pointerdownevents to capture the target element instead of only relying on themousedownevent. We can't just switch topointerdownalone because those events aren't available on older versions of iOS for example.Another small improvement is to also listen for
touchendevents. This will also be triggered on mobile devices which also improves the "outside click" behaviour in case themousedownevent is usinge.preventDefault().Fixes: #2564