Skip to content
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

Support eager navigation destination resolution #241

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stephencelis
Copy link
Member

Currently we steer folks to using viewDidLoad for all observation, but this laziness can lead to issues with deep-linking in type-erased navigation stacks, where a destination may declare a sub-destination in its viewDidLoad, causing the first drill-down to occur lazily over multiple animated steps instead of a single one. This behavior can be seen in the type-erased navigation case study in the repository.

This PR is a proof of concept to show that the library can populate a "current navigation stack" in the UI transaction so that a destination's initializer can declare its dependent destinations, fixing the case study behavior.

An idea brought up by @iampatbrown.

Currently we steer folks to using `viewDidLoad` for all observation, but
this laziness can lead to issues with deep-linking in type-erased
navigation stacks, where a destination may declare a sub-destination in
its `viewDidLoad`, causing the first drill-down to occur lazily over
multiple animated steps instead of a single one. This behavior can be
seen in the type-erased navigation case study in the repository.

This PR is a proof of concept to show that the library can populate a
"current navigation stack" in the UI transaction so that a destination's
initializer can declare its dependent destinations, fixing the case
study behavior.
Comment on lines +341 to +343
withUITransaction(\.stackController, stackController) {
stackController.path.append(.lazy(.element(value)))
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes things just for the case study, but because deep linking can happen directly from state we would need to explore a more universal solution to ship this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant