Cancel outside click behavior on touch devices when scrolling #3266
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
When on a touch device, then the `touchend` event will fire, even if you scrolled a bit and scrolling was your intention. This now tracks that touches were at least 30px apart in either the X or Y direction. If that's the case, then we do not consider it an outside click.
This now takes the new `enabled` value into account.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
RobinMalfait
changed the title
RobinMalfait
commented
Jun 3, 2024
Comment on lines
-21
to
-32
| useEffect( | ||
| process.env.NODE_ENV === 'test' | ||
| ? () => { | ||
| enabledRef.current = isTopLayer | ||
| } | ||
| : () => { | ||
| requestAnimationFrame(() => { | ||
| enabledRef.current = isTopLayer | ||
| }) | ||
| }, | ||
| [isTopLayer] | ||
| ) |
There was a problem hiding this comment.
The linked issue was expected behavior, since then we've also restructured some of this logic and this doesn't happen anymore.
The issue we were seeing was that opening a <Dialog /> by setting state in an onClick, registered a click listener on the window and immediately triggered it so the <Dialog /> was immediately closed.
Comment on lines
+27
to
+34
| "vue": "^3.4.27", | ||
| "vue-flatpickr-component": "^9.0.5", | ||
| "vue-router": "^4.0.0" | ||
| "vue-router": "^4.3.2" | ||
| }, | ||
| "devDependencies": { | ||
| "@floating-ui/vue": "^1.0.2", | ||
| "@vitejs/plugin-vue": "^3.0.3", | ||
| "vite": "^3.0.0" | ||
| "@vitejs/plugin-vue": "^5.0.5", | ||
| "vite": "^5.2.12" |
There was a problem hiding this comment.
Noticed that playgrounds for Vue were broken, bumping these dependencies fixed the playground again.
thecrypticace
approved these changes
Jun 3, 2024
This was referenced Jul 13, 2024
This was referenced Aug 29, 2024
This was referenced Sep 6, 2024
This was referenced Sep 10, 2024
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 behavior on touch devices.
Before this change, if a
<Menu />or<Dialog />was open, then "scrolling" or "swiping" your finger on the screen would close the<Menu />or<Dialog />respectively.This is because we also checked for
touchendevents (which are fired when you lift your finger). Right now, we will make sure that your finger has to move at least some amount (set it to 30px for now, which felt good while testing on mobile devices) in order to cancel the outside click behaviour. If you didn't move within 30px then we consider it an outside click.Now, scrolling doesn't close the component, but a tap (
touchend) will close it as expected.Fixes: #3038