Open projectdelta6 opened 4 months ago
I will have to take a look on this. We have a popUtil
function, we could have a replaceUntil
as well, this would combine multiples pops and the push at the same time.
The way that Voyager disposes the Screens and ViewModel/ScreenModel is the next tick of the composition when the event is a dispose event, when you do pop
+ push
at the same time, the last event will be a push
, so voyager can't dispose the screen that you have done pop. This is the nature of the library being built fully with Compose.
We have introduce a new way of handling disposing for Transition API that is still experimental, it does not relay on the lastEvent
, I have not tested, but maybe it could work with pop
+ push
, can you have a look?
https://github.com/adrielcafe/voyager/pull/436
The docs are here: https://voyager.adriel.cafe/transitions-api/
Is the disposeScreenAfterTransitionEnd = true
We are considering replace the StepDisposable with this approach if is stable enough.
I tried upgrading to 1.1.0-beta02
and using disposeScreenAfterTransitionEnd = true
as you suggested but I am now getting the Screen was used multiple times
crash mentioned in the docs when I use either pop
+ push
or just replace
when navigating from ScreenB
to ScreenC
I have tried setting a ScreenKey
on the screen using the class name + data ID but I still get the crash... the error is Key ScreenC:{ID}:transition was used multiple times
Try to use uniqueScreenKey instead
ah I didn't realise uniqueScreenKey
was an actual val...
So that stops the crash, but now it causes ScreenC's ScreenModel to be re-created in an infinite loop for some reason, I'm using replace
to go from ScreenB
to ScreenC
and I get the same using pop
+ push
ah I didn't realise
uniqueScreenKey
was an actual val... So that stops the crash, but now it causes ScreenC's ScreenModel to be re-created in an infinite loop for some reason, I'm usingreplace
to go fromScreenB
toScreenC
and I get the same usingpop
+push
I realised this issue was because I had the key as override val key: ScreenKey get() = uniqueScreenKey
, so I changed it to override val key: ScreenKey = uniqueScreenKey
and now using pop
+ push
it appears to work with ScreenB's ScreenModel being disposed, though I do also see the log saying that ScreenC's ScreenModel was also disposed but ScreenC appears for now to be working. However if I use replace
instead I do still get the Screen was used multiple times
crash.
Can someone write the steps to solve this
The way that Voyager disposes the Screens and ViewModel/ScreenModel is the next tick of the composition when the event is a dispose event, when you do
pop
+push
at the same time, the last event will be apush
, so voyager can't dispose the screen that you have done pop. This is the nature of the library being built fully with Compose.We have introduce a new way of handling disposing for Transition API that is still experimental, it does not relay on the
lastEvent
, I have not tested, but maybe it could work withpop
+push
, can you have a look? #436The docs are here: https://voyager.adriel.cafe/transitions-api/ Is the
disposeScreenAfterTransitionEnd = true
We are considering replace the StepDisposable with this approach if is stable enough.
This really helped me, thank you. If I add a delay in between the pop
and push
calls, it will work:
val coroutineScope = rememberCoroutineScope()
...
Button(
onClick = {
navigator.pop()
// don't push immediately, wait for Compose tick to happen, and for the screen model to be disposed
coroutineScope.launch {
// the tick can be just 1 millisecond, it doesn't have to be long
delay(1)
navigator push MyAwesomeNextScreen()
}
}
)
...
This works, but because of the delay, even thought it's tiny (just 1 millisecond), the screen flickers, at least in my use case, it doesn't look very nice.
@DevSrSouza Is there an API we could use to dispose a screen model manually? Without needing a delay? I have tried using the link you provided, but I don't use transitions on Desktop, I present my screens side-by-side instead. I also tried setting
disposeBehavior = NavigatorDisposeBehavior(disposeSteps = false),
in the Navigator, but that didn't help.
Hello! A little late to the discussion but I've found a solution that acts as replaceUntil
:
import cafe.adriel.voyager.core.stack.Stack
fun <Item> Stack<Item>.replaceUntil(item: Item, predicate: (Item) -> Boolean) {
val items = items.takeUntilInclusive(predicate) + item
this.replaceAll(items)
}
fun <T> List<T>.takeUntilInclusive(predicate: (T) -> Boolean): List<T> {
val result = mutableListOf<T>()
for (item in this) {
result.add(item)
if (predicate(item)) break
}
return result
}
Due to how voyager works internally the items inserted into the replaceAll
call that already exist won't be recreated, all new ones will be created and all items that are no longer there will get disposed!
Describe the bug I have been trying to figure out why my screenModel retains its state and data after the screen is popped and the screenModel should be disposed, I have my screen defined as this:
Then if I navigate from ScreenA to ScreenB, I see the "Created ScreenBViewModel" Log Then if I navigate back (either with system back button or my
onNavigateBack
callback I do see the "Disposed ScreenBViewModel" (this is as expected) however if, while on ScreenB, I call myonNavigateScreenC
callback I do not see the "Disposed ScreenBViewModel" and then if I navigate back to ScreenA and then forward into ScreenB again I do not see the "Created ScreenBViewModel" Log meaning that this is still using the previous instance that should have been disposed.in Testing I have found that if I replace the
with
then the ScreenModel is disposed correctly which is fine for this example but in other places I may need to pop 2 or 3 times before pushing a new screen, and I need to screenModel(s) to be disposed correctly in those instances
To Reproduce Steps to reproduce the behavior:
Expected behavior I would expect that is I pop a screen, then it's associated ScreenModel should be disposed. Even if that pop is immediately followed by a push.
Voyager module and version:
voyager-navigator:1.0.0
voyager-screenmodel:1.0.0
voyager-tab-navigator:1.0.0
voyager-transitions:1.0.0
voyager-koin:1.0.0
Koin module and version:
koin-bom:3.5.3
koin-core
koin-compose
koin-android
Snippet or Sample project to help reproduce See code snippet above