Hey guys, I’m trying to wrap my head around the ne...

# compose

[svenjacobs]

Hey guys, I’m trying to wrap my head around the new dropUnlessResumed API for navigation from Lifecycle 2.8.0 and how to deal with navigation callbacks that have arguments.

[svenjacobs]

So we got this nice new dropUnlessResumed (and other new related functions), which should prevent multiple navigation to the same screen or popping the back stack too often when a button was clicked in fast succession. Note that this function only works in conjunction with Navigation 2.8.0-beta01 because LocalLifecycleOwner was moved to a new package in Lifecycle 2.8.0.

Anyway, the release notes even show a simple example.

onClick: () -> Unit = dropUnlessResumed { navController.navigate(NEW_SCREEN) }

Also, according to the official documentation we should expose navigation events from composables (callbacks) and not pass in the NavController into composables, which is a good recommendation.

So what I usually do is that I bubble up all those navigation events to the place where the route is defined, where NavController is instantiated and where NavHost is placed. Then inside of all those navigation callbacks I call navController.navigate(ROUTE) or navController.popBackStack(), whatever is required. But of course there are routes that have arguments. Let’s imagine a simple onItemClick: (itemId: String) -> Unit which should open a detail screen of that specific item. However dropUnlessResumed only accepts lambdas without arguments (() -> Unit) so how am I supposed to use this “wrapper” for callbacks with arguments?

Do I get this right that I have to use dropUnlessResumed inside each composable where the original click event originates and before I enrich it with additional arguments? Something like

@Composable
fun Item(
  itemId: String,
  title: String,
  onItemClick: (itemId: String) -> Unit,
) {
  Column {
    Text(title)
    
    Button(
      onClick = dropUnlessResumed {
        onItemClick(itemId)
      }
    ) {
      Text("Show item")
    }
  }
}

I don’t know but it kind of feels wrong. I should not have to add this function somewhere deep in my composable hierarchy, which is mostly only required for navigation. I would rather like to have this safeguard at the place where navigation is defined and performed.

What do you think? Do you think this is okay? Did you find another solution?

[Stylianos Gakis]

Yeah I and those are composable functions too, so hoisting it to the NavGraphBuilder extension function isn't quite possible either in the first place.
What I've done so far before this API existed was pass up in the lambda a signature like onFoo: (NavBackStackEntry, Param) -> Unit, so that I can hoist the "do not do this when not resumed" part up to the highest point, and no screen itself needs to know about this detail. Since from the NavBackStackEntry you can grab its lifecycle to conditionally do nothing.

On the other hand, there have been times where I do in fact want something to be done even we are not in resumed state (bottom sheet is dismissing for example) and in those cases I suddenly had to know in my NavGraphBuilder extension function that this lambda had to not guard against this lifecycle check.

Perhaps the suggestion with this is to in fact do this check inside the screen itself. Or at least at the top-level of your screen if anything, so you get full control there.
Dropping this check all the way down to some small Item composable would be very susceptible to making mistakes for sure. Idk how your screens typically look like, but what I do is something like this, where the destination-level composable takes in a VM and then there is a more generic screen level one. I could imagine adding the dropUnlessResumed calls at that destination composable for example, so that it's still high enough that small composables don't need to know about it, while also grabbing the correct lifecycle and allowing some lambdas not to do this if you know you don't want them to be drop unless resumed?

[Stylianos Gakis]

If you do end up playing more with this and find a good balance please do let me know. I think I will not migrate to those yet so as not to have a mixed approach all over the app. Might revisit it later or if I find a good answer to this question myself too

[Pablichjenkov]

Haven't used it but the word drop kinda keeps me away from it

[svenjacobs]

Dropping this check all the way down to some small Item composable would be very susceptible to making mistakes for sure.

You’re right but dropUnlessResumed is a @Composable function and therefor requires a compose context. This is because it accesses LocalLifecycleOwner to check its current state and drop the function if necessary.

So once I have my regular, augmented callback onItemClick: (itemId: String) -> Unit it’s not possible to call dropUnlessResumed inside of it some layers above.

[svenjacobs]

@Pablichjenkov

Haven’t used it but the word drop kinda keeps me away from it

Why? It actually makes sense and solves a common problem in Compose navigation.

[Stylianos Gakis]

Well drop is just a word, and what this API does is in fact a common use case for some scenarios, especially around navigation as we're discussing here.
Not all "drop" are problematic. It depends on the context.

[Pablichjenkov]

Sounds to me like it allows to lose events, why not enque them

[Stylianos Gakis]

Because you do want to lose the event of navigating somewhere if you're already navigating to it?

[Stylianos Gakis]

Please read the context of this question, it's not about everything in general, but in the context of navigation

[svenjacobs]

> Sounds to me like it allows to lose events, why not enque them
Because you don’t want to call popBackStack() multiple times for example when someone clicked too fast on the back/close button. It may cause undefined behaviour in your app.

[Pablichjenkov]

Got it, yes I was off context

[Stylianos Gakis]

Going back to the question at hand. The "highest" place you could call this function is inside the composable lambda. And indeed I do not see a nice way to make this work along with parameters, which you are very often going to have 🤷

[svenjacobs]

I would really like to have a small example or example project by Google that showcases the usage of dropUnlessResumed that goes beyond some simple examples of navigation events without any arguments. Because usually we do have arguments for our navigation events.

[Stylianos Gakis]

It instead kinda makes you do the opposite, so what you were showing with the item composable example, where it only works really at the call site where you can access the parameter that you want to send upwards, and you do the dropUnlessResumed { doThing(paramYouHaveAccessToInThisContext) } there.
Again, maybe this kinda is the "right" way to do it, since it's only at the lower lever that you really know if you want to drop follow-up invocations or not, so maybe that's what this API is supposed to be used like

[svenjacobs]

I think an Item itself should not have to know that its callback should or must be dropped at a certain lifecycle state. I think this makes the Item too aware of the outside conditions (e.g. we’re using Navigation Compose) and probably breaks some architecture and best practices recommendations 😉

[Stylianos Gakis]

Going back to my example where I had a navigation even happening from a button which was at the same time dismissing a bottom sheet, or a dialog or smth like that. Don't remember, but it was something that was making the screen's Lifecycle to not be RESUMED when it was showing.
There, only at that lowest level will you actually know that this lambda is sent from this sheet context, where you do not want to drop it if it's not resumed. If you do it higher up, you might be making a wrong assumption, since that lambda could be called in 2 places, once like that, and once from a normal button where you do want to do dropUnlessResumed

[Stylianos Gakis]

I think an Item itself should not have to know that it’s callback should or must be dropped at a certain lifecycle state. I think this makes the Item too aware of the outside conditions (e.g. we’re using Navigation Compose) and probably breaks some architecture and best practices recommendations 😉

Well technically it doesn't know anything about navigation despite this, this is lifecycle related, not navigation related. The lifecycle just happens to be driven by navigation in this case.

And often it really is the case that only the real call-site (and not the ones just delegating it down) know if it wants to respect the lifecycle or not.
When done on the nav-graph level, I did remember now that I've also had problematic scenarios where a navigation lambda was hit by a button, so I protected it from the lifecycle like that. Then later, some state was also driving this navigation, basically if some state was set, we were navigating there and resetting the state. In that case we were sometimes silently dropping this nav event, despite clearing the state.

Again, I don't have the answers here, I am also quite curious myself how can one protect themselves as best as possible from misusing this API and nav in general with it.

[svenjacobs]

Let's see if someone from Google maybe reads this thread and can clear things up 🤞🏼

[svenjacobs]

I ended up writing my own version of dropUnlessResumed which is not a composable function.

fun dropUnlessResumed(
    lifecycleOwner: LifecycleOwner,
    block: () -> Unit,
) {
    if (lifecycleOwner.lifecycle.currentState.isAtLeast(State.RESUMED)) {
        block()
    }
}

So I can use it like this

NavHost(...) {
  composable("main") { navBackStackEntry ->
    MainScreen(
      onItemClick = { itemId -> 
        dropUnlessResumed(navBackStackEntry) {
          navController.navigate("itemDetail?id=$itemId")
        }
      }
    )
  }
}

[Stylianos Gakis]

Yeah I do this https://github.com/HedvigInsurance/android/blob/develop/app%2Fapp%2Fsrc%2Fmain%2Fkotlin%2Fcom%2Fhedvig%2Fandroid%2Fapp%2Fnavigation%2FRememberNavigator.kt#L20-L28
More or less the same thing

[svenjacobs]

That’s also a nice solution.

[Stylianos Gakis]

Copy freely if you wish 😅

[youssef hachicha]

I don't understand how the lifecycle is related or can solve the issue of clicking way too fast on the nav back button, can you elaborate more on that?

[svenjacobs]

The NavBackStackEntry provided by Navigation is itself a LifecycleOwner and the state is changed when a navigation is taking place. dropUnlessResumed takes the lifecycle from LocalLifecycleOwner and thus drops the second click event when the state is not RESUMED.

[Stylianos Gakis]

https://kotlinlang.slack.com/archives/CJLTWPH7S/p1703273055773909?thread_ts=1703264824.524189&cid=CJLTWPH7S
Previous discussion on how while there is a navigation going on, neither of the two screens which are animating in/out are in the RESUMED state

[Stylianos Gakis]

And here https://kotlinlang.slack.com/archives/C04TPPEQKEJ/p1699677678803439?thread_ts=1699651681.817549&cid=C04TPPEQKEJ

[youssef hachicha]

My knowledge about lifecycle is that if we exist the app without closing it the state of lifecycle change to paused if I enter back again its resumed and if I close the app its destroyed, seems like I am missing something right ?

[svenjacobs]

This specific lifecycle from NavBackStackEntry is bound to the lifecycle of the navigation, not of the app or the surrounding composable.

[Stylianos Gakis]

And each screen has their own instance of lifecycle, as Sven mentioned each destinations is a LifecycleOwner itself. Meaning that within the same app, two screens can and absolutely will be in different lifecycle states at the same time in various scenarios.

[Stylianos Gakis]

Btw for this https://kotlinlang.slack.com/archives/CJLTWPH7S/p1716317807630569?thread_ts=1716299306.290129&cid=CJLTWPH7S

What do you do besides doubles checking the impl that onItemClick for example does in fact want the even dropped if the state is not resumed?
I was doing some refactoring on some screen today and I figured that some lambdas did and some did not want to drop the event, and I had very low confidence that I wasn't breaking anything in my refactor so I had to go inside those small composables as we said and look when the lambda is actually invoked to know if I want or don't want to respect the lifecycle. Quite brittle and I am sure I can easily introduce bugs here.

The more I think about this, maybe we do want that little composable to call dropUnlessResumed itself if it knows it's doing a navigation event. Which I suppose is only "enforceable" through naming conventions on the lambdas that do navigation.

[svenjacobs]

Hm, I currently don't have a scenario where I don't want the event to be dropped so I can't say 🤔

[Stylianos Gakis]

Have you ever had a bottom sheet which has a dialog which should dismiss and navigate? When the dialog is animating away the lifecycle isn't on resumed yet as I recall.

Or doing a navigation from a state update coming from the VM. So your user may have just received a phone call, so the screen is not resumed, and the state comes in at that moment, and you do want to navigate still instead of dropping it.
Unless I suppose we'd have to wait to call the nav event while the lifecycle isn't resumed yet even if our state was updated to the "now navigate" state 👀

[svenjacobs]

Hm, interesting. I would have to test this if we have these problems.