Skip to content

Fix rapid-navigation misclicks and speed up screen transitions #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Marcogn merged 1 commit into from
Aug 21, 2026
Merged

Fix rapid-navigation misclicks and speed up screen transitions #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,21 @@ versioning follows the app's `versionName` in `app/build.gradle.kts`.

## [Unreleased]

- **Fixed rapid navigation taps landing on the wrong screen, and gave screen transitions a modern, faster feel.**
Navigating quickly (e.g. Home → Reviews → back → Backlog done in rapid succession) could
register the tap on the screen still fading out underneath instead of the intended
destination — reported live on-device, reproducible whenever two navigation actions happened
within the transition window. Root cause: `navigation-compose` 2.8.5's default transition is a
700ms crossfade during which both the outgoing and incoming screens stay composed and
clickable at the same time, so a fast second tap could hit whichever one happened to occupy
that screen coordinate. `ThePatientGamerHelperNavGraph` now (1) declares explicit, faster
slide+fade transitions (300ms, Material-style directional push/pop) instead of the default
crossfade, and (2) gates every `navigate()`/`popBackStack()` call behind a check that the
*specific* `NavBackStackEntry` triggering it has actually reached `RESUMED` — the officially
documented signal that its own transition has fully settled — so a stray tap on a screen still
mid-transition is ignored rather than misrouted. Also addresses the general "navigation feels
slow" complaint, since 300ms directional transitions read as snappier than the previous
generic 700ms fade.
- **Fixed a `release.yml` bug that could burn a version number on a failed signed build.** The
workflow used to commit and push the `versionName`/`versionCode` bump and the cut
`CHANGELOG.md` section to `main` *before* attempting the signed build. Found on the sibling
Expand Down
210 changes: 151 additions & 59 deletions ...ain/java/com/marcogn/thepatientgamerhelper/ui/navigation/ThePatientGamerHelperNavGraph.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
package com.marcogn.thepatientgamerhelper.ui.navigation

import androidx.compose.animation.AnimatedContentTransitionScope
import androidx.compose.animation.EnterTransition
import androidx.compose.animation.ExitTransition
import androidx.compose.animation.core.FastOutSlowInEasing
import androidx.compose.animation.core.tween
import androidx.compose.animation.fadeIn
import androidx.compose.animation.fadeOut
import androidx.compose.animation.slideInHorizontally
import androidx.compose.animation.slideOutHorizontally
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
Expand All @@ -23,6 +32,8 @@ import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.dp
import androidx.lifecycle.Lifecycle
import androidx.navigation.NavBackStackEntry
import androidx.navigation.NavHostController
import androidx.navigation.compose.NavHost
import androidx.navigation.compose.composable
Expand All @@ -41,6 +52,42 @@ import com.marcogn.thepatientgamerhelper.ui.settings.SettingsScreen
import com.marcogn.thepatientgamerhelper.ui.stats.StatsScreen
import kotlinx.coroutines.launch

// A NavBackStackEntry only reaches RESUMED once its enter/exit transition animation has
// fully completed and it is settled on top of the back stack (see NavHost/AnimatedContent
// docs). Gating every navigate()/popBackStack() call behind this check on the *specific*
// entry that owns the callback is the officially recommended fix for a fast double-tap
// (e.g. back then immediately tapping another destination) landing on a screen that is
// still being composed/torn down mid-transition instead of the intended one.
private fun NavBackStackEntry.lifecycleIsResumed() =
lifecycle.currentState == Lifecycle.State.RESUMED

private const val NAV_ANIM_DURATION_MS = 300

private val navEnterTransition: AnimatedContentTransitionScope<NavBackStackEntry>.() -> EnterTransition = {
slideInHorizontally(
animationSpec = tween(NAV_ANIM_DURATION_MS, easing = FastOutSlowInEasing),
initialOffsetX = { fullWidth -> fullWidth },
) + fadeIn(animationSpec = tween(NAV_ANIM_DURATION_MS))
}
private val navExitTransition: AnimatedContentTransitionScope<NavBackStackEntry>.() -> ExitTransition = {
slideOutHorizontally(
animationSpec = tween(NAV_ANIM_DURATION_MS, easing = FastOutSlowInEasing),
targetOffsetX = { fullWidth -> -fullWidth / 4 },
) + fadeOut(animationSpec = tween(NAV_ANIM_DURATION_MS))
}
private val navPopEnterTransition: AnimatedContentTransitionScope<NavBackStackEntry>.() -> EnterTransition = {
slideInHorizontally(
animationSpec = tween(NAV_ANIM_DURATION_MS, easing = FastOutSlowInEasing),
initialOffsetX = { fullWidth -> -fullWidth / 4 },
) + fadeIn(animationSpec = tween(NAV_ANIM_DURATION_MS))
}
private val navPopExitTransition: AnimatedContentTransitionScope<NavBackStackEntry>.() -> ExitTransition = {
slideOutHorizontally(
animationSpec = tween(NAV_ANIM_DURATION_MS, easing = FastOutSlowInEasing),
targetOffsetX = { fullWidth -> fullWidth },
) + fadeOut(animationSpec = tween(NAV_ANIM_DURATION_MS))
}

@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun ThePatientGamerHelperNavGraph(navController: NavHostController = rememberNavController()) {
Expand All @@ -49,11 +96,15 @@ fun ThePatientGamerHelperNavGraph(navController: NavHostController = rememberNav

val openDrawer: () -> Unit = { scope.launch { drawerState.open() } }
val navigateFromDrawer: (Destination) -> Unit = { destination ->
scope.launch { drawerState.close() }
navController.navigate(destination) {
popUpTo(Destination.Home) { saveState = true }
launchSingleTop = true
restoreState = true
// Guards against a drawer tap landing while the current screen is still mid
// transition (see lifecycleIsResumed() above) - same race as forward/back taps.
if (navController.currentBackStackEntry?.lifecycleIsResumed() != false) {
scope.launch { drawerState.close() }
navController.navigate(destination) {
popUpTo(Destination.Home) { saveState = true }
launchSingleTop = true
restoreState = true
}
}
}

Expand Down Expand Up @@ -99,26 +150,35 @@ fun ThePatientGamerHelperNavGraph(navController: NavHostController = rememberNav
}
},
) {
NavHost(navController = navController, startDestination = Destination.Home) {
composable<Destination.Home> {
NavHost(
navController = navController,
startDestination = Destination.Home,
enterTransition = navEnterTransition,
exitTransition = navExitTransition,
popEnterTransition = navPopEnterTransition,
popExitTransition = navPopExitTransition,
) {
composable<Destination.Home> { entry ->
HomeScreen(
onMenuClick = openDrawer,
onReviewsClick = { navController.navigate(Destination.Library) },
onBacklogClick = { navController.navigate(Destination.Backlog) },
onStatsClick = { navController.navigate(Destination.Stats) },
onReviewsClick = { if (entry.lifecycleIsResumed()) navController.navigate(Destination.Library) },
onBacklogClick = { if (entry.lifecycleIsResumed()) navController.navigate(Destination.Backlog) },
onStatsClick = { if (entry.lifecycleIsResumed()) navController.navigate(Destination.Stats) },
)
}
composable<Destination.Library> {
composable<Destination.Library> { entry ->
LibraryScreen(
onMenuClick = openDrawer,
onReviewClick = { id -> navController.navigate(Destination.Detail(id)) },
onAddClick = { navController.navigate(Destination.Form()) },
onReviewClick = { id ->
if (entry.lifecycleIsResumed()) navController.navigate(Destination.Detail(id))
},
onAddClick = { if (entry.lifecycleIsResumed()) navController.navigate(Destination.Form()) },
)
}
composable<Destination.Stats> {
StatsScreen(onMenuClick = openDrawer)
}
composable<Destination.Settings> {
composable<Destination.Settings> { entry ->
SettingsScreen(
onBack = {
// Not a plain popBackStack(): that destroys the entry outright, clearing
Expand All @@ -127,87 +187,119 @@ fun ThePatientGamerHelperNavGraph(navController: NavHostController = rememberNav
// back arrow is the only way to leave it. Mirrors navigateFromDrawer's
// popUpTo/saveState/restoreState so the entry (and its login state) is
// preserved across drawer round-trips, same as Library/Backlog/Stats.
navController.navigate(Destination.Home) {
popUpTo(Destination.Home) { saveState = true }
launchSingleTop = true
restoreState = true
if (entry.lifecycleIsResumed()) {
navController.navigate(Destination.Home) {
popUpTo(Destination.Home) { saveState = true }
launchSingleTop = true
restoreState = true
}
}
},
)
}
composable<Destination.Detail> {
composable<Destination.Detail> { entry ->
DetailScreen(
onBack = { navController.popBackStack() },
onEdit = { id -> navController.navigate(Destination.Form(id)) },
onBack = { if (entry.lifecycleIsResumed()) navController.popBackStack() },
onEdit = { id -> if (entry.lifecycleIsResumed()) navController.navigate(Destination.Form(id)) },
onDeleted = {
navController.popBackStack(Destination.Library, inclusive = false)
if (entry.lifecycleIsResumed()) {
navController.popBackStack(Destination.Library, inclusive = false)
}
},
)
}
composable<Destination.Form> { backStackEntry ->
val route = backStackEntry.toRoute<Destination.Form>()
ReviewFormScreen(
onSaved = { id ->
navController.navigate(Destination.Detail(id)) {
popUpTo(Destination.Library)
if (backStackEntry.lifecycleIsResumed()) {
navController.navigate(Destination.Detail(id)) {
popUpTo(Destination.Library)
}
}
},
onCancel = {
if (route.backlogItemId != null) {
// Opened from the backlog's "want to write a review?" prompt: leaving
// (with or without an implicit draft save, see ReviewFormViewModel.onBackPressed)
// should land on the reviews library, not back into the Backlog stack it came from.
// Deliberately NOT saveState = true here: this popUpTo always pops the
// Backlog -> BacklogListDetail -> BacklogItemDetail -> Form chain we're
// discarding on purpose, and NavController's saveState/restoreState keys
// saved back-stack "islands" by the id of the *first* entry above the
// popUpTo target (Backlog here) with a write-once guard per key. Saving it
// poisoned the drawer's "Backlog" entry: the next drawer tap to Backlog
// (restoreState = true in navigateFromDrawer) restored this exact stale
// chain wholesale, landing back on this Form/Detail screen instead of the
// Backlog list, popping straight back past it on the next back press. See
// REG-13 in docs/test-plan.md.
navController.navigate(Destination.Library) {
popUpTo(Destination.Home)
launchSingleTop = true
restoreState = true
if (backStackEntry.lifecycleIsResumed()) {
if (route.backlogItemId != null) {
// Opened from the backlog's "want to write a review?" prompt: leaving
// (with or without an implicit draft save, see ReviewFormViewModel.onBackPressed)
// should land on the reviews library, not back into the Backlog stack it came from.
// Deliberately NOT saveState = true here: this popUpTo always pops the
// Backlog -> BacklogListDetail -> BacklogItemDetail -> Form chain we're
// discarding on purpose, and NavController's saveState/restoreState keys
// saved back-stack "islands" by the id of the *first* entry above the
// popUpTo target (Backlog here) with a write-once guard per key. Saving it
// poisoned the drawer's "Backlog" entry: the next drawer tap to Backlog
// (restoreState = true in navigateFromDrawer) restored this exact stale
// chain wholesale, landing back on this Form/Detail screen instead of the
// Backlog list, popping straight back past it on the next back press. See
// REG-13 in docs/test-plan.md.
navController.navigate(Destination.Library) {
popUpTo(Destination.Home)
launchSingleTop = true
restoreState = true
}
} else {
navController.popBackStack()
}
} else {
navController.popBackStack()
}
},
)
}
composable<Destination.Backlog> {
composable<Destination.Backlog> { entry ->
BacklogScreen(
onMenuClick = openDrawer,
onListClick = { listId -> navController.navigate(Destination.BacklogListDetail(listId)) },
onItemClick = { itemId -> navController.navigate(Destination.BacklogItemDetail(itemId)) },
onListClick = { listId ->
if (entry.lifecycleIsResumed()) {
navController.navigate(Destination.BacklogListDetail(listId))
}
},
onItemClick = { itemId ->
if (entry.lifecycleIsResumed()) {
navController.navigate(Destination.BacklogItemDetail(itemId))
}
},
)
}
composable<Destination.BacklogListDetail> { backStackEntry ->
val listId = backStackEntry.toRoute<Destination.BacklogListDetail>().listId
BacklogListDetailScreen(
onBack = { navController.popBackStack() },
onAddItemClick = { navController.navigate(Destination.BacklogItemForm(listId)) },
onItemClick = { itemId -> navController.navigate(Destination.BacklogItemDetail(itemId)) },
onBack = { if (backStackEntry.lifecycleIsResumed()) navController.popBackStack() },
onAddItemClick = {
if (backStackEntry.lifecycleIsResumed()) {
navController.navigate(Destination.BacklogItemForm(listId))
}
},
onItemClick = { itemId ->
if (backStackEntry.lifecycleIsResumed()) {
navController.navigate(Destination.BacklogItemDetail(itemId))
}
},
)
}
composable<Destination.BacklogItemForm> {
composable<Destination.BacklogItemForm> { entry ->
BacklogItemFormScreen(
onSaved = { navController.popBackStack() },
onCancel = { navController.popBackStack() },
onSaved = { if (entry.lifecycleIsResumed()) navController.popBackStack() },
onCancel = { if (entry.lifecycleIsResumed()) navController.popBackStack() },
)
}
composable<Destination.BacklogItemDetail> {
composable<Destination.BacklogItemDetail> { entry ->
BacklogItemDetailScreen(
onBack = { navController.popBackStack() },
onEdit = { itemId, listId -> navController.navigate(Destination.BacklogItemForm(listId, itemId)) },
onDeleted = { navController.popBackStack() },
onBack = { if (entry.lifecycleIsResumed()) navController.popBackStack() },
onEdit = { itemId, listId ->
if (entry.lifecycleIsResumed()) {
navController.navigate(Destination.BacklogItemForm(listId, itemId))
}
},
onDeleted = { if (entry.lifecycleIsResumed()) navController.popBackStack() },
onWriteReview = { itemId ->
navController.navigate(Destination.Form(backlogItemId = itemId)) { launchSingleTop = true }
if (entry.lifecycleIsResumed()) {
navController.navigate(Destination.Form(backlogItemId = itemId)) { launchSingleTop = true }
}
},
onOpenReview = { reviewId ->
if (entry.lifecycleIsResumed()) navController.navigate(Destination.Detail(reviewId))
},
onOpenReview = { reviewId -> navController.navigate(Destination.Detail(reviewId)) },
)
}
}
Expand Down
Loading