Apply suggestions from code review

Infomaniak · Sep 20, 2024 · 98fc272 · 98fc272
1 parent 546708c
commit 98fc272
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 29 deletions.
diff --git a/...java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt b/...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/.../java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt b/.../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/...src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt b/...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/...java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsValidityPeriodScreen.kt b/...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/...in/java/com/infomaniak/swisstransfer/ui/screen/main/settings/components/OptionScaffold.kt b/...in/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<SettingOption>,
-    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
@@ -74,9 +74,7 @@ private fun OptionTitlePreview() {
 private fun SettingTitlePreview() {
     SwissTransferTheme {
         Surface {
-            Box {
-                SettingTitle(titleRes = R.string.appName)
-            }
+            SettingTitle(titleRes = R.string.appName)
         }
     }
 }