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

Fix saveable state being restored when using reset root navigation events #1354

Merged
merged 2 commits into from
May 27, 2024

Conversation

chrisbanes
Copy link
Contributor

@chrisbanes chrisbanes commented Apr 19, 2024

This PR migrates NavigableCircuitContent to use SaveableStateHolder, rather than our hand-rolled SaveableStateRegistryBackStackRecordLocalProvider.

I don't know the history behind SaveableStateRegistryBackStackRecordLocalProvider, but SaveableStateHolder is the first party solution for this. It is used by AndroidX Navigation, so we can assume it is well tested.

This PR relies on a bunch of movableContent fixes added in #1282 (I had to add a similar one in this PR for CupertinoGestureNavigationDecoration).

Fixes #1342

@chrisbanes chrisbanes force-pushed the cb/saveable-saved-backstack branch from 41c769d to b185cc5 Compare May 27, 2024 09:28
@chrisbanes chrisbanes changed the title WIP: Add test for asserting saveable state + reset roots Fix saveable state being restored when using reset root navigation events May 27, 2024
@chrisbanes chrisbanes marked this pull request as ready for review May 27, 2024 09:32
Copy link
Collaborator

@ZacSweers ZacSweers left a comment

Choose a reason for hiding this comment

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

Lgtm. This is from the original Back Stack impl we ported from @adamp but maybe predates the newer API.

@ZacSweers ZacSweers added this pull request to the merge queue May 27, 2024
Merged via the queue into slackhq:main with commit ae42097 May 27, 2024
5 checks passed
@ZacSweers ZacSweers deleted the cb/saveable-saved-backstack branch May 27, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

resetRoot(saveState = true, restoreState = true) loses saveable state
2 participants