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.
This commit is contained in:
rushiiMachine 2025-05-25 02:02:54 -07:00
parent a5571bc11f
commit c978ab727f
No known key found for this signature in database
GPG key ID: DCBE5952BB3B6420
7 changed files with 61 additions and 49 deletions

View file

@ -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())
}
}

View file

@ -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<HomeModel>()
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)

View file

@ -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<IconOptionsModel>() // Parameters are already injected in PatchOptionsScreen
IconOptionsScreenContent(
mode = model.mode,

View file

@ -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

View file

@ -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

View file

@ -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<IconOptionsModel>()
}
@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,

View file

@ -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<Screen>.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<Screen>.pushOnce(item: Screen) {
if (lastItemOrNull?.key != item.key) {
push(item)
}
}