https://kotlinlang.org logo
Join Slack
Powered by
Should we be disabling onPressBack in a TopAppBar ...
# compose-android
b
Should we be disabling onPressBack in a TopAppBar in order to prevent popBackStack() spam? on Navigation Compose 2.6.0. I don't run into this issue but on 2.7.5. I do. If I have two screens with their own backButton, I'm able to spam click through until I end up with an empty graph. Edit: I've attached multiple videos showing the behavior between 2.6.0. and 2.7.5. on both my app and a sample project app which I've provided the code file for in the thread. Any suggestions would be much appreciated.
here's 2.6.0.
2.6.0..webm
here's 2.7.5.
2.7.5.webm
I've traced the issue back to happening all the way in 2.7.0-alpha01, the first usable version after 2.6.0
I'm always calling popBackStack() also, not sure if this is when I'd want to use navigateUp() instead or something? But anytime my TopAppBar has a backButton it calls navController.popBackStack()
In order to test this behavior you can create a new compose sample android project and copy this code in.
MainActivity.kt
then just go into the build.gradle and ensure you're on 2.7.0 or newer, and then switch to 2.6.0. the behavior does not happen on 2.6.0. but does on 2.7.0. or newer.
and use the buttons at the top in the content of the scaffold instead of the buttons at the bottom in the bottomBar.
MainActivity.kt
MainActivity.kt
here's a video of the sample code on 2.7.5.
navigation bug.webm
here's a video of the sample code on 2.6.0. no bug or issues with the graph getting cleared.
2.6.0..webm
s
Does the same happen when you call navigateUp instead? In fact I think the top app bar's back button is the only place where you want to call navigateUp anyway, so I would try with that too.
b
I'll give that a shot, thanks.
s
Please do tell here if that helps, I am very curious myself too. Otherwise it may be worth pinging someone from Google to hear their thoughts about this.
b
For sure! My MainActivity.kt plops right into a new compose sample project created with Android studio, and the only variable you need to change to test is from compose navigation 2.6.0 to 2.7.0+ in order to test it, if you wanted to play around with it. But when I get back to the desk I'll try navigateUp() instead
thank you color 1
i
Are you only handling button clicks when the Lifecycle of the current screen is RESUMED? That's how you prevent clicks when a transition is already happening. Otherwise, yeah, tapping it multiple times will pop multiple times (and you'd want to stop showing the button if your current screen is the start destination of your graph (which should always be the last screen before leaving your app)
b
So the start destination of my graph does not include the back button at all. I'm not doing anything special with my button clicks at all. But it seems like the multiple presses from the screens that do have back button on clicks that pop the back stack are causing the last screen on the stack which does not have a back button to be popped off. My Main activity.kt I shared above is a simple example I created using the compose template from Android studio and you can reproduce by building to emulator and pressing the buttons to navigate to the end and then spam clicking the back button on the top app bar. I've demonstrated this in some videos above as well. Do you have any examples of only handling button clicks if the current screen is resumed like you've mentioned?
After I upgraded my project at work to 2.7.3. I ran into this issue pretty quickly and then whipped up that sample app to test and came to the conclusion that this happens specifically when upgrading compose navigation past 2.6.0.
i
And this is something you've always been able to do: Compose never disables your buttons, even when a transition is happening, so they're always "live"
b
Thanks for sharing. I'm guessing you would always want to do this check for the back button of a TopAppBar?
I noticed in the now in Android sample application that they simply call popBackStack, is this bug something you might run into there?
It's kind of curious as to why it doesn't happen on navigation 2.6.0 as well but does on 2.7.0+
i
It certainly does happen on every version of Navigation. The default animations we get from AnimatedContent are a bit slower though, so it may certainly be easier to trigger. Mouse clicking in an emulator is way faster than a finger if you want to try that
b
Right, I've only tried mouse clicking for both 2.6.0 and 2.7.3. I guess I'm just not fast enough as I've only ran into it on 2.7.3. I guess as a rule of thumb should we be doing what you suggested in that stack overflow post? Are there ever cases when a simple popBackStack alone is good enough? Or would you always want to check either the lifecycle or disable the button after being clicked once.
i
Like I said, do you want to pop more things while a transition is happening? That's a legit thing to do (you can do that with the system back button for instance). You can do that, but then you have to make sure you don't run out of back stack to pop
b
Hmmm, considering that I've switched to a per screen TopAppBar and that back action should only navigate from current screen to previous screen only, probably not. The behavior with the system back button will at least close the app out if a spam happens, whereas the issue I'm having with popBackStack leaves me with an empty screen the color of my Scaffold background color.
i
The 
popBackStack
guide does specifically say that if it returns false, you need to finish your activity: https://developer.android.com/guide/navigation/backstack#handle-failure But the Up button in a TopAppBar should never exit your app - that's the key difference between Up and system back. Which is why 
navigateUp
exists
b
I see. I'm planning on giving navigateUp() a shot. I have a screen that implements a backHandler and then shows a confirmation dialog. Would I want this to perform navigateUp() as well? So far I haven't thought of a case where I've wanted my TopAppBar back button action to do anything different that the system back button. (Especially since my topmost screen on the back stack does not have a back button but a hamburger icon that opens a modal drawer).
i
A BackHandler never, ever should be calling any NavController operation. It should only be 
enabled
when it should be handling back and disabled otherwise (to let Navigation handle back). That's how you'll get the Predictive Back animation for free in the future
b
Apologies, the backHandler simply shows a dialog when enabless. It's the confirmation action on the dialog I'm talking about. Should that popBackStack? Or should that navigateUp() same as what I'm planning on switching the TopAppBar action to?
I.e. I'm passing one lambda from graph to screen called onNavigateBack() which right now is used for the confirmation dialog and the TopAppBar back button and that just invokes popBackStack() in the graph where the navController is.
i
No, you'd just call 
popBackStack
b
So I'd want to have two lambdas, one for handling the back action from the dialog and one for handling the back action from the TopAppBar?
i
Seems reasonable since they're two separate things
b
I see. Even with navigateUp() you'd probably still want to check the LocalLifecycleOwners state, right? I wouldn't think that using navigate up would inherently protect against an extra onClick from happening during a transition.
i
It does not, no
b
I noticed here: https://github.com/android/nowinandroid/blob/main/app/src/main/kotlin/com/google/samples/apps/nowinandroid/navigation/NiaNavHost.kt. the onBackClick for search screen which is tied to a back button in a top app bar simply calls popBackStack, they're also using compose 2.7.3. so would it be likely to run into this issue on this app here? I'm guessing that popBackStack here is also not ideal since it's a TopAppBar? Or is it because of the way that the app is modularized and it might not be possible to know where that back action is going to be implemented so popBackStack is used instead?
OnBackClick which is passed to the SearchToolBar ( a custom TopAppBar from my understanding)
i
Any click event would have this issue, it isn't Navigation specific. That's just a specific example of something you may not want to trigger during an animation. A submit button might want that same behavior but for a different reason
b
Yeah I kind of figured that'd be the case. Seems like it'd make sense to make a Util function that takes in an action for handling this for any onClick event that we're wanting to prevent multiple actions on? Or is it more common to just disable buttons after they've been clicked?
i
Disabled buttons change their visual appearance, which might be what you want for some cases (a Submit button maybe), but not for others (you wouldn't want your button to change appearance while sliding off screen). Really you need to take these case by case and figure out 1) what state you care about 2) what changes that state makes on your UI
b
Makes sense. I appreciate your time!
👍 1
s
Guarding against navigation events when the lifecycle isn't resumed has definitely resulted in bugs sometimes for me, so be careful when you do that too. In particular, I was triggering a navigation event as a bottom sheet was dismissed, after the user had finished inputting something there and pressed confirm. That button closed the bottom sheet and triggered the nav event. Little did I know that meant that the lifecycle was not resumed at that point. Another one was when there was an event coming from the VM which resulted in me wanting to navigate (successfully did something, so wanna go to success screen). Then it just so happened that sometimes, the user would've not been in the resumed state at that exact moment, I think again due to bottom sheets animating? Don't remember. Again, used unsafe navigation for that case and it worked. That's why I made this navigate function which takes the NavBackStackEntry, for the normal scenarios, and then one which just is called unsafe that just navigates directly, for these scenarios. https://github.com/HedvigInsurance/android/blob/develop/app/app/src/main/kotlin/com/hedvig/android/app/navigation/HedvigNavHost.kt#L283-L314 Haven't found a better way to do this.
👀 1
3 Views
Open in Slack
PreviousNext
Powered by