From 5c71ce7b22b58a30b64679d4fe2a36140eb2af72 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 19 Sep 2024 07:53:46 +0200 Subject: [PATCH 1/5] Fix setting option text style --- .../ui/screen/main/settings/components/SingleSelectOptions.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SingleSelectOptions.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SingleSelectOptions.kt index 27548b433..8edd58a18 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SingleSelectOptions.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SingleSelectOptions.kt @@ -71,7 +71,7 @@ private fun SettingOptionItem(item: SettingOption, isSelected: Boolean, onClick: Spacer(modifier = Modifier.width(Margin.Medium)) } - Text(text = item.title(), Modifier.weight(1.0f)) + Text(text = item.title(), Modifier.weight(1.0f), style = SwissTransferTheme.typography.bodyRegular) if (isSelected) Spacer(modifier = Modifier.width(Margin.Medium)) AnimatedVisibility( From ff1a73087151b7f3110064e8e409a3b44dd1074b Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 19 Sep 2024 08:51:23 +0200 Subject: [PATCH 2/5] Fix option title paddings and simplify option screen definition with scaffold --- .../settings/SettingsDownloadsLimitScreen.kt | 42 +++---------- .../settings/SettingsEmailLanguageScreen.kt | 42 +++---------- .../main/settings/SettingsThemeScreen.kt | 42 +++---------- .../settings/SettingsValidityPeriodScreen.kt | 42 +++---------- .../settings/components/OptionScaffold.kt | 62 +++++++++++++++++++ .../main/settings/components/OptionTitle.kt | 49 +++++++++++++++ .../main/settings/components/SettingTitle.kt | 10 +-- .../main/settings/components/UnpaddedTitle.kt | 34 ++++++++++ 8 files changed, 184 insertions(+), 139 deletions(-) create mode 100644 app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionScaffold.kt create mode 100644 app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionTitle.kt create mode 100644 app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/UnpaddedTitle.kt diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt index a4b0d71a1..f2f798190 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt @@ -17,26 +17,13 @@ */ package com.infomaniak.swisstransfer.ui.screen.main.settings -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.rememberScrollState -import androidx.compose.foundation.verticalScroll -import androidx.compose.material3.Scaffold import androidx.compose.material3.Surface import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf -import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.runtime.setValue -import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.vector.ImageVector import com.infomaniak.multiplatform_swisstransfer.common.models.DownloadLimit import com.infomaniak.swisstransfer.R -import com.infomaniak.swisstransfer.ui.components.SwissTransferTobAppBar -import com.infomaniak.swisstransfer.ui.components.TopAppBarButton +import com.infomaniak.swisstransfer.ui.screen.main.settings.components.OptionScaffold import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SettingOption -import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SettingTitle -import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SingleSelectOptions import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme import com.infomaniak.swisstransfer.ui.utils.PreviewMobile @@ -46,25 +33,14 @@ fun SettingsDownloadsLimitScreen( navigateBack: (() -> Unit)?, onDownloadLimitChange: (DownloadLimit) -> Unit, ) { - Scaffold(topBar = { - val canDisplayBackButton = navigateBack?.let { TopAppBarButton.backButton(navigateBack) } - SwissTransferTobAppBar(R.string.settingsOptionDownloadLimit, navigationMenu = canDisplayBackButton) - }) { paddingsValue -> - Column( - modifier = Modifier - .verticalScroll(rememberScrollState()) - .padding(paddingsValue), - ) { - SettingTitle(titleRes = R.string.settingsDownloadsLimitTitle) - - var selectedItem by rememberSaveable { mutableIntStateOf(downloadLimit.ordinal) } - SingleSelectOptions(DownloadLimitOption.entries, { selectedItem }, { position -> - selectedItem = position - val selectedDownloadLimit = DownloadLimit.entries[position] - onDownloadLimitChange(selectedDownloadLimit) - }) - } - } + OptionScaffold( + R.string.settingsOptionDownloadLimit, + R.string.settingsDownloadsLimitTitle, + DownloadLimitOption.entries, + { downloadLimit.ordinal }, + { position -> onDownloadLimitChange(DownloadLimit.entries[position]) }, + navigateBack + ) } enum class DownloadLimitOption( diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt index 3aa88b978..999be82ba 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt @@ -17,27 +17,14 @@ */ package com.infomaniak.swisstransfer.ui.screen.main.settings -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.rememberScrollState -import androidx.compose.foundation.verticalScroll -import androidx.compose.material3.Scaffold import androidx.compose.material3.Surface import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf -import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.runtime.setValue -import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.stringResource import com.infomaniak.multiplatform_swisstransfer.common.models.EmailLanguage import com.infomaniak.swisstransfer.R -import com.infomaniak.swisstransfer.ui.components.SwissTransferTobAppBar -import com.infomaniak.swisstransfer.ui.components.TopAppBarButton +import com.infomaniak.swisstransfer.ui.screen.main.settings.components.OptionScaffold import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SettingOption -import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SettingTitle -import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SingleSelectOptions import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme import com.infomaniak.swisstransfer.ui.utils.PreviewMobile @@ -47,25 +34,14 @@ fun SettingsEmailLanguageScreen( navigateBack: (() -> Unit)?, onEmailLanguageChange: (EmailLanguage) -> Unit, ) { - Scaffold(topBar = { - val canDisplayBackButton = navigateBack?.let { TopAppBarButton.backButton(navigateBack) } - SwissTransferTobAppBar(R.string.settingsOptionEmailLanguage, navigationMenu = canDisplayBackButton) - }) { paddingsValue -> - Column( - modifier = Modifier - .verticalScroll(rememberScrollState()) - .padding(paddingsValue), - ) { - SettingTitle(titleRes = R.string.settingsEmailLanguageTitle) - - var selectedItem by rememberSaveable { mutableIntStateOf(emailLanguage.ordinal) } - SingleSelectOptions(EmailLanguageOption.entries, { selectedItem }, { position -> - selectedItem = position - val selectedEmailLanguage = EmailLanguage.entries[position] - onEmailLanguageChange(selectedEmailLanguage) - }) - } - } + OptionScaffold( + R.string.settingsOptionEmailLanguage, + R.string.settingsEmailLanguageTitle, + EmailLanguageOption.entries, + { emailLanguage.ordinal }, + { position -> onEmailLanguageChange(EmailLanguage.entries[position]) }, + navigateBack + ) } enum class EmailLanguageOption( diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt index e30770521..675c766ad 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt @@ -17,31 +17,18 @@ */ package com.infomaniak.swisstransfer.ui.screen.main.settings -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.rememberScrollState -import androidx.compose.foundation.verticalScroll -import androidx.compose.material3.Scaffold import androidx.compose.material3.Surface import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf -import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.runtime.setValue -import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.stringResource import com.infomaniak.multiplatform_swisstransfer.common.models.Theme import com.infomaniak.swisstransfer.R -import com.infomaniak.swisstransfer.ui.components.SwissTransferTobAppBar -import com.infomaniak.swisstransfer.ui.components.TopAppBarButton import com.infomaniak.swisstransfer.ui.images.AppImages.AppIcons import com.infomaniak.swisstransfer.ui.images.icons.CircleBlack import com.infomaniak.swisstransfer.ui.images.icons.CircleBlackAndWhite import com.infomaniak.swisstransfer.ui.images.icons.CircleWhite +import com.infomaniak.swisstransfer.ui.screen.main.settings.components.OptionScaffold import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SettingOption -import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SettingTitle -import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SingleSelectOptions import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme import com.infomaniak.swisstransfer.ui.utils.PreviewMobile @@ -51,25 +38,14 @@ fun SettingsThemeScreen( navigateBack: (() -> Unit)?, onThemeUpdate: (Theme) -> Unit, ) { - Scaffold(topBar = { - val canDisplayBackButton = navigateBack?.let { TopAppBarButton.backButton(navigateBack) } - SwissTransferTobAppBar(R.string.settingsOptionTheme, navigationMenu = canDisplayBackButton) - }) { paddingsValue -> - Column( - modifier = Modifier - .verticalScroll(rememberScrollState()) - .padding(paddingsValue), - ) { - SettingTitle(titleRes = R.string.settingsThemeTitle) - - var selectedItem by rememberSaveable { mutableIntStateOf(theme.ordinal) } - SingleSelectOptions(ThemeOption.entries, { selectedItem }, { position -> - selectedItem = position - val selectedTheme = Theme.entries[position] - onThemeUpdate(selectedTheme) - }) - } - } + OptionScaffold( + R.string.settingsOptionTheme, + R.string.settingsThemeTitle, + ThemeOption.entries, + { theme.ordinal }, + { position -> onThemeUpdate(Theme.entries[position]) }, + navigateBack + ) } enum class ThemeOption( diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsValidityPeriodScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsValidityPeriodScreen.kt index d49cc307d..6f28ecbc9 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsValidityPeriodScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsValidityPeriodScreen.kt @@ -17,27 +17,14 @@ */ package com.infomaniak.swisstransfer.ui.screen.main.settings -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.rememberScrollState -import androidx.compose.foundation.verticalScroll -import androidx.compose.material3.Scaffold import androidx.compose.material3.Surface import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf -import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.runtime.setValue -import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.pluralStringResource import com.infomaniak.multiplatform_swisstransfer.common.models.ValidityPeriod import com.infomaniak.swisstransfer.R -import com.infomaniak.swisstransfer.ui.components.SwissTransferTobAppBar -import com.infomaniak.swisstransfer.ui.components.TopAppBarButton +import com.infomaniak.swisstransfer.ui.screen.main.settings.components.OptionScaffold import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SettingOption -import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SettingTitle -import com.infomaniak.swisstransfer.ui.screen.main.settings.components.SingleSelectOptions import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme import com.infomaniak.swisstransfer.ui.utils.PreviewMobile @@ -47,25 +34,14 @@ fun SettingsValidityPeriodScreen( navigateBack: (() -> Unit)?, onValidityPeriodChange: (ValidityPeriod) -> Unit, ) { - Scaffold(topBar = { - val canDisplayBackButton = navigateBack?.let { TopAppBarButton.backButton(navigateBack) } - SwissTransferTobAppBar(R.string.settingsOptionValidityPeriod, navigationMenu = canDisplayBackButton) - }) { paddingsValue -> - Column( - modifier = Modifier - .verticalScroll(rememberScrollState()) - .padding(paddingsValue), - ) { - SettingTitle(titleRes = R.string.settingsValidityPeriodTitle) - - var selectedItem by rememberSaveable { mutableIntStateOf(validityPeriod.ordinal) } - SingleSelectOptions(ValidityPeriodOption.entries, { selectedItem }, { position -> - selectedItem = position - val selectedValidityPeriod = ValidityPeriod.entries[position] - onValidityPeriodChange(selectedValidityPeriod) - }) - } - } + OptionScaffold( + R.string.settingsOptionValidityPeriod, + R.string.settingsValidityPeriodTitle, + ValidityPeriodOption.entries, + { validityPeriod.ordinal }, + { position -> onValidityPeriodChange(ValidityPeriod.entries[position]) }, + navigateBack + ) } enum class ValidityPeriodOption( diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionScaffold.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionScaffold.kt new file mode 100644 index 000000000..7a8d25fc3 --- /dev/null +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionScaffold.kt @@ -0,0 +1,62 @@ +/* + * Infomaniak SwissTransfer - Android + * Copyright (C) 2024 Infomaniak Network SA + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package com.infomaniak.swisstransfer.ui.screen.main.settings.components + +import androidx.annotation.StringRes +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.Scaffold +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue +import androidx.compose.ui.Modifier +import com.infomaniak.swisstransfer.ui.components.SwissTransferTobAppBar +import com.infomaniak.swisstransfer.ui.components.TopAppBarButton + +@Composable +fun OptionScaffold( + @StringRes topAppBarTitleRes: Int, + @StringRes optionTitleRes: Int, + enumEntries: List, + getSelectedSettingOptionPosition: () -> Int, + setSelectedSettingOptionPosition: (Int) -> Unit, + navigateBack: (() -> Unit)? = null, +) { + Scaffold(topBar = { + val canDisplayBackButton = navigateBack?.let { TopAppBarButton.backButton(navigateBack) } + SwissTransferTobAppBar(topAppBarTitleRes, navigationMenu = canDisplayBackButton) + }) { paddingsValue -> + Column( + modifier = Modifier + .verticalScroll(rememberScrollState()) + .padding(paddingsValue), + ) { + OptionTitle(titleRes = optionTitleRes) + + var selectedItem by rememberSaveable { mutableIntStateOf(getSelectedSettingOptionPosition()) } + SingleSelectOptions(enumEntries, { selectedItem }, { position -> + selectedItem = position + setSelectedSettingOptionPosition(position) + }) + } + } +} diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionTitle.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionTitle.kt new file mode 100644 index 000000000..99e0bc68f --- /dev/null +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionTitle.kt @@ -0,0 +1,49 @@ +/* + * Infomaniak SwissTransfer - Android + * Copyright (C) 2024 Infomaniak Network SA + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package com.infomaniak.swisstransfer.ui.screen.main.settings.components + +import android.content.res.Configuration +import androidx.annotation.StringRes +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.padding +import androidx.compose.material3.Surface +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.tooling.preview.Preview +import com.infomaniak.swisstransfer.R +import com.infomaniak.swisstransfer.ui.theme.Dimens +import com.infomaniak.swisstransfer.ui.theme.Margin +import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme + +@Composable +fun OptionTitle(@StringRes titleRes: Int) { + UnpaddedTitle(Modifier.padding(horizontal = Dimens.SettingHorizontalMargin, vertical = Margin.Large), titleRes) +} + +@Preview(name = "Light") +@Preview(name = "Dark", uiMode = Configuration.UI_MODE_NIGHT_YES or Configuration.UI_MODE_TYPE_NORMAL) +@Composable +private fun OptionTitlePreview() { + SwissTransferTheme { + Surface { + Box { + OptionTitle(titleRes = R.string.appName) + } + } + } +} diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SettingTitle.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SettingTitle.kt index d792d1ab4..f09b6fd7f 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SettingTitle.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SettingTitle.kt @@ -22,10 +22,8 @@ import androidx.annotation.StringRes import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.padding import androidx.compose.material3.Surface -import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier -import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import com.infomaniak.swisstransfer.R import com.infomaniak.swisstransfer.ui.theme.Dimens @@ -33,11 +31,9 @@ import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme @Composable fun SettingTitle(@StringRes titleRes: Int) { - Text( - modifier = Modifier.padding(horizontal = Dimens.SettingHorizontalMargin, vertical = Dimens.SettingVerticalMargin), - text = stringResource(id = titleRes), - style = SwissTransferTheme.typography.bodySmallRegular, - color = SwissTransferTheme.colors.secondaryTextColor, + UnpaddedTitle( + Modifier.padding(horizontal = Dimens.SettingHorizontalMargin, vertical = Dimens.SettingVerticalMargin), + titleRes ) } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/UnpaddedTitle.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/UnpaddedTitle.kt new file mode 100644 index 000000000..78b5bc04b --- /dev/null +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/UnpaddedTitle.kt @@ -0,0 +1,34 @@ +/* + * Infomaniak SwissTransfer - Android + * Copyright (C) 2024 Infomaniak Network SA + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package com.infomaniak.swisstransfer.ui.screen.main.settings.components + +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme + +@Composable +fun UnpaddedTitle(modifier: Modifier, titleRes: Int) { + Text( + modifier = modifier, + text = stringResource(id = titleRes), + style = SwissTransferTheme.typography.bodySmallRegular, + color = SwissTransferTheme.colors.secondaryTextColor, + ) +} From 6e3c83fcfb08e233bb1a42360815bb8661f00b90 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 19 Sep 2024 08:58:33 +0200 Subject: [PATCH 3/5] Fix option item height with icons vs without icons --- .../settings/components/SingleSelectOptions.kt | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SingleSelectOptions.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SingleSelectOptions.kt index 8edd58a18..19442f6d1 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SingleSelectOptions.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SingleSelectOptions.kt @@ -69,6 +69,9 @@ private fun SettingOptionItem(item: SettingOption, isSelected: Boolean, onClick: item.icon?.let { Image(imageVector = it, contentDescription = null) Spacer(modifier = Modifier.width(Margin.Medium)) + } ?: run { + // Make sure the items with no icons have the same height as the ones with icons + Spacer(modifier = Modifier.height(Margin.Large)) } Text(text = item.title(), Modifier.weight(1.0f), style = SwissTransferTheme.typography.bodyRegular) @@ -105,13 +108,19 @@ private fun SettingOptionItemPreview() { SwissTransferTheme { Surface { Column { - val item = object : SettingOption { + val iconItem = object : SettingOption { override val title: @Composable () -> String = { stringResource(R.string.appName) } override val imageVector: ImageVector = AppIcons.Add override val imageVectorResId = null } - SettingOptionItem(item = item, isSelected = true) {} - SettingOptionItem(item = item, isSelected = false) {} + val textItem = object : SettingOption { + override val title: @Composable () -> String = { stringResource(R.string.appName) } + override val imageVector = null + override val imageVectorResId = null + } + SettingOptionItem(item = iconItem, isSelected = true) {} + SettingOptionItem(item = iconItem, isSelected = false) {} + SettingOptionItem(item = textItem, isSelected = false) {} } } } From 546708c72a695761c1e1bd70d4bddfa9835ab0c7 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 19 Sep 2024 10:52:17 +0200 Subject: [PATCH 4/5] Hide UnpaddedTitle by making it private --- .../main/settings/components/SettingTitle.kt | 51 ------------------- .../components/{OptionTitle.kt => Titles.kt} | 33 ++++++++++++ .../main/settings/components/UnpaddedTitle.kt | 34 ------------- 3 files changed, 33 insertions(+), 85 deletions(-) delete mode 100644 app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SettingTitle.kt rename app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/{OptionTitle.kt => Titles.kt} (66%) delete mode 100644 app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/UnpaddedTitle.kt diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SettingTitle.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SettingTitle.kt deleted file mode 100644 index f09b6fd7f..000000000 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/SettingTitle.kt +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Infomaniak SwissTransfer - Android - * Copyright (C) 2024 Infomaniak Network SA - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ -package com.infomaniak.swisstransfer.ui.screen.main.settings.components - -import android.content.res.Configuration -import androidx.annotation.StringRes -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.padding -import androidx.compose.material3.Surface -import androidx.compose.runtime.Composable -import androidx.compose.ui.Modifier -import androidx.compose.ui.tooling.preview.Preview -import com.infomaniak.swisstransfer.R -import com.infomaniak.swisstransfer.ui.theme.Dimens -import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme - -@Composable -fun SettingTitle(@StringRes titleRes: Int) { - UnpaddedTitle( - Modifier.padding(horizontal = Dimens.SettingHorizontalMargin, vertical = Dimens.SettingVerticalMargin), - titleRes - ) -} - -@Preview(name = "Light") -@Preview(name = "Dark", uiMode = Configuration.UI_MODE_NIGHT_YES or Configuration.UI_MODE_TYPE_NORMAL) -@Composable -private fun SettingTitlePreview() { - SwissTransferTheme { - Surface { - Box { - SettingTitle(titleRes = R.string.appName) - } - } - } -} diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionTitle.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/Titles.kt similarity index 66% rename from app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionTitle.kt rename to app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/Titles.kt index 99e0bc68f..977acde65 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionTitle.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/Titles.kt @@ -22,19 +22,39 @@ import androidx.annotation.StringRes import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.padding import androidx.compose.material3.Surface +import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import com.infomaniak.swisstransfer.R import com.infomaniak.swisstransfer.ui.theme.Dimens import com.infomaniak.swisstransfer.ui.theme.Margin import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme +@Composable +fun SettingTitle(@StringRes titleRes: Int) { + UnpaddedTitle( + Modifier.padding(horizontal = Dimens.SettingHorizontalMargin, vertical = Dimens.SettingVerticalMargin), + titleRes + ) +} + @Composable fun OptionTitle(@StringRes titleRes: Int) { UnpaddedTitle(Modifier.padding(horizontal = Dimens.SettingHorizontalMargin, vertical = Margin.Large), titleRes) } +@Composable +private fun UnpaddedTitle(modifier: Modifier, titleRes: Int) { + Text( + modifier = modifier, + text = stringResource(id = titleRes), + style = SwissTransferTheme.typography.bodySmallRegular, + color = SwissTransferTheme.colors.secondaryTextColor, + ) +} + @Preview(name = "Light") @Preview(name = "Dark", uiMode = Configuration.UI_MODE_NIGHT_YES or Configuration.UI_MODE_TYPE_NORMAL) @Composable @@ -47,3 +67,16 @@ private fun OptionTitlePreview() { } } } + +@Preview(name = "Light") +@Preview(name = "Dark", uiMode = Configuration.UI_MODE_NIGHT_YES or Configuration.UI_MODE_TYPE_NORMAL) +@Composable +private fun SettingTitlePreview() { + SwissTransferTheme { + Surface { + Box { + SettingTitle(titleRes = R.string.appName) + } + } + } +} diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/UnpaddedTitle.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/UnpaddedTitle.kt deleted file mode 100644 index 78b5bc04b..000000000 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/UnpaddedTitle.kt +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Infomaniak SwissTransfer - Android - * Copyright (C) 2024 Infomaniak Network SA - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ -package com.infomaniak.swisstransfer.ui.screen.main.settings.components - -import androidx.compose.material3.Text -import androidx.compose.runtime.Composable -import androidx.compose.ui.Modifier -import androidx.compose.ui.res.stringResource -import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme - -@Composable -fun UnpaddedTitle(modifier: Modifier, titleRes: Int) { - Text( - modifier = modifier, - text = stringResource(id = titleRes), - style = SwissTransferTheme.typography.bodySmallRegular, - color = SwissTransferTheme.colors.secondaryTextColor, - ) -} From 98fc2729a3597728dcfde3e8941291bb853dd6b0 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Fri, 20 Sep 2024 09:16:00 +0200 Subject: [PATCH 5/5] Apply suggestions from code review --- .../main/settings/SettingsDownloadsLimitScreen.kt | 12 ++++++------ .../main/settings/SettingsEmailLanguageScreen.kt | 12 ++++++------ .../ui/screen/main/settings/SettingsThemeScreen.kt | 12 ++++++------ .../main/settings/SettingsValidityPeriodScreen.kt | 12 ++++++------ .../main/settings/components/OptionScaffold.kt | 4 ++-- .../ui/screen/main/settings/components/Titles.kt | 4 +--- 6 files changed, 27 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt index f2f798190..69a621132 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt @@ -34,12 +34,12 @@ fun SettingsDownloadsLimitScreen( onDownloadLimitChange: (DownloadLimit) -> Unit, ) { OptionScaffold( - R.string.settingsOptionDownloadLimit, - R.string.settingsDownloadsLimitTitle, - DownloadLimitOption.entries, - { downloadLimit.ordinal }, - { position -> onDownloadLimitChange(DownloadLimit.entries[position]) }, - navigateBack + topAppBarTitleRes = R.string.settingsOptionDownloadLimit, + optionTitleRes = R.string.settingsDownloadsLimitTitle, + enumEntries = DownloadLimitOption.entries, + selectedSettingOptionPosition = downloadLimit.ordinal, + setSelectedSettingOptionPosition = { position -> onDownloadLimitChange(DownloadLimit.entries[position]) }, + navigateBack = navigateBack ) } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt index 999be82ba..6b9601652 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt @@ -35,12 +35,12 @@ fun SettingsEmailLanguageScreen( onEmailLanguageChange: (EmailLanguage) -> Unit, ) { OptionScaffold( - R.string.settingsOptionEmailLanguage, - R.string.settingsEmailLanguageTitle, - EmailLanguageOption.entries, - { emailLanguage.ordinal }, - { position -> onEmailLanguageChange(EmailLanguage.entries[position]) }, - navigateBack + topAppBarTitleRes = R.string.settingsOptionEmailLanguage, + optionTitleRes = R.string.settingsEmailLanguageTitle, + enumEntries = EmailLanguageOption.entries, + selectedSettingOptionPosition = emailLanguage.ordinal, + setSelectedSettingOptionPosition = { position -> onEmailLanguageChange(EmailLanguage.entries[position]) }, + navigateBack = navigateBack ) } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt index 675c766ad..87eda75e7 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt @@ -39,12 +39,12 @@ fun SettingsThemeScreen( onThemeUpdate: (Theme) -> Unit, ) { OptionScaffold( - R.string.settingsOptionTheme, - R.string.settingsThemeTitle, - ThemeOption.entries, - { theme.ordinal }, - { position -> onThemeUpdate(Theme.entries[position]) }, - navigateBack + topAppBarTitleRes = R.string.settingsOptionTheme, + optionTitleRes = R.string.settingsThemeTitle, + enumEntries = ThemeOption.entries, + selectedSettingOptionPosition = theme.ordinal, + setSelectedSettingOptionPosition = { position -> onThemeUpdate(Theme.entries[position]) }, + navigateBack = navigateBack ) } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsValidityPeriodScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsValidityPeriodScreen.kt index 6f28ecbc9..dd344d655 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsValidityPeriodScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsValidityPeriodScreen.kt @@ -35,12 +35,12 @@ fun SettingsValidityPeriodScreen( onValidityPeriodChange: (ValidityPeriod) -> Unit, ) { OptionScaffold( - R.string.settingsOptionValidityPeriod, - R.string.settingsValidityPeriodTitle, - ValidityPeriodOption.entries, - { validityPeriod.ordinal }, - { position -> onValidityPeriodChange(ValidityPeriod.entries[position]) }, - navigateBack + topAppBarTitleRes = R.string.settingsOptionValidityPeriod, + optionTitleRes = R.string.settingsValidityPeriodTitle, + enumEntries = ValidityPeriodOption.entries, + selectedSettingOptionPosition = validityPeriod.ordinal, + setSelectedSettingOptionPosition = { position -> onValidityPeriodChange(ValidityPeriod.entries[position]) }, + navigateBack = navigateBack ) } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionScaffold.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionScaffold.kt index 7a8d25fc3..2ae90d55d 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionScaffold.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionScaffold.kt @@ -37,7 +37,7 @@ fun OptionScaffold( @StringRes topAppBarTitleRes: Int, @StringRes optionTitleRes: Int, enumEntries: List, - getSelectedSettingOptionPosition: () -> Int, + selectedSettingOptionPosition: Int, setSelectedSettingOptionPosition: (Int) -> Unit, navigateBack: (() -> Unit)? = null, ) { @@ -52,7 +52,7 @@ fun OptionScaffold( ) { OptionTitle(titleRes = optionTitleRes) - var selectedItem by rememberSaveable { mutableIntStateOf(getSelectedSettingOptionPosition()) } + var selectedItem by rememberSaveable { mutableIntStateOf(selectedSettingOptionPosition) } SingleSelectOptions(enumEntries, { selectedItem }, { position -> selectedItem = position setSelectedSettingOptionPosition(position) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/Titles.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/Titles.kt index 977acde65..fbd4b3a03 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/Titles.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/Titles.kt @@ -74,9 +74,7 @@ private fun OptionTitlePreview() { private fun SettingTitlePreview() { SwissTransferTheme { Surface { - Box { - SettingTitle(titleRes = R.string.appName) - } + SettingTitle(titleRes = R.string.appName) } } }