search

Zhuinden / simple-stack

[ACTIVE] Simple Stack, a backstack library / navigation framework for simpler navigation and state management (for fragments, views, or whatevers).
Apache License 2.0
1.36k stars 76 forks source link

`persistViewToState` should initialize the SavedState via `getSavedState` #217

Closed Zhuinden closed 4 years ago

Zhuinden commented 4 years ago

https://github.com/Zhuinden/simple-stack/blob/7d51efbf2fe393651b2bb4a70f5afa2671c64e1c/simple-stack/src/main/java/com/zhuinden/simplestack/Backstack.java#L618-L623

My comment from almost a year ago says "oh I should fix this later"

https://github.com/Zhuinden/simple-stack/issues/153#issuecomment-473483498

Fixing things typically involves remembering them, so now there is an issue for it.

If one were to use SavedState for result passing (see https://github.com/Zhuinden/simple-stack/issues/216), this could result in a tricky API:

     val savedState = backstack.getSavedState(previousKey)
     Navigator.persistViewToState(previousView)
     backstack.getSavedState(previousKey).setBundle(savedState.getBundle())

Terrible and unexpected, though nobody has ever asked about it (probably because more people use fragments than the view-helper APIs, AND nobody ever thought about using getSavedState for this sort of thing) so I can treat it as a regular bug fix rather than behavior change. 🤔

Zhuinden commented 4 years ago

Also remove this comment:

https://github.com/Zhuinden/simple-stack/blob/7d51efbf2fe393651b2bb4a70f5afa2671c64e1c/simple-stack/src/main/java/com/zhuinden/simplestack/SavedState.java#L27

Zhuinden commented 4 years ago

Fixed in 2.2.5 https://github.com/Zhuinden/simple-stack/commit/8e2a9cc519f12ca0c4a996ee4fe1a5f8b0ef08a9