perf: don't animate setViewControllers on empty stack#5
Merged
Conversation
setRootViewControllerIfEmptyElsePush passed the caller's `animated` parameter through to `setViewControllers([vc], animated: animated)` even on the empty-stack branch — i.e. when seating the *first* VC into a fresh nav controller. UIKit still spins a ~0.35s push-style animation in that case, even though the user can't see it: the typical caller is `presentModalCoordinator` which is about to follow up with a separate `navigationController.present(modalNav, animated: true)` of its own. The user only ever sees the modal-present animation; the inner empty-to-first-VC animation is invisible-but-blocking, gating the visible animation chain on it finishing first. Net: ~1-2s perceived lag between "tap entry button" and "modal flow appears" on iOS 26 simulators (worse on cold launches where UIKit machinery for the new stack is also being inflated for the first time). Fix: force `animated: false` on the empty-stack branch. The visible modal-present animation now starts immediately. `forceReplaceAllVCs- InsteadOfPush` still honours the caller's `animated` because that path is for in-stack replacements where the user *is* watching the transition. Bookkeeping: track `didAnimate` explicitly so the completion-routing fall-through (transition-coordinator vs DispatchQueue.main.async) matches the actual transition we performed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5 +/- ##
=======================================
Coverage 87.77% 87.77%
=======================================
Files 30 30
Lines 1562 1562
=======================================
Hits 1371 1371
Misses 191 191 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce perceived latency when starting a flow on an empty UINavigationController, especially for modal coordinators, by avoiding UIKit’s hidden first-push animation and aligning completion handling with the transition that actually ran.
Changes:
- Force
setViewControllers([vc], animated: false)when seating the first view controller into an empty navigation stack. - Split the previous empty-stack/replace-all branch so
forceReplaceAllVCsInsteadOfPushcan still use the caller’sanimatedflag on non-empty stacks. - Track
didAnimateexplicitly so completion dispatch uses the transition coordinator only when an actual animated transition occurred.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
147
to
+149
| } else { | ||
| pushViewController(viewController, animated: animated) | ||
| didAnimate = animated |
Comment on lines
+133
to
+137
| if viewControllers.isEmpty, viewIfLoaded?.window == nil { | ||
| // Only suppress the initial empty-stack animation when this nav | ||
| // controller is still off-screen. Typical case: a brand-new | ||
| // modal nav controller that is about to be | ||
| // `present(animated: true)`-ed by the caller. In contrast, an |
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.
setRootViewControllerIfEmptyElsePush passed the caller's
animatedparameter through to
setViewControllers([vc], animated: animated)evenon the empty-stack branch — i.e. when seating the first VC into a
fresh nav controller. UIKit still spins a ~0.35s push-style animation
in that case, even though the user can't see it: the typical caller is
presentModalCoordinatorwhich is about to follow up with a separatenavigationController.present(modalNav, animated: true)of its own.The user only ever sees the modal-present animation; the inner
empty-to-first-VC animation is invisible-but-blocking, gating the
visible animation chain on it finishing first.
Net: ~1-2s perceived lag between "tap entry button" and "modal flow
appears" on iOS 26 simulators (worse on cold launches where UIKit
machinery for the new stack is also being inflated for the first time).
Fix: force
animated: falseon the empty-stack branch. The visiblemodal-present animation now starts immediately.
forceReplaceAllVCs- InsteadOfPushstill honours the caller'sanimatedbecause that pathis for in-stack replacements where the user is watching the
transition.
Bookkeeping: track
didAnimateexplicitly so the completion-routingfall-through (transition-coordinator vs DispatchQueue.main.async)
matches the actual transition we performed.
Co-Authored-By: Claude Opus 4.7 noreply@anthropic.com