From 1aa4a71e64cbd3e2f4178cca05a6e71d6f74b3e2 Mon Sep 17 00:00:00 2001 From: Kevin Boulongne Date: Fri, 6 Dec 2024 07:19:23 +0100 Subject: [PATCH 1/4] refactor: Factorize use of horizontal padding --- .../importfiles/ImportFilesScreen.kt | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) 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 c8372578b..daebfb944 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 @@ -50,6 +50,8 @@ import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme import com.infomaniak.swisstransfer.ui.utils.GetSetCallbacks import com.infomaniak.swisstransfer.ui.utils.PreviewAllWindows +private val HORIZONTAL_MARGIN = Margin.Medium + @Composable fun ImportFilesScreen( importFilesViewModel: ImportFilesViewModel = hiltViewModel(), @@ -156,15 +158,21 @@ private fun ImportFilesScreen( }, content = { Column(modifier = Modifier.verticalScroll(rememberScrollState())) { - FilesToImport(files, removeFileByUid, addFiles, shouldStartByPromptingUserForFiles) + FilesToImport( + modifier = Modifier.padding(horizontal = HORIZONTAL_MARGIN), + files = files, + removeFileByUid = removeFileByUid, + addFiles = addFiles, + shouldStartByPromptingUserForFiles = shouldStartByPromptingUserForFiles, + ) Spacer(Modifier.height(Margin.Medium)) ImportTextFields( - modifier = Modifier.padding(horizontal = Margin.Medium), + modifier = Modifier.padding(horizontal = HORIZONTAL_MARGIN), transferMessage = transferMessage, selectedTransferType = selectedTransferType.get, ) SendByOptions(selectedTransferType) - TransferOptions(transferOptionsCallbacks) + TransferOptions(Modifier.padding(horizontal = HORIZONTAL_MARGIN), transferOptionsCallbacks) } } ) @@ -172,6 +180,7 @@ private fun ImportFilesScreen( @Composable private fun FilesToImport( + modifier: Modifier, files: () -> List, removeFileByUid: (uid: String) -> Unit, addFiles: (List) -> Unit, @@ -191,31 +200,24 @@ private fun FilesToImport( LaunchedEffect(Unit) { if (shouldShowInitialFilePick) pickFiles() } - ImportFilesTitle(modifier = Modifier.padding(horizontal = Margin.Medium), titleRes = R.string.myFilesTitle) - ImportedFilesCard( - modifier = Modifier.padding(horizontal = Margin.Medium), - files = files, - pickFiles = ::pickFiles, - removeFileByUid = removeFileByUid, - ) + ImportFilesTitle(modifier, R.string.myFilesTitle) + ImportedFilesCard(modifier, files, ::pickFiles, removeFileByUid) } @Composable -private fun ImportTextFields( +private fun ColumnScope.ImportTextFields( modifier: Modifier, transferMessage: GetSetCallbacks, selectedTransferType: () -> TransferTypeUi, ) { - Column(modifier) { - EmailAddressesTextFields(Modifier.fillMaxWidth(), selectedTransferType) - SwissTransferTextField( - modifier = Modifier.fillMaxWidth(), - label = stringResource(R.string.transferMessagePlaceholder), - isRequired = false, - minLineNumber = 3, - onValueChange = transferMessage.set, - ) - } + EmailAddressesTextFields(modifier.fillMaxWidth(), selectedTransferType) + SwissTransferTextField( + modifier = modifier.fillMaxWidth(), + label = stringResource(R.string.transferMessagePlaceholder), + isRequired = false, + minLineNumber = 3, + onValueChange = transferMessage.set, + ) } @Composable @@ -243,12 +245,12 @@ private fun ColumnScope.EmailAddressesTextFields(modifier: Modifier, selectedTra @Composable private fun SendByOptions(selectedTransferType: GetSetCallbacks) { - ImportFilesTitle(Modifier.padding(horizontal = Margin.Medium), titleRes = R.string.transferTypeTitle) + ImportFilesTitle(Modifier.padding(horizontal = HORIZONTAL_MARGIN), R.string.transferTypeTitle) TransferTypeButtons(selectedTransferType) } @Composable -private fun TransferOptions(transferOptionsCallbacks: TransferOptionsCallbacks) { +private fun TransferOptions(modifier: Modifier, transferOptionsCallbacks: TransferOptionsCallbacks) { var showTransferOption by rememberSaveable { mutableStateOf(null) } @@ -256,9 +258,9 @@ private fun TransferOptions(transferOptionsCallbacks: TransferOptionsCallbacks) showTransferOption = null } - ImportFilesTitle(Modifier.padding(horizontal = Margin.Medium), titleRes = R.string.advancedSettingsTitle) + ImportFilesTitle(modifier, R.string.advancedSettingsTitle) TransferOptionsTypes( - modifier = Modifier.padding(horizontal = Margin.Medium), + modifier = modifier, transferOptionsStates = transferOptionsCallbacks.transferOptionsStates, onClick = { selectedOptionType -> showTransferOption = selectedOptionType }, ) From 9cc7f91a83aba06ff1871488a6e2240ff8867ca1 Mon Sep 17 00:00:00 2001 From: Kevin Boulongne Date: Fri, 6 Dec 2024 08:42:51 +0100 Subject: [PATCH 2/4] refactor: Create a modifier to factorize usage --- .../importfiles/ImportFilesScreen.kt | 27 +++++++------------ .../components/TransferTypeButtons.kt | 15 +++++++---- 2 files changed, 19 insertions(+), 23 deletions(-) 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 daebfb944..05cbdbe88 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 @@ -50,7 +50,7 @@ import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme import com.infomaniak.swisstransfer.ui.utils.GetSetCallbacks import com.infomaniak.swisstransfer.ui.utils.PreviewAllWindows -private val HORIZONTAL_MARGIN = Margin.Medium +private val HORIZONTAL_PADDING = Margin.Medium @Composable fun ImportFilesScreen( @@ -158,21 +158,12 @@ private fun ImportFilesScreen( }, content = { Column(modifier = Modifier.verticalScroll(rememberScrollState())) { - FilesToImport( - modifier = Modifier.padding(horizontal = HORIZONTAL_MARGIN), - files = files, - removeFileByUid = removeFileByUid, - addFiles = addFiles, - shouldStartByPromptingUserForFiles = shouldStartByPromptingUserForFiles, - ) + val modifier = Modifier.padding(horizontal = HORIZONTAL_PADDING) + FilesToImport(modifier, files, removeFileByUid, addFiles, shouldStartByPromptingUserForFiles) Spacer(Modifier.height(Margin.Medium)) - ImportTextFields( - modifier = Modifier.padding(horizontal = HORIZONTAL_MARGIN), - transferMessage = transferMessage, - selectedTransferType = selectedTransferType.get, - ) - SendByOptions(selectedTransferType) - TransferOptions(Modifier.padding(horizontal = HORIZONTAL_MARGIN), transferOptionsCallbacks) + ImportTextFields(modifier, transferMessage, selectedTransferType.get) + SendByOptions(modifier, selectedTransferType) + TransferOptions(modifier, transferOptionsCallbacks) } } ) @@ -244,9 +235,9 @@ private fun ColumnScope.EmailAddressesTextFields(modifier: Modifier, selectedTra } @Composable -private fun SendByOptions(selectedTransferType: GetSetCallbacks) { - ImportFilesTitle(Modifier.padding(horizontal = HORIZONTAL_MARGIN), R.string.transferTypeTitle) - TransferTypeButtons(selectedTransferType) +private fun SendByOptions(modifier: Modifier, selectedTransferType: GetSetCallbacks) { + ImportFilesTitle(modifier, R.string.transferTypeTitle) + TransferTypeButtons(HORIZONTAL_PADDING, selectedTransferType) } @Composable diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/components/TransferTypeButtons.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/components/TransferTypeButtons.kt index fd2d5d665..6c6b867ca 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/components/TransferTypeButtons.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/components/TransferTypeButtons.kt @@ -20,13 +20,15 @@ package com.infomaniak.swisstransfer.ui.screen.newtransfer.importfiles.component import androidx.annotation.PluralsRes import androidx.annotation.StringRes import androidx.compose.foundation.horizontalScroll -import androidx.compose.foundation.layout.* +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.padding import androidx.compose.foundation.rememberScrollState import androidx.compose.material3.Surface import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.vector.ImageVector -import androidx.compose.ui.unit.dp +import androidx.compose.ui.unit.Dp import com.infomaniak.multiplatform_swisstransfer.common.models.TransferType import com.infomaniak.swisstransfer.R import com.infomaniak.swisstransfer.ui.images.AppImages.AppIcons @@ -40,11 +42,11 @@ import com.infomaniak.swisstransfer.ui.utils.GetSetCallbacks import com.infomaniak.swisstransfer.ui.utils.PreviewLightAndDark @Composable -fun TransferTypeButtons(transferType: GetSetCallbacks) { +fun TransferTypeButtons(horizontalPadding: Dp, transferType: GetSetCallbacks) { Row( modifier = Modifier .horizontalScroll(rememberScrollState()) - .padding(horizontal = Margin.Medium), + .padding(horizontal = horizontalPadding), horizontalArrangement = Arrangement.spacedBy(Margin.Mini), ) { for (transferTypeEntry in TransferTypeUi.entries) { @@ -108,7 +110,10 @@ enum class TransferTypeUi( private fun TransferTypeButtonsPreview() { SwissTransferTheme { Surface { - TransferTypeButtons(GetSetCallbacks(get = { TransferTypeUi.QR_CODE }, set = {})) + TransferTypeButtons( + horizontalPadding = Margin.Medium, + transferType = GetSetCallbacks(get = { TransferTypeUi.QR_CODE }, set = {}), + ) } } } From 901d45860f9796a3477099111f80a7c35f08f92f Mon Sep 17 00:00:00 2001 From: Kevin Boulongne Date: Fri, 6 Dec 2024 07:44:29 +0100 Subject: [PATCH 3/4] refactor: Sort `SendButton` parameters --- .../ui/screen/newtransfer/importfiles/ImportFilesScreen.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 05cbdbe88..55b23b723 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 @@ -154,7 +154,7 @@ private fun ImportFilesScreen( ) }, topButton = { modifier -> - SendButton(filesToImportCount, currentSessionFilesCount, files, modifier, sendTransfer) + SendButton(modifier, filesToImportCount, currentSessionFilesCount, files, sendTransfer) }, content = { Column(modifier = Modifier.verticalScroll(rememberScrollState())) { @@ -304,10 +304,10 @@ private fun ImportFilesTitle(modifier: Modifier = Modifier, @StringRes titleRes: @Composable private fun SendButton( + modifier: Modifier, filesToImportCount: () -> Int, currentSessionFilesCount: () -> Int, importedFiles: () -> List, - modifier: Modifier, navigateToUploadProgress: () -> Unit, ) { val remainingFilesCount = filesToImportCount() From d026b3dfa3237fe0afc55a2bcd141c7554e0605d Mon Sep 17 00:00:00 2001 From: Kevin Boulongne Date: Fri, 6 Dec 2024 08:49:24 +0100 Subject: [PATCH 4/4] refactor: Add another factorizing Modifier in `ImportTextFields` --- .../ui/screen/newtransfer/importfiles/ImportFilesScreen.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 55b23b723..0322b5fa2 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 @@ -197,13 +197,14 @@ private fun FilesToImport( @Composable private fun ColumnScope.ImportTextFields( - modifier: Modifier, + horizontalPaddingModifier: Modifier, transferMessage: GetSetCallbacks, selectedTransferType: () -> TransferTypeUi, ) { - EmailAddressesTextFields(modifier.fillMaxWidth(), selectedTransferType) + val modifier = horizontalPaddingModifier.fillMaxWidth() + EmailAddressesTextFields(modifier, selectedTransferType) SwissTransferTextField( - modifier = modifier.fillMaxWidth(), + modifier = modifier, label = stringResource(R.string.transferMessagePlaceholder), isRequired = false, minLineNumber = 3,