From 087be8290f4a170fef1e02c86b27cbf1a5154bb7 Mon Sep 17 00:00:00 2001 From: Dima Date: Thu, 16 May 2024 20:57:06 +0900 Subject: [PATCH] Fixing AllByIdentifierScreen crashing after app going into background for too long. Upping versionCode to 62 --- .../navigation/CommonScreenNavigation.kt | 2 +- .../architecture/navigation/NavigationDialogs.kt | 5 +++++ .../phone/DashboardRootPhoneNavigation.kt | 14 ++++++++++---- .../tablet/TabletRightPaneNavigation.kt | 12 +++++++++--- .../android/screens/allitems/AllItemsScreen.kt | 6 +++--- .../android/screens/allitems/AllItemsViewModel.kt | 7 ++++--- .../addbyidentifier/AddByIdentifierViewModel.kt | 15 +++++++++++++-- buildSrc/src/main/kotlin/BuildConfig.kt | 2 +- 8 files changed, 46 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/zotero/android/architecture/navigation/CommonScreenNavigation.kt b/app/src/main/java/org/zotero/android/architecture/navigation/CommonScreenNavigation.kt index 4a66a65e..63ecec31 100644 --- a/app/src/main/java/org/zotero/android/architecture/navigation/CommonScreenNavigation.kt +++ b/app/src/main/java/org/zotero/android/architecture/navigation/CommonScreenNavigation.kt @@ -24,7 +24,7 @@ fun NavGraphBuilder.allItemsScreen( navigateToAddOrEditNote: () -> Unit, navigateToSinglePicker: () -> Unit, navigateToAllItemsSort: () -> Unit, - navigateToAddByIdentifier: () -> Unit, + navigateToAddByIdentifier: (addByIdentifierParams: String) -> Unit, navigateToVideoPlayerScreen: () -> Unit, navigateToImageViewerScreen: () -> Unit, navigateToZoterWebViewScreen: (String) -> Unit, diff --git a/app/src/main/java/org/zotero/android/architecture/navigation/NavigationDialogs.kt b/app/src/main/java/org/zotero/android/architecture/navigation/NavigationDialogs.kt index 73ca2b8f..bc9e4ce7 100644 --- a/app/src/main/java/org/zotero/android/architecture/navigation/NavigationDialogs.kt +++ b/app/src/main/java/org/zotero/android/architecture/navigation/NavigationDialogs.kt @@ -10,6 +10,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.unit.dp import androidx.compose.ui.window.DialogProperties +import androidx.navigation.NamedNavArgument import androidx.navigation.NavGraphBuilder import androidx.navigation.compose.dialog import org.zotero.android.uicomponents.theme.CustomTheme @@ -28,10 +29,12 @@ fun NavGraphBuilder.dialogFixedDimens( fun NavGraphBuilder.dialogFixedMaxHeight( route: String, + arguments: List = emptyList(), content: @Composable () -> Unit, ) { customDialog( route = route, + arguments = arguments, dialogModifier = Modifier.requiredHeightIn(max = 400.dp), content = content ) @@ -51,10 +54,12 @@ fun NavGraphBuilder.dialogDynamicHeight( private fun NavGraphBuilder.customDialog( route: String, dialogModifier: Modifier, + arguments: List = emptyList(), content: @Composable () -> Unit, ) { dialog( route = route, + arguments = arguments, dialogProperties = DialogProperties( dismissOnBackPress = true, dismissOnClickOutside = true, diff --git a/app/src/main/java/org/zotero/android/architecture/navigation/phone/DashboardRootPhoneNavigation.kt b/app/src/main/java/org/zotero/android/architecture/navigation/phone/DashboardRootPhoneNavigation.kt index ae06c433..a1ad3296 100644 --- a/app/src/main/java/org/zotero/android/architecture/navigation/phone/DashboardRootPhoneNavigation.kt +++ b/app/src/main/java/org/zotero/android/architecture/navigation/phone/DashboardRootPhoneNavigation.kt @@ -13,6 +13,8 @@ import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.navigation.NavHostController +import androidx.navigation.NavType +import androidx.navigation.navArgument import com.google.accompanist.insets.navigationBarsPadding import com.google.accompanist.navigation.animation.composable import com.google.accompanist.navigation.animation.rememberAnimatedNavController @@ -56,6 +58,8 @@ import org.zotero.android.uicomponents.singlepicker.SinglePickerScreen import org.zotero.android.uicomponents.theme.CustomTheme import java.io.File +internal const val ARG_ADD_BY_IDENTIFIER = "addByIdentifierArg" + @Composable internal fun DashboardRootPhoneNavigation( onPickFile: (callPoint: EventBusConstants.FileWasSelected.CallPoint) -> Unit, @@ -172,8 +176,10 @@ internal fun DashboardRootPhoneNavigation( } composable( - route = DashboardRootPhoneDestinations.ADD_BY_IDENTIFIER, - arguments = listOf(), + route = "${DashboardRootPhoneDestinations.ADD_BY_IDENTIFIER}/{$ARG_ADD_BY_IDENTIFIER}", + arguments = listOf( + navArgument(ARG_ADD_BY_IDENTIFIER) { type = NavType.StringType }, + ), ) { AddByIdentifierScreen( onClose = navigation::onBack, @@ -235,8 +241,8 @@ private fun ZoteroNavigation.toSinglePicker() { navController.navigate(DashboardRootPhoneDestinations.SINGLE_PICKER) } -private fun ZoteroNavigation.toAddByIdentifier() { - navController.navigate(DashboardRootPhoneDestinations.ADD_BY_IDENTIFIER) +private fun ZoteroNavigation.toAddByIdentifier(params: String) { + navController.navigate("${DashboardRootPhoneDestinations.ADD_BY_IDENTIFIER}/$params") } private fun ZoteroNavigation.toCollectionPicker() { diff --git a/app/src/main/java/org/zotero/android/architecture/navigation/tablet/TabletRightPaneNavigation.kt b/app/src/main/java/org/zotero/android/architecture/navigation/tablet/TabletRightPaneNavigation.kt index 2e1ca824..01068430 100644 --- a/app/src/main/java/org/zotero/android/architecture/navigation/tablet/TabletRightPaneNavigation.kt +++ b/app/src/main/java/org/zotero/android/architecture/navigation/tablet/TabletRightPaneNavigation.kt @@ -5,6 +5,8 @@ import android.net.Uri import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.navigation.NavHostController +import androidx.navigation.NavType +import androidx.navigation.navArgument import com.google.accompanist.insets.navigationBarsPadding import org.zotero.android.architecture.EventBusConstants.FileWasSelected.CallPoint import org.zotero.android.architecture.navigation.CommonScreenDestinations @@ -15,6 +17,7 @@ import org.zotero.android.architecture.navigation.dialogFixedMaxHeight import org.zotero.android.architecture.navigation.imageViewerScreen import org.zotero.android.architecture.navigation.itemDetailsScreen import org.zotero.android.architecture.navigation.loadingScreen +import org.zotero.android.architecture.navigation.phone.ARG_ADD_BY_IDENTIFIER import org.zotero.android.architecture.navigation.toImageViewerScreen import org.zotero.android.architecture.navigation.toItemDetails import org.zotero.android.architecture.navigation.toVideoPlayerScreen @@ -110,7 +113,10 @@ internal fun TabletRightPaneNavigation( } dialogFixedMaxHeight( - route = TabletRightPaneDestinations.ADD_BY_IDENTIFIER_DIALOG, + route = "${TabletRightPaneDestinations.ADD_BY_IDENTIFIER_DIALOG}/{$ARG_ADD_BY_IDENTIFIER}", + arguments = listOf( + navArgument(ARG_ADD_BY_IDENTIFIER) { type = NavType.StringType }, + ), ) { AddByIdentifierScreen( onClose = { @@ -147,8 +153,8 @@ private fun ZoteroNavigation.toSinglePickerDialog() { navController.navigate(TabletRightPaneDestinations.SINGLE_PICKER_DIALOG) } -private fun ZoteroNavigation.toAddByIdentifierDialog() { - navController.navigate(TabletRightPaneDestinations.ADD_BY_IDENTIFIER_DIALOG) +private fun ZoteroNavigation.toAddByIdentifierDialog(params: String) { + navController.navigate("${TabletRightPaneDestinations.ADD_BY_IDENTIFIER_DIALOG}/$params") } private fun ZoteroNavigation.toCollectionPickerDialog() { diff --git a/app/src/main/java/org/zotero/android/screens/allitems/AllItemsScreen.kt b/app/src/main/java/org/zotero/android/screens/allitems/AllItemsScreen.kt index 7e4e4401..8480fa69 100644 --- a/app/src/main/java/org/zotero/android/screens/allitems/AllItemsScreen.kt +++ b/app/src/main/java/org/zotero/android/screens/allitems/AllItemsScreen.kt @@ -38,7 +38,7 @@ internal fun AllItemsScreen( onOpenWebpage: (uri: Uri) -> Unit, navigateToCollectionsScreen: () -> Unit, navigateToSinglePicker: () -> Unit, - navigateToAddByIdentifier: () -> Unit, + navigateToAddByIdentifier: (addByIdentifierParams: String) -> Unit, navigateToAllItemsSort: () -> Unit, navigateToCollectionPicker: () -> Unit, navigateToItemDetails: () -> Unit, @@ -72,8 +72,8 @@ internal fun AllItemsScreen( navigateToSinglePicker() } - AllItemsViewEffect.ShowAddByIdentifierEffect -> { - navigateToAddByIdentifier() + is AllItemsViewEffect.ShowAddByIdentifierEffect -> { + navigateToAddByIdentifier(consumedEffect.params) } AllItemsViewEffect.ShowSortPickerEffect -> { diff --git a/app/src/main/java/org/zotero/android/screens/allitems/AllItemsViewModel.kt b/app/src/main/java/org/zotero/android/screens/allitems/AllItemsViewModel.kt index 5a10d1ca..f0f861dd 100644 --- a/app/src/main/java/org/zotero/android/screens/allitems/AllItemsViewModel.kt +++ b/app/src/main/java/org/zotero/android/screens/allitems/AllItemsViewModel.kt @@ -1043,9 +1043,10 @@ internal class AllItemsViewModel @Inject constructor( } fun onAddByIdentifier() { - ScreenArguments.addByIdentifierPickerArgs = + val addByIdentifierPickerArgs = AddByIdentifierPickerArgs(restoreLookupState = false) - triggerEffect(ShowAddByIdentifierEffect) + val params = navigationParamsMarshaller.encodeObjectToBase64(addByIdentifierPickerArgs) + triggerEffect(ShowAddByIdentifierEffect(params)) } fun dismissDownloadedFilesPopup() { @@ -1135,7 +1136,7 @@ internal sealed class AllItemsViewEffect : ViewEffect { object ShowItemDetailEffect: AllItemsViewEffect() object ShowAddOrEditNoteEffect: AllItemsViewEffect() object ShowItemTypePickerEffect : AllItemsViewEffect() - object ShowAddByIdentifierEffect : AllItemsViewEffect() + data class ShowAddByIdentifierEffect(val params: String) : AllItemsViewEffect() object ShowSortPickerEffect : AllItemsViewEffect() object ShowCollectionPickerEffect: AllItemsViewEffect() object ShowFilterEffect : AllItemsViewEffect() diff --git a/app/src/main/java/org/zotero/android/uicomponents/addbyidentifier/AddByIdentifierViewModel.kt b/app/src/main/java/org/zotero/android/uicomponents/addbyidentifier/AddByIdentifierViewModel.kt index 92a6bcfd..5959057a 100644 --- a/app/src/main/java/org/zotero/android/uicomponents/addbyidentifier/AddByIdentifierViewModel.kt +++ b/app/src/main/java/org/zotero/android/uicomponents/addbyidentifier/AddByIdentifierViewModel.kt @@ -1,6 +1,7 @@ package org.zotero.android.uicomponents.addbyidentifier import android.content.Context +import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope import com.google.mlkit.vision.codescanner.GmsBarcodeScanning import dagger.hilt.android.lifecycle.HiltViewModel @@ -9,15 +10,18 @@ import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import org.zotero.android.api.pojo.sync.KeyBaseKeyPair import org.zotero.android.architecture.BaseViewModel2 -import org.zotero.android.architecture.ScreenArguments import org.zotero.android.architecture.ViewEffect import org.zotero.android.architecture.ViewState +import org.zotero.android.architecture.navigation.NavigationParamsMarshaller +import org.zotero.android.architecture.navigation.phone.ARG_ADD_BY_IDENTIFIER +import org.zotero.android.architecture.require import org.zotero.android.attachmentdownloader.RemoteAttachmentDownloader import org.zotero.android.attachmentdownloader.RemoteAttachmentDownloaderEventStream import org.zotero.android.database.objects.FieldKeys import org.zotero.android.files.FileStore import org.zotero.android.sync.LibraryIdentifier import org.zotero.android.sync.SchemaController +import org.zotero.android.uicomponents.addbyidentifier.data.AddByIdentifierPickerArgs import org.zotero.android.uicomponents.addbyidentifier.data.ISBNParser import org.zotero.android.uicomponents.addbyidentifier.data.LookupRow import org.zotero.android.uicomponents.addbyidentifier.data.LookupRowItem @@ -32,8 +36,15 @@ internal class AddByIdentifierViewModel @Inject constructor( private val schemaController: SchemaController, private val remoteFileDownloader: RemoteAttachmentDownloader, private val context: Context, + stateHandle: SavedStateHandle, + private val navigationParamsMarshaller: NavigationParamsMarshaller, ) : BaseViewModel2(AddByIdentifierViewState()) { + private val screenArgs: AddByIdentifierPickerArgs by lazy { + val argsEncoded = stateHandle.get(ARG_ADD_BY_IDENTIFIER).require() + navigationParamsMarshaller.decodeObjectFromBase64(argsEncoded) + } + private val scannerPatternRegex = "10.\\d{4,9}\\/[-._;()\\/:a-zA-Z0-9]+" @@ -42,7 +53,7 @@ internal class AddByIdentifierViewModel @Inject constructor( val collectionKeys = fileStore.getSelectedCollectionId().keyGet?.let { setOf(it) } ?: emptySet() val libraryId = fileStore.getSelectedLibrary() - val restoreLookupState = ScreenArguments.addByIdentifierPickerArgs.restoreLookupState + val restoreLookupState = screenArgs.restoreLookupState initState( restoreLookupState = restoreLookupState, hasDarkBackground = false, diff --git a/buildSrc/src/main/kotlin/BuildConfig.kt b/buildSrc/src/main/kotlin/BuildConfig.kt index 9f93bdbe..aa61ab36 100644 --- a/buildSrc/src/main/kotlin/BuildConfig.kt +++ b/buildSrc/src/main/kotlin/BuildConfig.kt @@ -4,7 +4,7 @@ object BuildConfig { const val compileSdkVersion = 34 const val targetSdk = 33 - val versionCode = 61 // Must be updated on every build + val versionCode = 62 // Must be updated on every build val version = Version( major = 1, minor = 0,