From c978ab727fe342577f048bd47615257637566c4f Mon Sep 17 00:00:00 2001 From: rushiiMachine <33725716+rushiiMachine@users.noreply.github.com> Date: Sun, 25 May 2025 02:02:54 -0700 Subject: [PATCH] fix: weird double push navigation bug The "Add Install" button was somehow able to push a screen with the same key onto the stack twice, messing up model retrieval downstream. --- .../manager/ui/components/BackButton.kt | 29 +++------------- .../manager/ui/screens/home/HomeScreen.kt | 6 ++-- .../ui/screens/iconopts/IconOptionsScreen.kt | 27 +++++++-------- .../manager/ui/screens/log/LogScreen.kt | 2 +- .../ui/screens/patching/PatchingScreen.kt | 1 + .../screens/patchopts/PatchOptionsScreen.kt | 12 +++---- .../com/aliucord/manager/util/Navigation.kt | 33 +++++++++++++++++++ 7 files changed, 61 insertions(+), 49 deletions(-) create mode 100644 app/src/main/kotlin/com/aliucord/manager/util/Navigation.kt diff --git a/app/src/main/kotlin/com/aliucord/manager/ui/components/BackButton.kt b/app/src/main/kotlin/com/aliucord/manager/ui/components/BackButton.kt index 52ab3e0..554cf05 100644 --- a/app/src/main/kotlin/com/aliucord/manager/ui/components/BackButton.kt +++ b/app/src/main/kotlin/com/aliucord/manager/ui/components/BackButton.kt @@ -1,29 +1,27 @@ package com.aliucord.manager.ui.components -import android.app.Activity import androidx.activity.compose.LocalActivity import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.runtime.Composable -import androidx.compose.ui.platform.LocalInspectionMode import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import cafe.adriel.voyager.navigator.LocalNavigator -import cafe.adriel.voyager.navigator.Navigator import com.aliucord.manager.R -import com.aliucord.manager.ui.screens.home.HomeScreen +import com.aliucord.manager.util.back /** * Standalone back button for interacting with the current navigator. */ @Composable fun BackButton() { - // Support rendering in @Preview - val context = if (LocalInspectionMode.current) null else LocalActivity.current val navigator = LocalNavigator.current + val activity = LocalActivity.current IconButton( - onClick = { if (context != null) navigator?.back(context) }, + onClick = { + navigator?.back(activity) + }, ) { Icon( painter = painterResource(R.drawable.ic_back), @@ -31,20 +29,3 @@ fun BackButton() { ) } } - -/** - * Custom back logic for handling special screens differently. - * If on Home, Landing, or Debug then exit immediately otherwise go back one. - */ -fun Navigator.back(currentActivity: Activity?) { - val top = this.lastItemOrNull ?: return - val stackSize = this.items.size - - if (stackSize > 1) { - pop() - } else if (top is HomeScreen) { - currentActivity?.finish() - } else { - replaceAll(HomeScreen()) - } -} diff --git a/app/src/main/kotlin/com/aliucord/manager/ui/screens/home/HomeScreen.kt b/app/src/main/kotlin/com/aliucord/manager/ui/screens/home/HomeScreen.kt index fe05885..0d9fce7 100644 --- a/app/src/main/kotlin/com/aliucord/manager/ui/screens/home/HomeScreen.kt +++ b/app/src/main/kotlin/com/aliucord/manager/ui/screens/home/HomeScreen.kt @@ -34,6 +34,7 @@ import com.aliucord.manager.ui.screens.plugins.PluginsScreen import com.aliucord.manager.ui.util.DiscordVersion import com.aliucord.manager.ui.util.paddings.PaddingValuesSides import com.aliucord.manager.ui.util.paddings.exclude +import com.aliucord.manager.util.pushOnce import kotlinx.coroutines.launch import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize @@ -48,7 +49,6 @@ class HomeScreen : Screen, Parcelable { val navigator = LocalNavigator.currentOrThrow val scope = rememberCoroutineScope() val model = koinScreenModel() - val onClickInstall = remember { { navigator.push(PatchOptionsScreen()) } } // Refresh installations list when the screen changes or activity resumes LifecycleResumeEffect(Unit) { @@ -65,7 +65,7 @@ class HomeScreen : Screen, Parcelable { state = state, supportedVersion = model.supportedVersion, padding = padding, - onClickInstall = onClickInstall, + onClickInstall = { navigator.pushOnce(PatchOptionsScreen()) }, onUpdate = { scope.launch { navigator.push(model.createPrefilledPatchOptsScreen(it)) @@ -80,7 +80,7 @@ class HomeScreen : Screen, Parcelable { InstallsState.None -> HomeScreenNoneContent( padding = padding, - onClickInstall = onClickInstall, + onClickInstall = { navigator.pushOnce(PatchOptionsScreen()) }, ) InstallsState.Error -> HomeScreenFailureContent(padding = padding) diff --git a/app/src/main/kotlin/com/aliucord/manager/ui/screens/iconopts/IconOptionsScreen.kt b/app/src/main/kotlin/com/aliucord/manager/ui/screens/iconopts/IconOptionsScreen.kt index fc8f948..f691308 100644 --- a/app/src/main/kotlin/com/aliucord/manager/ui/screens/iconopts/IconOptionsScreen.kt +++ b/app/src/main/kotlin/com/aliucord/manager/ui/screens/iconopts/IconOptionsScreen.kt @@ -23,35 +23,36 @@ import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import cafe.adriel.voyager.core.screen.Screen +import cafe.adriel.voyager.koin.koinScreenModel +import cafe.adriel.voyager.navigator.LocalNavigator +import cafe.adriel.voyager.navigator.currentOrThrow import coil3.compose.AsyncImage import com.aliucord.manager.R import com.aliucord.manager.ui.components.* import com.aliucord.manager.ui.screens.iconopts.components.* +import com.aliucord.manager.ui.screens.patchopts.PatchOptionsScreen import com.aliucord.manager.ui.util.ColorSaver import dev.zt64.compose.pipette.CircularColorPicker import dev.zt64.compose.pipette.HsvColor import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize -interface IconOptionsScreenParent : Parcelable { - /** - * The screen model is elevated outside of this screen - * in order to share the state with the parent screen. - */ - @Composable - fun getIconModel(): IconOptionsModel -} - @Parcelize -class IconOptionsScreen( - private val parent: IconOptionsScreenParent, -) : Screen, Parcelable { +class IconOptionsScreen : Screen, Parcelable { @IgnoredOnParcel override val key = "IconOptions" @Composable override fun Content() { - val model = parent.getIconModel() + // Retrieves a global model owned by the navigator + val navigator = LocalNavigator.currentOrThrow + val modelScreen by remember { + derivedStateOf { + navigator.items.lastOrNull { it is PatchOptionsScreen } + ?: error("No PatchOptionsScreen in stack") + } + } + val model = modelScreen.koinScreenModel() // Parameters are already injected in PatchOptionsScreen IconOptionsScreenContent( mode = model.mode, diff --git a/app/src/main/kotlin/com/aliucord/manager/ui/screens/log/LogScreen.kt b/app/src/main/kotlin/com/aliucord/manager/ui/screens/log/LogScreen.kt index 79ae2c1..7d8da7c 100644 --- a/app/src/main/kotlin/com/aliucord/manager/ui/screens/log/LogScreen.kt +++ b/app/src/main/kotlin/com/aliucord/manager/ui/screens/log/LogScreen.kt @@ -16,9 +16,9 @@ import cafe.adriel.voyager.navigator.currentOrThrow import com.aliucord.manager.R import com.aliucord.manager.manager.InstallLogData import com.aliucord.manager.ui.components.Label -import com.aliucord.manager.ui.components.back import com.aliucord.manager.ui.screens.log.components.LogAppBar import com.aliucord.manager.ui.screens.log.components.LogTextArea +import com.aliucord.manager.util.back import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize import org.koin.core.parameter.parametersOf diff --git a/app/src/main/kotlin/com/aliucord/manager/ui/screens/patching/PatchingScreen.kt b/app/src/main/kotlin/com/aliucord/manager/ui/screens/patching/PatchingScreen.kt index d524523..39a099d 100644 --- a/app/src/main/kotlin/com/aliucord/manager/ui/screens/patching/PatchingScreen.kt +++ b/app/src/main/kotlin/com/aliucord/manager/ui/screens/patching/PatchingScreen.kt @@ -38,6 +38,7 @@ import com.aliucord.manager.ui.screens.patchopts.PatchOptions import com.aliucord.manager.ui.util.paddings.* import com.aliucord.manager.ui.util.spacedByLastAtBottom import com.aliucord.manager.ui.util.thenIf +import com.aliucord.manager.util.back import com.aliucord.manager.util.isIgnoringBatteryOptimizations import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.flow.filter diff --git a/app/src/main/kotlin/com/aliucord/manager/ui/screens/patchopts/PatchOptionsScreen.kt b/app/src/main/kotlin/com/aliucord/manager/ui/screens/patchopts/PatchOptionsScreen.kt index 882df5e..53205ef 100644 --- a/app/src/main/kotlin/com/aliucord/manager/ui/screens/patchopts/PatchOptionsScreen.kt +++ b/app/src/main/kotlin/com/aliucord/manager/ui/screens/patchopts/PatchOptionsScreen.kt @@ -22,7 +22,8 @@ import com.aliucord.manager.R import com.aliucord.manager.ui.components.TextDivider import com.aliucord.manager.ui.components.dialogs.NetworkWarningDialog import com.aliucord.manager.ui.components.dialogs.UnknownSourcesPermissionDialog -import com.aliucord.manager.ui.screens.iconopts.* +import com.aliucord.manager.ui.screens.iconopts.IconOptionsModel +import com.aliucord.manager.ui.screens.iconopts.IconOptionsScreen import com.aliucord.manager.ui.screens.patching.PatchingScreen import com.aliucord.manager.ui.screens.patchopts.components.PackageNameStateLabel import com.aliucord.manager.ui.screens.patchopts.components.PatchOptionsAppBar @@ -39,15 +40,10 @@ import org.koin.core.parameter.parametersOf @Parcelize class PatchOptionsScreen( private val prefilledOptions: PatchOptions? = null, -) : Screen, Parcelable, IconOptionsScreenParent { +) : Screen, Parcelable { @IgnoredOnParcel override val key = "PatchOptions" - @Composable - override fun getIconModel(): IconOptionsModel { - return koinScreenModel() - } - @Composable override fun Content() { val context = LocalContext.current @@ -85,7 +81,7 @@ class PatchOptionsScreen( debuggable = model.debuggable, setDebuggable = model::changeDebuggable, - onOpenIconOptions = { navigator.push(IconOptionsScreen(this@PatchOptionsScreen)) }, + onOpenIconOptions = { navigator.push(IconOptionsScreen()) }, appName = model.appName, appNameIsError = model.appNameIsError, diff --git a/app/src/main/kotlin/com/aliucord/manager/util/Navigation.kt b/app/src/main/kotlin/com/aliucord/manager/util/Navigation.kt new file mode 100644 index 0000000..2f51b61 --- /dev/null +++ b/app/src/main/kotlin/com/aliucord/manager/util/Navigation.kt @@ -0,0 +1,33 @@ +package com.aliucord.manager.util + +import android.app.Activity +import cafe.adriel.voyager.core.screen.Screen +import cafe.adriel.voyager.core.stack.Stack +import com.aliucord.manager.ui.screens.home.HomeScreen + +/** + * Custom back logic for handling special screens differently. + * If on Home, Landing, or Debug then exit immediately otherwise go back one. + */ +fun Stack.back(currentActivity: Activity?) { + val top = this.lastItemOrNull ?: return + val stackSize = this.items.size + + if (stackSize > 1) { + pop() + } else if (top is HomeScreen) { + currentActivity?.finish() + } else { + replaceAll(HomeScreen()) + } +} + +/** + * Workaround for an issue where a button can push a screen with the same key twice onto the stack. + * https://github.com/adrielcafe/voyager/issues/474#issuecomment-2907701009 + */ +infix fun Stack.pushOnce(item: Screen) { + if (lastItemOrNull?.key != item.key) { + push(item) + } +}