Multiple Instances of Screens and Actions Triggered by Rapid Taps

qwexter commented 1 month ago

Bitwarden Beta

[X] I'm using the new native Bitwarden Beta app and I'm aware that legacy .NET app bugs should be reported in bitwarden/mobile

Steps To Reproduce

Open the Bitwarden Android app. Rapidly tap any navigation button multiple times. Observe that multiple instances of the same screen are opened. Navigate back through the screens. Note the occurrence of empty screens or broken navigation requiring an app restart. Perform an action that involves a network request (e.g., synchronization). Rapidly initiate the action multiple times. Observe that multiple network requests are sent.

Expected Result

The app should process single events only once, preventing multiple instances of the same screen or multiple network requests from being triggered.

Actual Result

Multiple instances of the same screen can be opened, and multiple network requests can be sent, leading to broken navigation and potential app instability.

Screenshots or Videos

Additional Context

The Bitwarden Android app allows users to perform the same action multiple times when rapidly tapping buttons. This is particularly noticeable with navigation buttons, where multiple instances of the same screen can be opened. Navigating back through these screens can lead to empty screens or broken navigation, sometimes requiring a restart of the app. This issue also occurs with actions involving network requests, such as synchronization, resulting in multiple requests being sent.

This problem stems from how the app handles user events within the MVI and Compose architecture. It affects the entire app and should ideally be addressed at the architectural level.

Build Version

Version 2024.6.0

Environment Details

Android Emulator, Pixel 8, API 33
Reakme c35, API 32

Issue Tracking Info

[X] I understand that work is tracked outside of Github. A PR will be linked to this issue should one be opened to address it, but Bitwarden doesn't use fields like "assigned", "milestone", or "project" to track progress.

bitwarden-bot commented 1 month ago

Thank you for your report! We've added this to our internal board for review. ID: PM-10075

mak1nt0sh commented 1 month ago

To solve the issue of duplicate navigation events, especially during rapid interactions or animations, you can implement a safety check using the current NavBackStackEntry's lifecycle state. This approach is inspired by the Jetsnack sample from the Compose samples repository.

Add a lifecycle check extension function to NavBackStackEntry:

/**
 * If the lifecycle is not resumed it means this NavBackStackEntry already processed a nav event.
 *
 * This is used to de-duplicate navigation events.
 */
fun NavBackStackEntry.lifecycleIsResumed() =
    this.lifecycle.currentState == Lifecycle.State.RESUMED

Create a safe navigation extension function for NavController:

fun NavController.safeNavigate(
    route: String,
    navOptions: NavOptions? = null,
    navigatorExtras: Navigator.Extras? = null
) {
    val navBackStackEntry = currentBackStackEntry
    if (navBackStackEntry?.lifecycleIsResumed() == true) {
        navigate(route, navOptions, navigatorExtras)
    }
}

Use safeNavigate instead of navigate for all navigation actions.

This solution ensures that navigation events are only processed when the current NavBackStackEntry is in the RESUMED state, effectively preventing duplicate navigations during animations or rapid interactions.

For more context, you can refer to the implementation in the Jetsnack sample: https://github.com/android/compose-samples/blob/081721ad44dfb29b55b1bc34f83d693b6b8dc9dd/Jetsnack/app/src/main/java/com/example/jetsnack/ui/JetsnackAppState.kt#L146

For handling double-click issues in non-navigation events, you might want to consider this: https://android-review.googlesource.com/c/platform/frameworks/support/+/2944594

michaldrabik commented 4 weeks ago

There's also 1st line of defense against this by simply debouncing taps by 300-500ms. It's just on UI level but we usually combine this with navigation guards proposed above since there might be some actions not related to nav.

bitwarden / android