Fix(Swiper): ensure initial slide is displayed

[Isaacmaamouche]

Bug

Swiper's initialIndex option was ignored whenever the swiper mounted into a subtree that was still painting (happened to me in a Next.js app) because of internal mechanism, it would always show the first slide.

Timeline:

1. The init effect ran too early: On the first render, during the same frame the container first paints, the react effect issued a programmatic scroll to the target slide.
2. CSS scroll-snap undid it: The slide track css has scroll-snap-type: x mandatory. A scroll issued mid-first-layout isn't a settled snap point yet, so the browser's snap correction yanked it back to offset 0px.
3. That revert became the internal truth for the swiper's state: The scroll-back-to-0 fired an onScroll, the debounced handler read scrollLeft (now 0px) to derive which slide is visible, computed it as the current slide and set state currentPage to 0. A one-time init flag then blocked any retry, so the wrong position was permanent.

A Vite SPA escaped it only by luck: its client-only mount left layout already settled before the react effect ran, so the initial scroll landed on a valid snap point and stuck.

Two latent bugs sat in the same path:

• the default initialIndex of 0 computed to slide -1 (it's a 1-based value)
• undefined initialIndex produced NaN, both were used into the scroll math

Fix (thank you Claude)

Defer the initial scroll to after the first paint:

• It's scheduled across two animation frames instead of running during the commit, so it lands once layout is settled, and snap-back no longer fires.
• Two frames are required because a frame scheduled from an effect still runs before the next paint.
• The second frame also gives the viewport-size hook time to resolve, so the target page is correct even when slides-per-view varies by breakpoint.

Ignore the swiper's own scroll once:

• A one-shot guard makes the scroll handler skip exactly the settle that follows the initial programmatic scroll, so a scroll correction by the css can't be computed as the current page.
• It's aimed only at the initial scroll, and only when the target isn't the first page, so ordinary arrow/keyboard/swipe navigation, which relies on that same "scroll -> state" feedback, is untouched.

Sync state directly at init:

• Since the one-shot guard suppresses the usual feedback that does it, and we skip the redundant follow-up scroll that the state change would otherwise trigger.
• Clamp the target page into valid range, which fixes the -1 default and the NaN case and makes an out-of-range initialIndex land on the last page instead of scrolling past the end.
• Also cancel the pending frame on unmount (clean StrictMode re-mount) and cancel one pending scroll-handler call on unmount.

none of the unit tests reproduce the bug

jsdom has no layout, paint, or scroll-snap. Tests are here to lock the fix's mechanism against future refactors (that the scroll is deferred, and that the guard is one-shot rather than permanent).

Bonus fixes

Two doc examples (asset-swiper, asset-swiper-title) were also fixed: their assets weren't wrapped in Swiper.Slides, so the swiper never registered any slides and initialIndex did nothing.

Screen captures from marketplace-front X Next.js

Before

brand-profile-swiper-bug.mp4

After

brand-profile-swiper-bug-fix.mp4

Summary by CodeRabbit

• Bug Fixes

  • Improved Swiper’s initial positioning across breakpoints and multi-page layouts.
  • Prevented redundant scrolling and scroll-snap corrections during initialization.
  • Preserved reliable navigation after the initial position is set.
  • Improved handling when the requested starting slide exceeds the available slides.

• Examples

  • Updated modal swiper examples to use the supported slide container structure.

• Tests

  • Added coverage for initial positioning, deferred scrolling, boundary handling, and subsequent user navigation.

[coderabbitai]

📝 Walkthrough

Walkthrough

Swiper initial navigation now waits for committed layout across two animation frames, clamps the initial page, and avoids redundant synchronization. Scroll handling and lifecycle cleanup were tightened, tests cover the behavior, and modal examples now use Swiper.Slides.

Changes

Swiper initialization and modal updates

Layer / File(s)	Summary
Deferred initial navigation lib/src/components/Swiper/index.tsx, lib/src/components/Swiper/utils.ts	Swiper computes a clamped initial page, defers navigation across two animation frames, supports nullable slide refs, and safely handles nullable interval callbacks.
Scroll synchronization and validation lib/src/components/Swiper/index.tsx, lib/src/components/Swiper/index.test.tsx	Scroll state refresh, debounced callback cleanup, redundant-page suppression, and initialIndex behavior tests were added or updated.
Modal swiper slide structure lib/src/components/Modal/docs/examples/asset-swiper.tsx, lib/src/components/Modal/docs/examples/asset-swiper-title.tsx	Media entries are nested inside Swiper.Slides containers while retaining their existing sources and attributes.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Swiper
  participant Browser
  participant User
  Swiper->>Browser: Defer initial page scroll
  Browser->>Swiper: Complete guarded initialization
  User->>Swiper: Trigger scroll event
  Swiper->>Swiper: Refresh navigation state
  Swiper->>Browser: Schedule debounced scroll handling

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: fixing Swiper's initial slide behavior.
Description check	✅ Passed	The description covers the bug, fix rationale, test notes, and screenshots; it mostly satisfies the template despite missing the exact checklist sections.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-swiper-initial-slide

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

👀 Visit Preview

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

lib/src/components/Swiper/index.test.tsx (1)

178-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Rename the use* helpers — they aren't hooks.

useSyncAnimationFrames / useCapturedAnimationFrames are plain setup helpers called inside it() callbacks; the use prefix misleads readers and can trip react-hooks/rules-of-hooks if that rule is enabled for test files. mockSyncAnimationFrames / captureAnimationFrames read better.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/src/components/Swiper/index.test.tsx` around lines 178 - 215, Rename the
plain test setup helpers useSyncAnimationFrames and useCapturedAnimationFrames
to mockSyncAnimationFrames and captureAnimationFrames, respectively, and update
every invocation in the related tests. Preserve their existing animation-frame
mocking behavior.

lib/src/components/Swiper/index.tsx (1)

237-240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a stable debounced handler instead of a mutable ref.

Two smells here: handleScrollRef.current = handleScroll mutates a ref during render (not concurrent-render safe), and the previous debounced instance is never cancelled when handleScroll is re-memoized on every currentPage change — a pending call from the old instance can still fire with a stale updatePage closure. A single debounce created once, calling through refs to the latest getNavigationState/updatePage, avoids both.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/src/components/Swiper/index.tsx` around lines 237 - 240, Replace the
mutable handleScrollRef pattern with one stable debounced handler created once,
while routing its invocation through refs that are updated to the latest
getNavigationState and updatePage callbacks. Ensure the previous debounced
instance is cancelled when dependencies are re-memoized, preventing stale
pending calls while preserving current-page behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/src/components/Swiper/index.test.tsx`:
- Around line 295-302: Restore the original HTMLElement.prototype offsetWidth
and scrollWidth descriptors after the test that patches them, using test-scoped
setup/cleanup or a finally-style teardown so later tests observe the pre-test
geometry. Keep the mocked 896 and 2688 values available for the intended test
while ensuring cleanup runs even when assertions fail.

In `@lib/src/components/Swiper/index.tsx`:
- Around line 336-353: In the deferred initialization callback around goTo and
setCurrentPage, arm skipSnapBackRef only when goTo will produce an actual
scroll, and arm skipNextPageScrollRef only when setCurrentPage(initialPage) will
change the current page. Avoid setting either one unconditionally, preserving
initialization behavior while preventing unused one-shot guards from affecting
later user or external navigation.

---

Nitpick comments:
In `@lib/src/components/Swiper/index.test.tsx`:
- Around line 178-215: Rename the plain test setup helpers
useSyncAnimationFrames and useCapturedAnimationFrames to mockSyncAnimationFrames
and captureAnimationFrames, respectively, and update every invocation in the
related tests. Preserve their existing animation-frame mocking behavior.

In `@lib/src/components/Swiper/index.tsx`:
- Around line 237-240: Replace the mutable handleScrollRef pattern with one
stable debounced handler created once, while routing its invocation through refs
that are updated to the latest getNavigationState and updatePage callbacks.
Ensure the previous debounced instance is cancelled when dependencies are
re-memoized, preventing stale pending calls while preserving current-page
behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e1cffa4e-c1ea-4a56-8dbb-9aa7b9aa9ce7

📥 Commits

Reviewing files that changed from the base of the PR and between faa2dd1 and 67a2f92.

📒 Files selected for processing (5)

• lib/src/components/Modal/docs/examples/asset-swiper-title.tsx
• lib/src/components/Modal/docs/examples/asset-swiper.tsx
• lib/src/components/Swiper/index.test.tsx
• lib/src/components/Swiper/index.tsx
• lib/src/components/Swiper/utils.ts

[guillaumewttj]

I'm ok with the behavior / functionnal part of the code ... but i think that we're at a poing where the amount of comments we have in there is a strong signal to break this component into functions / hooks.
Maybe using the principle of VM + breaking each action / reaction into a named function would help reading this component more easily ?

[linear]

ROI-1895

[Isaacmaamouche]

I'm ok with the behavior / functionnal part of the code ... but i think that we're at a poing where the amount of comments we have in there is a strong signal to break this component into functions / hooks. Maybe using the principle of VM + breaking each action / reaction into a named function would help reading this component more easily ?

@guillaumewttj PR ready for review: #3342