fix(Carousel): skip scroll-sync during programmatic scrolls to fix Firefox interaction test failures - #3939
Draft
rzp-slash[bot] wants to merge 1 commit into
Draft
Conversation
![rzp-slash[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…refox interaction test failures
|
Contributor
|
🤖 Slash AI Review has been triggered. View execution logs |
rzp-slash
Bot
added
the
Auto-Created PR
Collaborator
🛡️ Coverage ReportSummaryFull Coverage Details |
kamaleshs-bridge4
left a comment
kamaleshs-bridge4
left a comment
There was a problem hiding this comment.
✨ Agentic PR Review ✨
UI Review
✅ 3 passed
Passing checks (3)
| Check | Screenshot |
|---|---|
| ✅ Carousel autofit with side navigation - boundary button visibility |  |
| ✅ Carousel autoplay - autoplay and navigation |  |
| ✅ Carousel side navigation - manual scroll-sync still works |  |
Usage
import { Carousel } from '@razorpay/blade/components';
<Carousel visibleItems="autofit" navigationButtonPosition="side">...</Carousel>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Problem
The Blade Interaction Tests CI workflow (run 33082693641) is failing on the master branch. Two Carousel interaction tests fail specifically on Firefox (chromium passes):
TestAutofit— expects the "Next Slide" button to be null after clicking the last indicator, but the button is still renderedTestAutoPlayPause— fails as a side effect of the TestAutofit failureRoot Cause
When an indicator button is clicked in a responsive (
visibleItems="autofit") carousel withnavigationButtonPosition="side"andshouldAddStartEndSpacing, the following sequence occurs:goToSlideIndex(6)setsactiveSlide = 6→shouldShowNextButton = false(button hidden)scrollToSlide(6)starts a smooth scroll and setsisProgrammaticScrollRef.current = truefor 600msdocument.elementFromPointat the carousel center to detect the current slide — but the scroll hasn't finished yet, so it detects an intermediate slide (e.g., slide 4 or 5)setActiveSlide(() => goTo, isProgrammaticScrollRef.current)— the second argument (true) only preventsonChangefrom firing, but the state is still updated to the intermediate valueactiveSlidegets overridden to the intermediate value →shouldShowNextButtonbecomestrueagain → the Next button reappearsqueryByRole('button', { name: 'Next Slide' })returns a button instead of nullFix
Add an early return at the top of the debounced scroll-sync handler to skip it entirely when
isProgrammaticScrollRef.currentistrue. This preserves theactiveSlidevalue set by the programmatic action (indicator click, next/prev button, autoplay) during the scroll animation.The programmatic actions (
goToSlideIndex,goToNextSlide,goToPreviousSlide) already set bothactiveSlideandactiveIndicatorto the correct values, so the scroll-sync handler is not needed during programmatic scrolls. After the 600ms programmatic-scroll window expires, the handler resumes normal operation for user-initiated manual scrolls.Also removed the now-unnecessary
isProgrammaticScrollRef.currentsecond argument fromsetActiveSlidesince the handler is skipped entirely during programmatic scrolls.Testing
yarn build(blade package) — passesyarn typecheck(blade package) — passesTestAutofitandTestAutoPlayPauseFirefox failuresAutomated changes by autonomous agent.