Fixed mobile lightbox gallery on PDP#151
Conversation
* close button works * swipe gestures work
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR adds swipe gesture navigation to media browsing components. ChangesSwipe Navigation for Media Components
Sequence DiagramssequenceDiagram
participant User
participant MediaGallery
participant State
User->>MediaGallery: touchstart
MediaGallery->>State: recordStartCoordinates
User->>MediaGallery: touchend (horizontal swipe)
MediaGallery->>State: calculateDelta
State->>State: updateSelectedIndex
State->>State: setSuppressClick
MediaGallery->>User: navigate image (no lightbox)
sequenceDiagram
participant User
participant MediaLightbox
participant State
User->>MediaLightbox: touchstart on backdrop
MediaLightbox->>State: recordBackdropTarget
MediaLightbox->>State: recordTouchStart
User->>MediaLightbox: touchend (swipe detected)
State->>State: navigatePrevOrNext
MediaLightbox->>MediaLightbox: suppressSyntheticClick
User->>MediaLightbox: click on backdrop
MediaLightbox->>MediaLightbox: checkBackdropTarget
MediaLightbox->>User: close only if backdrop click was direct
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/products/MediaLightbox.tsx (1)
58-89: 💤 Low valueConsider extracting the swipe state machine into a shared hook.
This block is essentially identical to the one just added in
MediaGallery.tsx(sameSWIPE_THRESHOLD_PX/SWIPE_MAX_VERTICAL_PX, same start/end refs, same dx/dy filter). A smalluseSwipe({ onSwipeLeft, onSwipeRight })hook in@/hookswould centralize the thresholds and the touchstart/changedTouches edge cases (!touchguards, clearingtouchStartRefeven on invalid swipes), and it would keep both consumers in sync if you later tune thresholds or addonSwipeStart/onSwipeEndcallbacks (the latter is already needed here to clearpressedOnBackdropRef).Not blocking — the current implementation reads correctly.
🤖 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 `@src/components/products/MediaLightbox.tsx` around lines 58 - 89, Extract the duplicated swipe logic in handleTouchStart and handleTouchEnd (which use SWIPE_THRESHOLD_PX, SWIPE_MAX_VERTICAL_PX, touchStartRef, pressedOnBackdropRef and call goNext/goPrev) into a shared hook like useSwipe({ onSwipeLeft, onSwipeRight, onSwipeEnd? }) in `@/hooks`; move the touchstart/touchend guards (!touch, clearing touchStartRef on end regardless of validity), the dx/dy threshold checks, and the synthetic-backdrop suppression (clearing pressedOnBackdropRef) into the hook, return handlers (e.g., onTouchStart, onTouchEnd) for MediaLightbox and MediaGallery to call so both consumers use the same thresholds and behavior and can add optional onSwipeStart/onSwipeEnd callbacks later.
🤖 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.
Nitpick comments:
In `@src/components/products/MediaLightbox.tsx`:
- Around line 58-89: Extract the duplicated swipe logic in handleTouchStart and
handleTouchEnd (which use SWIPE_THRESHOLD_PX, SWIPE_MAX_VERTICAL_PX,
touchStartRef, pressedOnBackdropRef and call goNext/goPrev) into a shared hook
like useSwipe({ onSwipeLeft, onSwipeRight, onSwipeEnd? }) in `@/hooks`; move the
touchstart/touchend guards (!touch, clearing touchStartRef on end regardless of
validity), the dx/dy threshold checks, and the synthetic-backdrop suppression
(clearing pressedOnBackdropRef) into the hook, return handlers (e.g.,
onTouchStart, onTouchEnd) for MediaLightbox and MediaGallery to call so both
consumers use the same thresholds and behavior and can add optional
onSwipeStart/onSwipeEnd callbacks later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4f8f216-5c23-4784-9407-a27c34b27d74
📒 Files selected for processing (2)
src/components/products/MediaGallery.tsxsrc/components/products/MediaLightbox.tsx
1 participant
Summary by CodeRabbit