Fix carousel "hover" behavior on touch-enabled devices #22442
Merged
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
- touch events are enabled not just on "mobile", just also on touch-enabled desktop/laptop devices; additionally, it's possible to pair a mouse with traditionally touch-only devices (e.g. Android phones/tablets); currently, in these situations the carousel WON'T pause even when using a mouse
patrickhlauke
requested review from
cvrebert,
hnrch02,
bardiharborow and
Johann-S
|
This PR does not contain a unit test, as I can't figure out how this new behavior could even be tested...any suggestions welcome... |
as `mouseenter` is fired as part of the touch compatibility events, the previous change results in carousels which cycle until the user tapped/interacted with them. after that they stop cycling (as `mouseleave` is not sent to the carousel after user scrolled/tapped away). this fix resets the cycling after `touchend` - essentially returning to the previous behavior, where on touch the carousel essentially never pauses, but now with the previous fix it at least pauses correctly for mouse users on touch-enabled devices. includes documentation for this new behavior.
Closed
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.
Currently,
mouseenter/mouseleavepause the carousel, but these listeners are not added if it's a touch-enabled device. This causes problems on touch-enabled laptop/desktop devices, or when a mouse is paired with, say, an Android phone/tablet, as then the mouse listeners are not added and the carousel does not pause as it should when the user interacts with the mouse.This PR modifies the behavior: mouse listeners are always added. Additionally, on touch devices, an extra
touchendlistener is included. Mouse users on touch-enabled devices now can pause the carousel as expected. If the user interacts with the carousel with the touchscreen, the carousel is now paused for the duration ofthis._config.interval(plus an extra 500 milliseconds, just to make sure we always wait until the mouse compat events have definitely been fired - which is what made the original code necessary in the first place since touch events also fire amouseentercompatibility event), and then the cycling is explicitly restarted.This now allows touchscreen users some time to read/interact with the carousel slide, before cycling is resumed. And it fixes the broken behavior for mouse users on touch-enabled devices too.
Closes #20847 (but the problem is not limited to Firefox, same happens on, say, Chrome on a touch-enabled device)