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

Recreate nested nav to work with AnimatedPane #1380

Merged
merged 3 commits into from May 6, 2024

Conversation

alexvanyo
Copy link
Contributor

Adds AnimatedPane to ListDetailPaneScaffold for interests by introducing a workaround for the nested navigation host setup.

Previously, adding AnimatedPane would crash when just the list was showing, since we'd try to call navigate on the nested nav controller before the graph for the nested nav controller was setup.

The workaround in this PR is to only call navigate on the nested nav controller in the situation where both the list and detail are showing.

If just the list is showing, then instead of calling navigate on the nested nav controller, we completely tear-down and recreate the nested nav controller and the nested NavHost via a key on a unique id. Then, the start destination of the nested nav host becomes where we were trying to navigate to.

Since we were popping the nested nav controller anyway, we don't mind that we completely wipe the state of the nested nav host in this situation.

@alexvanyo alexvanyo requested a review from jdkoren April 16, 2024 00:11
Copy link

Combined test coverage report

Overall Project 40.33% -0.07% 🍏
Files changed 87.66% 🍏

Module Coverage
topic 46.54% -0.51%
app 30.39% -0.24% 🍏
Files
Module File Coverage
topic TopicNavigation.kt 20.37% -9.88%
app InterestsListDetailScreen.kt 88% -4% 🍏

@alexvanyo alexvanyo force-pushed the av/nested-nav-with-animated-pane branch from 6fce017 to 8b9bc76 Compare April 16, 2024 00:32
Copy link

Combined test coverage report

Overall Project 40.31% -0.07% 🍏
Files changed 87.66% 🍏

Module Coverage
topic 46.54% -0.51%
app 30.39% -0.24% 🍏
Files
Module File Coverage
topic TopicNavigation.kt 20.37% -9.88%
app InterestsListDetailScreen.kt 88% -4% 🍏

Copy link

Combined test coverage report

Overall Project 40.34% -0.07% 🍏
Files changed 88.05% 🍏

Module Coverage
topic 46.54% -0.51%
app 30.42% -0.24% 🍏
Files
Module File Coverage
topic TopicNavigation.kt 20.37% -9.88%
app Interests2PaneViewModel.kt 100% 🍏
InterestsListDetailScreen.kt 88.04% -3.99% 🍏

@alexvanyo alexvanyo force-pushed the av/nested-nav-with-animated-pane branch from e1b4679 to 0b1e65c Compare April 17, 2024 01:15
Copy link

Combined test coverage report

Overall Project 40.31% -0.07% 🍏
Files changed 88.05% 🍏

Module Coverage
topic 46.54% -0.51%
app 30.42% -0.24% 🍏
Files
Module File Coverage
topic TopicNavigation.kt 20.37% -9.88%
app Interests2PaneViewModel.kt 100% 🍏
InterestsListDetailScreen.kt 88.04% -3.99% 🍏

alexvanyo and others added 3 commits May 3, 2024 14:57
Change-Id: I6b526331b7fc62b968ac39e91753a8a1e5343023
Change-Id: If1155bfbe080eb4df3c59faaec0fb4cd4da3821d
@alexvanyo alexvanyo force-pushed the av/nested-nav-with-animated-pane branch from 0b1e65c to c2fc34c Compare May 3, 2024 21:57
@alexvanyo alexvanyo marked this pull request as ready for review May 3, 2024 22:02
Copy link

github-actions bot commented May 3, 2024

Combined test coverage report

Overall Project 40.54% -0.07% 🍏
Files changed 90.26% 🍏

Module Coverage
topic 47.09% -0.46%
app 32.72% -0.23% 🍏
Files
Module File Coverage
topic TopicNavigation.kt 20.37% -9.88%
app Interests2PaneViewModel.kt 100% 🍏
InterestsListDetailScreen.kt 89.26% -3.41% 🍏

@alexvanyo alexvanyo merged commit 3dd4240 into main May 6, 2024
4 checks passed
@alexvanyo alexvanyo deleted the av/nested-nav-with-animated-pane branch May 6, 2024 16:47
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.

None yet

2 participants