Always refresh BackSwipeGesture context after a navigation change#111
Merged
Conversation
Closes a remaining class of stale-cache bugs in the back-swipe gesture that survived the prior Y-1316 / Y-1342 / Y-1345 fixes. Each of those attacked one path; this folds the remaining ones together. - `_transitionDidEnd` no longer early-returns at `stackDepth <= 1`. The prior guard skipped `setupContext()` after a programmatic pop-to-root and after a gesture pop whose overlay clone was null, leaving the gesture's `canNavigateBack` stale at `true`. At root, `setupContext` correctly sets `canNavigateBack` and the recognizer's `enable` to false; at depth 0 the function's own null-item guard makes it a safe no-op. - `_handlePanEvent` now calls `setupContext()` on `ev.isFirst`. The gesture's cached geometry (`startingX`, `backX`, `thresholdX`, `canNavigateBack`) is normally kept fresh by `setupContext` calls from `_transitionDidEnd` and `_finishGestureBack`, but the latter is deferred via `next()`. A user can lift after a successful back-swipe and start a new pan before that microtask flushes, so refreshing on `isFirst` guarantees the new pan sees the current container position and depth (rapid double-back-swipe race). - `_finishGestureBack` now re-anchors the item container to the new depth's `_computeXPosition()` after the opacity restore. The overlay-consumed path runs no programmatic transition, so the spring's final `translateX` is still latched at the pre-pop `backX`. In a rapid double-back-swipe where the second spring's `backX` was computed from stale geometry, the container ends up at a value that is invalid for the new depth and strands the viewport blank. Re- anchoring after the re-render lands corrects this. - `SWIPE_SPRING_CONFIG` and `_createSpring` both get `overshootClamping: true`. Without it, the spring rests ~0.3px past `toValue` for a tick before settling, painting a 1-retina-pixel sliver of whatever's behind the NavStack on the left edge as the slide-back lands. No motion-design reason to keep the overshoot. Adds two regression assertions in the existing "drilled down two levels" integration module: after a back-swipe (single or double-pop), the item container's transform exactly matches the new depth's resting position. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a set of iOS back-swipe gesture bugs caused by stale cached gesture context (e.g., startingX, canNavigateBack, recognizer enablement) not being refreshed on all navigation paths. It ensures the back-swipe gesture state and the item-container transform are consistently re-synchronized after stack depth changes and at the beginning of a new pan.
Changes:
- Always call
BackSwipeGesture#setupContext()after transitions end (including pop-to-root) to prevent stalecanNavigateBack/ recognizer state. - Refresh gesture context on
panstart (ev.isFirst) to close a race where a second swipe can begin before deferred cleanup runs. - Re-anchor the item container to
_computeXPosition()after a gesture-driven pop and clamp spring overshoot to avoid off-screen stranding and 1px edge flashes; adds integration test coverage for the resting-position behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test-app/tests/integration/nav-stack-test.js | Adds assertions/tests that the item container ends at the canonical per-depth resting position after gesture pops. |
| ember-nav-stack/src/components/nav-stack.js | Always refreshes gesture context on transition end; re-anchors container after gesture pops; clamps spring overshoot in programmatic transitions. |
| ember-nav-stack/src/back-swipe-gesture.js | Clamps spring overshoot for swipe springs; refreshes gesture context at the start of each pan to prevent stale geometry/race issues. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per PR #111 review (thanks @copilot-pull-request-reviewer): the `currentStackItemElement === null` branch in `setupContext()` was resetting `canNavigateBack` but leaving the Hammer pan recognizer's `enable` at whatever the previous depth had set. With the new unconditional `setupContext()` call on every transition (the main change in this PR), the early-return branch is now reachable during real navigation events — most plausibly during a layer-dismissal slide-down where the items drain to 0 but the NavStack is still mounted. Force `enable: false` in that branch so a stale `true` from a prior depth can't fire pan events. Also updated the misleading "safe no-op" comment in `_transitionDidEnd` to reflect that the depth-0 branch is defensive, not strictly inert. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Background and Goal
A user-visible bug remained on TestFlight after the 7.1.4 / 7.1.5 / 7.1.6 fixes (Y-1316 / Y-1342 / Y-1345): at root, a left-edge back-swipe still translated the item container all the way to
backXand firedonBack(), leaving the viewport blank. A rapid double-back-swipe from depth 3 also stranded the viewport blank. And after every successful back-swipe, a 1-retina-pixel sliver flashed on the left edge as the spring landed.Tracing the symptoms with a Web Inspector stack-trace probe against a local Debug build confirmed the pan recognizer was firing with
startingX = 0ANDcanNavigateBack = trueat root — both wrong for depth 1, both owned bysetupContext(), which wasn't running on every navigation path that needed it.Where to start
Read in this order:
_transitionDidEndinember-nav-stack/src/components/nav-stack.js— the priorstackDepth <= 1guard skippedsetupContext()after pop-to-root and after a non-overlay gesture pop, leavingcanNavigateBackstale attrue._handlePanEventinember-nav-stack/src/back-swipe-gesture.js— thesetupContext()onev.isFirstcloses the rapid-double-swipe race (where_finishGestureBack's deferrednext()hasn't flushed by the time the next touch lands)._finishGestureBackinember-nav-stack/src/components/nav-stack.js— re-anchors the container to_computeXPosition()after the opacity restore, so a springtoValuederived from stale geometry doesn't strand the container off-screen at the new depth.SWIPE_SPRING_CONFIG+_createSpring—overshootClamping: trueon both. Kills the 1px white-line flash.Key decisions and non-obvious mechanics
setupContext()doesn't run on every navigation path. The fold-together fix is making suresetupContext()always runs whenever depth changes, and as a belt-and-suspenders, also when a new pan begins._finishGestureBackis the saving throw for the rapid-double-swipe scenario verified on-device. The gesture's ownsetupContext()in_finishGestureBackalready keeps the next swipe correct, but if the user doesn't swipe again, the container's translateX (latched at the pre-popbackX) may not match the new depth's resting position. Re-anchoring corrects this without depending on a follow-up gesture.