slackhq/circuit

Latest 0.22.1 `resetRoot(saveState = true, restoreState = true)` loses state when using `pop`

Opened this issue · 6 comments

TepesLucian commented

Updating to newest 0.22.1 resetRoot(saveState = true, restoreState = true) restores save states correctly but after reseting to a tab with a backstack size greater than 1 calling navigator.pop() loses state on previous screens vs going back using gesture navigation which restores everything correctly.

Steps to reproduce in chrisbanes/tivi:

  1. Go to settings in discover (profile picture -> settings)
  2. Scroll to Open source licenses
  3. Scroll a bit for testing
  4. Switch to other tab
  5. Switch back to discover (scroll state is restored correctly)
  6. Going back using gesture navigation restores settings scroll correctly vs tapping back icon (or 2/3 system button back) which calls pop loses scroll position
chrisbanes commented

I've just added a test for this in #1454 and everything seems to be working as expected. I can't reproduce this.

Are you seeing this on any particular device? Also, are you using rememberSaveable or retained?

TepesLucian commented

@chrisbanes just restested on api pre 34 and 34. Pre 34 everything looks okay but 34 seems to be the problem. Commenting out decoration param on NavigableCircuitContent seems to fix the issue on 34.

chrisbanes commented

I just locally converted the test to run on device instead, and seeing the same thing, everything is passing. Try 0.21.2 or 0.22.2, as there's some fixes in there for pausable state. They're not directly related to saveable, but it wouldn't surprise me if they fix some things.

TepesLucian commented

Unfortunately still seeing the issue. Here is a recording of the issue if that helps. Going back via gesture navigation works, but going back using the up arrow resets the scroll position.
Screen_recording_20240604_210457.webm

TepesLucian commented

A state in the ui like rememberSaveable { mutableStateOf(int) } restores fine from my testing but there's something up with rememberLazyListState on api 34. Swapping GestureNavigationDecoration to CupertinoGestureNavigationDecoration or commeting decoration all out fixes the scroll position issue.

chrisbanes commented

Ah so this is UI state, rather than something in the Presenter. I'll tweak the test and see what it does

  • 👍1