Posted By: Torkel Velure
I recently found a bug that would cause a crash in all the apps I ever made. In some apps the crash had never happened, even though the bug was there, and in others the crash would happen 1 in every 1 million sessions or so, which made it a low priority issue.
What caused the crash?
I always use a navigation graph where each fragment has a list of actions available, something like this:
And as an example, HomeFragment has two accounts you can click on and navigate to AccountFragment.
| account1.setOnClickListener { findNavController().navigate(HomeFragmentDirections.toAccount("1")) } | |
| account2.setOnClickListener { findNavController().navigate(HomeFragmentDirections.toAccount("2")) } |
Now, if you click on account1 and account2 very quickly (almost simultaneously), the app crashes with:
Fatal Exception: java.lang.IllegalArgumentException
Navigation action/destination no.shortcut:id/to_account cannot be found from the current destination a(no.shortcut:id/account_fragment) class=no.shortcut.AccountFragment
Why?
The click listener for account1 fires and navigates to AccountFragment.
The click listener for account2 fires and tries to navigate to Account again, but we are already on AccountFragment. AccountFragment does not have access to the AccountFragment action/destination, which makes the app crash.
If you click two “buttons” that navigate and the second navigation action/direction is not available to the first destination, the app crashes with IllegalArgumentException.
Workaround 1
The easiest “solution” is to declare all actions at the top level. If every fragment has access to all other fragments it won’t crash. However, it will navigate twice and put the first destination on the backstack, which could be confusing if the user does not realize that they clicked twice.
I am text block. Click edit button to change this text. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut elit tellus, luctus nec ullamcorper mattis, pulvinar dapibus leo.
👆 No crash because the action is available to all fragments.
Workaround 2
A better solution is to wrap the navigation function and validate that the currentDestination is the same as the calling destination. We could do that with a simple if statement before every navigation call:
| account1.setOnClickListener { | |
| val controller = findNavController() | |
| if(controller.currentDestination == R.id.account_fragment){ | |
| controller.navigate(HomeFragmentDirections.toAccount("1")) | |
| } | |
| } | |
However, I find it kind of clunky to pass the current destination id every time, so a nicer solution can be to compare the class name of the caller with the class name of the current destination:
| fun Fragment.navigate(directions: NavDirections) { | |
| val controller = findNavController() | |
| val currentDestination = (controller.currentDestination as? FragmentNavigator.Destination)?.className | |
| ?: (controller.currentDestination as? DialogFragmentNavigator.Destination)?.className | |
| if (currentDestination == this.javaClass.name) { | |
| controller.navigate(directions) | |
| } | |
| } |
| account1.setOnClickListener { | |
| navigate(HomeFragmentDirections.toAccount("1")) | |
| } |
Job Offers
OUR VIDEO RECOMMENDATION
Jobs
With this solution the first navigation call will be executed, and the second one will be ignored. The refactoring job also becomes easier as all we have to do is replacefindNavController.navigate() with our ownnavigate() function.
How do other apps behave?
If you click to navigate twice from the same destination, should you navigate once or twice? Should the first or second navigation be used? What should the backstack look like?
Even amongst the giants there does not seem to be a common implementation when it comes to navigation.
In the Airbnb app, if you click on two cities, it will open the search screen and add a second search screen to your backstack.
The same happens in the Play Console app:
The native settings app only navigates once:
The Youtube app will only open one video, but if you click on a video and a channel, it will open the video and put the channel on the backstack. Here we open a Runescape video, and when we go back we come to the Numberphile channel:
Confused?
What is the correct behaviour?
To me it seems more correct to only navigate once, and only respect the first navigation. Should you even be able to click on anything else after intialiating navigation? Assuming no-one double clicks on purpose, but that it sometimes happens by mistake, then only navigating once makes more sense.
At least the app should not crash, like all my apps used to do 😅
How does your app behave? What do you think is the correct behaviour? Is double navigation a bug or a feature?
Would love to hear your thoughts in the comments 👇
