-
Notifications
You must be signed in to change notification settings - Fork 942
fix(Carousel): consistent stopOnInteraction behavior #5489
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
base: v4
Are you sure you want to change the base?
fix(Carousel): consistent stopOnInteraction behavior #5489
Conversation
| if (typeof props.autoplay === 'boolean' || props.autoplay.stopOnInteraction === undefined || props.autoplay.stopOnInteraction) { | ||
| emblaApi.value?.plugins().autoplay?.stop() | ||
| } | ||
| } | ||
| function scrollNext() { | ||
| emblaApi.value?.scrollNext() | ||
| if (typeof props.autoplay === 'boolean' || props.autoplay.stopOnInteraction === undefined || props.autoplay.stopOnInteraction) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (typeof props.autoplay === 'boolean' || props.autoplay.stopOnInteraction === undefined || props.autoplay.stopOnInteraction) { | |
| emblaApi.value?.plugins().autoplay?.stop() | |
| } | |
| } | |
| function scrollNext() { | |
| emblaApi.value?.scrollNext() | |
| if (typeof props.autoplay === 'boolean' || props.autoplay.stopOnInteraction === undefined || props.autoplay.stopOnInteraction) { | |
| if (props.autoplay && (typeof props.autoplay === 'boolean' || props.autoplay.stopOnInteraction !== false)) { | |
| emblaApi.value?.plugins().autoplay?.stop() | |
| } | |
| } | |
| function scrollNext() { | |
| emblaApi.value?.scrollNext() | |
| if (props.autoplay && (typeof props.autoplay === 'boolean' || props.autoplay.stopOnInteraction !== false)) { |
The condition to stop autoplay on arrow click is incorrect: it tries to stop autoplay even when autoplay is false (disabled), which is unnecessary and logically wrong.
View Details
Analysis
Incorrect autoplay stop condition allows stopping non-existent plugin
What fails: scrollPrev() and scrollNext() in Carousel.vue attempt to stop the autoplay plugin even when autoplay is disabled (default: autoplay: false), causing semantically incorrect logic.
How to reproduce:
- Create a carousel with default props (autoplay disabled)
- Click the previous/next arrow buttons
- The condition
typeof props.autoplay === 'boolean'evaluates totruewhenautoplay = false - Code attempts
emblaApi.value?.plugins().autoplay?.stop()on non-existent autoplay plugin
Result: While optional chaining (?.) prevents a runtime error, the code executes logically incorrect behavior: attempting to stop a plugin that was never initialized. According to embla-carousel-autoplay documentation, the plugin is only initialized when autoplay is truthy (if (props.autoplay)), but the stop condition incorrectly evaluates to true even when autoplay is false.
Expected: The condition should only attempt to stop autoplay when autoplay is actually enabled (truthy), ensuring semantic correctness.
Fix: Changed condition from:
if (typeof props.autoplay === 'boolean' || props.autoplay.stopOnInteraction === undefined || props.autoplay.stopOnInteraction)to:
if (props.autoplay && (typeof props.autoplay === 'boolean' || props.autoplay.stopOnInteraction !== false))This ensures:
- Only attempts to stop when autoplay is enabled (truthy check first)
- When enabled as boolean, always stops on interaction
- When enabled as object, respects the
stopOnInteractionoption (defaults to true per embla docs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@J-Michalek Seems relevant
commit: |
π Linked issue
Fixes #5488
β Type of change
π Description
There was inconsistency between swipe and click on arrow button while using the autoplay plugin - swipe used to stop the playing, but clicking the button did not. To me both seems like an interaction that should stop the playing.
π Checklist