From c0e90094ff9b1f065596aba04277f2781f68b203 Mon Sep 17 00:00:00 2001 From: Kevin Boulongne Date: Tue, 17 Dec 2024 11:44:08 +0100 Subject: [PATCH 1/4] feat: When cancelling the uploading transfer, we're going back to ImportFiles instead of closing Activity --- .../screen/newtransfer/NewTransferNavHost.kt | 2 +- .../importfiles/ImportFilesScreen.kt | 5 ++++- .../upload/UploadProgressScreen.kt | 21 ++++++++++++------- .../swisstransfer/workers/UploadWorker.kt | 6 +++++- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt index 1929e0861..3541112c6 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt @@ -53,7 +53,7 @@ fun NewTransferNavHost(navController: NavHostController, closeActivity: () -> Un navController.navigate(UploadSuccessDestination(args.transferType, transferUrl)) }, navigateToUploadError = { navController.navigate(UploadErrorDestination) }, - closeActivity = closeActivity, + navigateBackToImportFiles = { navController.popBackStack() }, ) } composable { diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt index 4178ce1ce..c453247ef 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt @@ -161,7 +161,10 @@ private fun HandleSendActionResult( val errorMessage = stringResource(R.string.errorUnknown) LaunchedEffect(getSendActionResult()) { when (val actionResult = getSendActionResult()) { - is SendActionResult.Success -> navigateToUploadProgress(transferType(), actionResult.totalSize) + is SendActionResult.Success -> { + resetSendActionResult() + navigateToUploadProgress(transferType(), actionResult.totalSize) + } is SendActionResult.Failure -> { snackbarHostState.showSnackbar(errorMessage) resetSendActionResult() diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/upload/UploadProgressScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/upload/UploadProgressScreen.kt index 5c6a292db..69ad7570d 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/upload/UploadProgressScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/upload/UploadProgressScreen.kt @@ -49,7 +49,7 @@ fun UploadProgressScreen( totalSizeInBytes: Long, navigateToUploadSuccess: (String) -> Unit, navigateToUploadError: () -> Unit, - closeActivity: () -> Unit, + navigateBackToImportFiles: () -> Unit, uploadProgressViewModel: UploadProgressViewModel = hiltViewModel(), ) { val uiState by uploadProgressViewModel.transferProgressUiState.collectAsStateWithLifecycle() @@ -66,7 +66,7 @@ fun UploadProgressScreen( uploadProgressViewModel.trackUploadProgress() } - HandleProgressState({ uiState }, navigateToUploadSuccess, navigateToUploadError) + HandleProgressState({ uiState }, navigateToUploadSuccess, navigateToUploadError, navigateBackToImportFiles) UploadProgressScreen( progressState = { uiState }, @@ -74,10 +74,7 @@ fun UploadProgressScreen( totalSizeInBytes = totalSizeInBytes, showBottomSheet = GetSetCallbacks(get = { showBottomSheet }, set = { showBottomSheet = it }), adScreenType = adScreenType, - onCancel = { - uploadProgressViewModel.cancelUpload() - closeActivity() - } + onCancel = { uploadProgressViewModel.cancelUpload() } ) } @@ -86,12 +83,14 @@ private fun HandleProgressState( uiState: () -> UploadProgressUiState, navigateToUploadSuccess: (String) -> Unit, navigateToUploadError: () -> Unit, + navigateBackToImportFiles: () -> Unit, ) { val currentUiState = uiState() LaunchedEffect(uiState()) { when (currentUiState) { is UploadProgressUiState.Success -> navigateToUploadSuccess(currentUiState.transferUrl) is UploadProgressUiState.Error -> navigateToUploadError() + is UploadProgressUiState.Cancel -> navigateBackToImportFiles() else -> Unit } } @@ -129,7 +128,15 @@ private fun UploadProgressScreen( Spacer(Modifier.height(Margin.Huge)) } - if (showBottomSheet.get()) CancelUploadBottomSheet(onCancel = onCancel, closeButtonSheet = { showBottomSheet.set(false) }) + if (showBottomSheet.get()) { + CancelUploadBottomSheet( + onCancel = { + onCancel() + showBottomSheet.set(false) + }, + closeButtonSheet = { showBottomSheet.set(false) }, + ) + } } } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/workers/UploadWorker.kt b/app/src/main/java/com/infomaniak/swisstransfer/workers/UploadWorker.kt index db0085152..c25636180 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/workers/UploadWorker.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/workers/UploadWorker.kt @@ -124,7 +124,8 @@ class UploadWorker @AssistedInject constructor( return@mapLatest when (workInfo.state) { State.RUNNING -> UploadProgressUiState.Progress(workInfo.progress).also { lastUploadedSize = it.uploadedSize } State.SUCCEEDED -> UploadProgressUiState.Success.create(workInfo.outputData, sharedApiUrlCreator) - State.FAILED, State.CANCELLED -> UploadProgressUiState.Error(lastUploadedSize) + State.FAILED -> UploadProgressUiState.Error(lastUploadedSize) + State.CANCELLED -> UploadProgressUiState.Cancel() else -> UploadProgressUiState.Default(lastUploadedSize) } ?: UploadProgressUiState.Error(lastUploadedSize) }.filterNotNull() @@ -160,6 +161,9 @@ class UploadWorker @AssistedInject constructor( @Immutable data class Error(override val uploadedSize: Long = 0) : UploadProgressUiState(uploadedSize) + + @Immutable + data class Cancel(override val uploadedSize: Long = 0) : UploadProgressUiState(uploadedSize) } companion object { From 5f3557cfe740e923566918f03d2295ca166b1f53 Mon Sep 17 00:00:00 2001 From: Kevin Boulongne Date: Tue, 17 Dec 2024 11:44:17 +0100 Subject: [PATCH 2/4] docs: Add explanatory comment --- .../ui/screen/newtransfer/importfiles/ImportFilesScreen.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt index c453247ef..014b9fab7 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt @@ -162,6 +162,10 @@ private fun HandleSendActionResult( LaunchedEffect(getSendActionResult()) { when (val actionResult = getSendActionResult()) { is SendActionResult.Success -> { + // If the user cancels the transfer while in UploadProgress, we're gonna popBackStack to ImportFiles. + // If we don't reset the ImportFiles state machine, we'll automatically navigate-back to UploadProgress again. + // So, before leaving ImportFiles to go to UploadProgress, we need to reset the ImportFiles state machine. + // TODO: Maybe merging the 2 screens state machines into 1 could help simplify this ? resetSendActionResult() navigateToUploadProgress(transferType(), actionResult.totalSize) } From caa855daf40881be772e18176f2102694d48d92f Mon Sep 17 00:00:00 2001 From: Kevin Boulongne Date: Tue, 17 Dec 2024 13:01:35 +0100 Subject: [PATCH 3/4] refactor: Int are now Long, as they should be --- .../com/infomaniak/swisstransfer/workers/UploadWorker.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/workers/UploadWorker.kt b/app/src/main/java/com/infomaniak/swisstransfer/workers/UploadWorker.kt index c25636180..81444e5b2 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/workers/UploadWorker.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/workers/UploadWorker.kt @@ -139,7 +139,7 @@ class UploadWorker @AssistedInject constructor( sealed class UploadProgressUiState(open val uploadedSize: Long) { @Immutable - data class Default(override val uploadedSize: Long = 0) : UploadProgressUiState(uploadedSize) + data class Default(override val uploadedSize: Long = 0L) : UploadProgressUiState(uploadedSize) @Immutable data class Progress(override val uploadedSize: Long) : UploadProgressUiState(uploadedSize) { @@ -160,10 +160,10 @@ class UploadWorker @AssistedInject constructor( } @Immutable - data class Error(override val uploadedSize: Long = 0) : UploadProgressUiState(uploadedSize) + data class Error(override val uploadedSize: Long = 0L) : UploadProgressUiState(uploadedSize) @Immutable - data class Cancel(override val uploadedSize: Long = 0) : UploadProgressUiState(uploadedSize) + data class Cancel(override val uploadedSize: Long = 0L) : UploadProgressUiState(uploadedSize) } companion object { From 37e8fc201e4388f9a56985701712520110c5c0bb Mon Sep 17 00:00:00 2001 From: Kevin Boulongne Date: Wed, 18 Dec 2024 10:17:33 +0100 Subject: [PATCH 4/4] refactor: Use `popBackStack` with correct route instead of just popping back blindly --- .../swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt index 3541112c6..8a25375cb 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferNavHost.kt @@ -53,7 +53,7 @@ fun NewTransferNavHost(navController: NavHostController, closeActivity: () -> Un navController.navigate(UploadSuccessDestination(args.transferType, transferUrl)) }, navigateToUploadError = { navController.navigate(UploadErrorDestination) }, - navigateBackToImportFiles = { navController.popBackStack() }, + navigateBackToImportFiles = { navController.popBackStack(route = ImportFilesDestination, inclusive = false) }, ) } composable {