Tlaster / PreCompose

Compose Multiplatform Navigation && State Management
https://tlaster.github.io/PreCompose/
MIT License
852 stars 49 forks source link

[BUG] Rapid back navigation breaks layout on Android #314

Open jirkli opened 6 months ago

jirkli commented 6 months ago

Rapidly clicking the back button breaks the application layout. Sometimes the nav transition animation doesn't finish and you end up with a previous screen still covering 2/3 of the screen. Sometimes the "real" active scene is hidden under the previous scene, so you can't see what you're interacting with, etc.

To Reproduce

  1. Click a few times through the application to build the back stack entry.
  2. Use the NavHost as you normally would, seems that transitions slideInHorizontally and slideOutHorizontally make it easier to reproduce, but they aren't necessary.
  3. Rapidly click the back button. Depending on the complexity of the rendered scene, doesn't even have to be that rapid.
  4. NavHost breaks, without any obvious way of mitigation. Manually navigating to a next scene doesn't solve this.

This is reproducible everywhere, in a virtual device, in a release build and on a real device. Tested on five different Android devices with varying Android versions and was easily able to reproduce this everywhere.

Expected behavior User is able to click back rapidly without breaking the application, even if it means that some of the back clicks are ignored.

What's interesting is that rapid navigation "forward" doesn't seem to have the same issue.

Edit If it helps your investigation, I've found out that once the navigation breaks, the backStacks on the BackStackManager get stuck and aren't changing on back button press anymore. So for example I start with 58 stacks, by pressing 10 times rapidly I break the navigation, the stack count drops to 48 and any other back button press doesn't seem to do anything and stays at 48. Also I'm testing this in your sample ToDo application, where I'm mocking the history by navigating to /edit/1 and home repeatedly to build the back stacks.

jirkli commented 6 months ago

Ah, I'm finally starting to understand a little bit what's going on here. In the NavHost composable there's a LaunchedEffect(progress) on line 190. During fast navigation this can (for one reason or another, probably just recomposition synchronization) get stuck at the same target value and the effect is no longer launched.

My trivial fix (without any further knowledge into the code and topic) would be to change the line into LaunchedEffect(progress, sceneEntry.stateId) {. According to my very limited testing (only in virtual device) this solves the issue entirely. I'm sure a proper fix will look somewhat differently though.

Tlaster commented 6 months ago

I am currently reworking this, which may also address issue #288, as both appear to stem from the same underlying cause. The current implementation is rather unrefined and problematic, especially in handling numerous edge cases.