-
Notifications
You must be signed in to change notification settings - Fork 111
fix: prevent swipe back gesture during push/pop transitions #640
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
Conversation
🦋 Changeset detectedLatest commit: 8d38200 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploying stackflow-demo with
|
| Latest commit: |
8d38200
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://228cb229.stackflow-demo.pages.dev |
| Branch Preview URL: | https://claude-issue-609-fix-011cumz.stackflow-demo.pages.dev |
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces previous non-capture touchstart/touchend listeners with capture-phase listeners for Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Touch
participant Doc as document (capture)
participant Hook as usePreventTouchDuringTransition
participant App as App / Children
Note over User,Doc: Any touch event during transition (start/move/end/cancel)
User->>Doc: touchstart / touchmove / touchend / touchcancel (capture)
Doc->>Hook: preventTouch invoked
alt globalTransitionState active
Hook->>Hook: e.stopPropagation()
Hook-->>Doc: interception (propagation stopped)
else not active
Hook-->>App: allow event to continue
end
Note over App: Children receive events only when not intercepted
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
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 |
commit: |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | 8d38200 | Commit Preview URL | Oct 22 2025, 06:39 AM |
This fix resolves a race condition where swiping back during a push
transition caused the app to freeze. The issue was particularly severe
when using historySyncPlugin, causing complete UI lockup.
Root Cause:
The usePreventTouchDuringTransition hook was using bubble phase event
listeners on appScreenRef, which executed AFTER the edge element's
swipe gesture listeners. This meant the edge listener's touch handlers
ran before the parent could prevent them, allowing the swipe gesture
to start during transitions.
Solution:
Modified usePreventTouchDuringTransition to use capture phase listeners:
- Added { capture: true } option to addEventListener
- Added e.stopPropagation() to prevent events from reaching child elements
- This ensures the parent's prevention logic runs BEFORE edge's swipe listeners
Event Flow (Before):
1. User touches edge element
2. Edge's touchstart listener executes (swipe starts) ❌
3. Event bubbles to appScreenRef
4. Parent's preventDefault() executes (too late!)
Event Flow (After):
1. User touches edge element
2. Parent's capture phase listener executes first
3. stopPropagation() + preventDefault() blocks further propagation ✅
4. Edge's touchstart listener never executes
Changes:
- File: extensions/react-ui-core/src/usePreventTouchDuringTransition.ts
- Lines: 23, 28, 33-34, 37-38
- Added stopPropagation() calls
- Changed to capture phase with { capture: true } option
This fix ensures that when globalTransitionState is not "idle", ALL
touch events (including edge swipe gestures) are properly prevented
across the entire activity, preventing the race condition.
Fixes #609
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
fa211a0 to
e11c802
Compare
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
extensions/react-ui-core/src/usePreventTouchDuringTransition.ts (1)
4-41: Clarify PR description inaccuracy: transition state check not added to useStyleEffectSwipeBack.The PR description states "Added transition state check in useStyleEffectSwipeBack's onTouchStart," but
useStyleEffectSwipeBack.tscontains no such check. TheonTouchStartfunction (line ~231) only handles blur, state caching, and callback invocation—no transition state validation.The actual changes are in
usePreventTouchDuringTransition.ts:
- Added
stopPropagation()in touch event handlers- Switched to capture-phase listeners (
{ capture: true })- Transition state check was already present (line 17:
stack.globalTransitionState === "idle")These are separate concerns:
usePreventTouchDuringTransitionblocks touches during transitions;useStyleEffectSwipeBackdetects swipe gestures independently. Update the PR description to accurately reflect the changes made.
🧹 Nitpick comments (1)
extensions/react-ui-core/src/usePreventTouchDuringTransition.ts (1)
33-34: Consider adding touchmove to event handling for edge case coverage.The implementation currently prevents
touchstartandtouchend, which effectively blocks most new gestures from initiating during transitions. However, a theoretical edge case exists: if a user begins a touch gesture while idle and a transition starts during that gesture,touchmoveevents would continue firing and potentially interfere with transition state.This scenario has low probability since transitions typically originate from discrete user actions rather than during ongoing gestures. The current implementation is sound for the primary use case. However, for defensive robustness, consider adding a
touchmovehandler:$ref.addEventListener("touchmove", onTouchMove, { capture: true });with a corresponding handler that prevents default and stops propagation, similar to
onTouchStartandonTouchEnd.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
extensions/react-ui-core/src/usePreventTouchDuringTransition.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write source in TypeScript with strict typing enabled across the codebase
Files:
extensions/react-ui-core/src/usePreventTouchDuringTransition.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: stackflow-docs
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
extensions/react-ui-core/src/usePreventTouchDuringTransition.ts (3)
21-29: LGTM: stopPropagation() addition strengthens event isolation.Adding
stopPropagation()to both touch handlers prevents events from bubbling to parent elements, complementing the capture-phase strategy to fully isolate touch events during transitions.
31-34: LGTM: Capture phase correctly prevents edge swipe conflicts.The capture-phase listeners intercept touch events before they reach child elements (like the edge swipe area), directly addressing the race condition described in issue #609 where swipe-back gestures could start during transition animations.
36-39: LGTM: Cleanup correctly matches addEventListener options.The
removeEventListenercalls properly include{ capture: true }to match theaddEventListeneroptions, ensuring listeners are correctly removed and preventing memory leaks.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/fix-swipe-back-transition.md (1)
5-5: Use hyphen for the compound modifier.The description contains a minor grammar issue: "capture phase event listeners" should use a hyphen to join the compound modifier.
Apply this diff to fix the grammar:
-Fix swipe back gesture during push/pop transitions by using capture phase event listeners to prevent touch events from reaching child elements during transitions +Fix swipe back gesture during push/pop transitions by using capture-phase event listeners to prevent touch events from reaching child elements during transitions
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.changeset/fix-swipe-back-transition.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Include a Changeset entry for any user-facing package change
Files:
.changeset/fix-swipe-back-transition.md
🪛 LanguageTool
.changeset/fix-swipe-back-transition.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...ng push/pop transitions by using capture phase event listeners to prevent touch e...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: stackflow-docs
- GitHub Check: Cloudflare Pages
Add touchmove and touchcancel event prevention to ensure complete touch gesture blocking during transitions. This prevents edge cases where: 1. Touch starts before transition (touchstart passes) 2. Transition begins mid-gesture 3. touchmove continues the unwanted gesture Also refactored to use a single preventTouch handler for cleaner code.
This fix resolves a race condition where swiping back during a push transition caused the app to freeze. The issue was particularly severe when using historySyncPlugin, causing complete UI lockup.
The root cause was that the swipe gesture listener on the edge element did not check if the activity was in an active transition state before starting the swipe. This allowed users to initiate a swipe gesture while a new activity was sliding in (enter-active) or sliding out (exit-active), creating conflicting animations.
Changes:
Fixes #609
🤖 Generated with Claude Code