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

dispose steps after transition #427

Closed

Conversation

shpasha
Copy link
Contributor

@shpasha shpasha commented May 21, 2024

ISSUE SOLVED: 106

There is a problem. Let's take a look at the sample code.

If we set disposeStepsBehavior = DisposeStepsBehavior.DisposeWhenStackChanged in my sample, then when navigating to the SampleScreen#1 and going back, ViewModel on the SampleScreen#1 will be recreated because of transition and work will go on in it.

Also there is another problem. Suppose we use screenModel instead of viewModel. If we navigate back from the SampleScreen#1 and, without waiting for the transition to complete, navigate back to the SampleScreen#1, screenModel will be recreated, I expect the screenModel to be destroyed only after the screen has become invisible to the user and disappeared from the stack.

upd.

So, i've created 2 sample activities: DisposeWhenStackChangedSampleActivity, DisposeWhenTransitionFinishedSampleActivity.

  • DisposeWhenStackChangedSampleActivity. Just open it, then click Next, on the next screen click Back. You can see in logcat, that Doing job is still printing.
  • DisposeWhenTransitionFinishedSampleActivity. Same steps, but everything works correctly

@shpasha shpasha force-pushed the fix/dispose-after-transition branch 12 times, most recently from 4c0551f to 5878254 Compare May 21, 2024 15:18
@shpasha
Copy link
Contributor Author

shpasha commented May 21, 2024

Also we can get rid of StepDisposableEffect. And emit transition finished events also in CurrentScreen method:

@Composable
public fun CurrentScreen() {
    val navigator = LocalNavigator.currentOrThrow
    val currentScreen = navigator.lastItem
    val transitionFinishedEvents = LocalTransitionFinishedEvents.current

    LaunchedEffect(currentScreen) { transitionFinishedEvents.trySend(Unit) }
    navigator.saveableState("currentScreen") {
        currentScreen.Content()
    }
}

@shpasha shpasha force-pushed the fix/dispose-after-transition branch from 5878254 to a3fe7a7 Compare May 21, 2024 15:28
@shpasha shpasha force-pushed the fix/dispose-after-transition branch from a3fe7a7 to 2cbc09d Compare June 3, 2024 17:22
@DevSrSouza
Copy link
Collaborator

I have built another solution that is easier and does not change the Voyager core api and let the Transition api do all the work.

#436

DevSrSouza added a commit that referenced this pull request Jun 4, 2024
* Dispose Screens only when transition ends

* Adding the new parameter to all transitions apis

* fix lint

* Disposing solution from #427 to fix animation not ended dispose bug

* fix doc workflow

* fix lint

* update docs

* update api dump

* fix lint

* addressing reviews
@shpasha shpasha closed this Jun 5, 2024
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.

2 participants