Enable confirmButton only when needed to respond to user input (#8848)

* Enable `confirmButton` when appropriate * Show error in dialog instead * Follow M3 guidelines
2024-06-27 16:16:03 +02:00 · 2023-01-15 07:24:57 +08:00 · 2023-01-15 07:24:57 +08:00 · 33a2219716
commit 33a2219716
parent 62480f090b
12 changed files with 68 additions and 47 deletions
--- a/app/src/main/java/eu/kanade/domain/category/interactor/CreateCategoryWithName.kt
+++ b/app/src/main/java/eu/kanade/domain/category/interactor/CreateCategoryWithName.kt
@ -1,7 +1,6 @@
 package eu.kanade.domain.category.interactor

 import eu.kanade.domain.category.model.Category
-import eu.kanade.domain.category.model.anyWithName
 import eu.kanade.domain.category.repository.CategoryRepository
 import eu.kanade.domain.library.service.LibraryPreferences
 import eu.kanade.tachiyomi.util.lang.withNonCancellableContext
@ -23,10 +22,6 @@ class CreateCategoryWithName(

    suspend fun await(name: String): Result = withNonCancellableContext {
        val categories = categoryRepository.getAll()
-        if (categories.anyWithName(name)) {
-            return@withNonCancellableContext Result.NameAlreadyExistsError
-        }
-
        val nextOrder = categories.maxOfOrNull { it.order }?.plus(1) ?: 0
        val newCategory = Category(
            id = 0,
@ -46,7 +41,6 @@ class CreateCategoryWithName(

    sealed class Result {
        object Success : Result()
-        object NameAlreadyExistsError : Result()
        data class InternalError(val error: Throwable) : Result()
    }
 }
--- a/app/src/main/java/eu/kanade/domain/category/interactor/RenameCategory.kt
+++ b/app/src/main/java/eu/kanade/domain/category/interactor/RenameCategory.kt
@ -2,7 +2,6 @@ package eu.kanade.domain.category.interactor

 import eu.kanade.domain.category.model.Category
 import eu.kanade.domain.category.model.CategoryUpdate
-import eu.kanade.domain.category.model.anyWithName
 import eu.kanade.domain.category.repository.CategoryRepository
 import eu.kanade.tachiyomi.util.lang.withNonCancellableContext
 import eu.kanade.tachiyomi.util.system.logcat
@ -13,11 +12,6 @@ class RenameCategory(
 ) {

    suspend fun await(categoryId: Long, name: String) = withNonCancellableContext {
-        val categories = categoryRepository.getAll()
-        if (categories.anyWithName(name)) {
-            return@withNonCancellableContext Result.NameAlreadyExistsError
-        }
-
        val update = CategoryUpdate(
            id = categoryId,
            name = name,
@ -36,7 +30,6 @@ class RenameCategory(

    sealed class Result {
        object Success : Result()
-        object NameAlreadyExistsError : Result()
        data class InternalError(val error: Throwable) : Result()
    }
 }
--- a/app/src/main/java/eu/kanade/presentation/category/components/CategoryDialogs.kt
+++ b/app/src/main/java/eu/kanade/presentation/category/components/CategoryDialogs.kt
@ -15,6 +15,7 @@ import androidx.compose.ui.focus.FocusRequester
 import androidx.compose.ui.focus.focusRequester
 import androidx.compose.ui.res.stringResource
 import eu.kanade.domain.category.model.Category
+import eu.kanade.domain.category.model.anyWithName
 import eu.kanade.tachiyomi.R
 import kotlinx.coroutines.delay
 import kotlin.time.Duration.Companion.seconds
@ -23,17 +24,23 @@ import kotlin.time.Duration.Companion.seconds
 fun CategoryCreateDialog(
    onDismissRequest: () -> Unit,
    onCreate: (String) -> Unit,
+    categories: List<Category>,
 ) {
    var name by remember { mutableStateOf("") }
+
    val focusRequester = remember { FocusRequester() }
+    val nameAlreadyExists = remember(name) { categories.anyWithName(name) }

    AlertDialog(
        onDismissRequest = onDismissRequest,
        confirmButton = {
-            TextButton(onClick = {
-                onCreate(name)
-                onDismissRequest()
-            },) {
+            TextButton(
+                enabled = name.isNotEmpty() && !nameAlreadyExists,
+                onClick = {
+                    onCreate(name)
+                    onDismissRequest()
+                },
+            ) {
                Text(text = stringResource(R.string.action_add))
            }
        },
@ -47,13 +54,15 @@ fun CategoryCreateDialog(
        },
        text = {
            OutlinedTextField(
-                modifier = Modifier
-                    .focusRequester(focusRequester),
+                modifier = Modifier.focusRequester(focusRequester),
                value = name,
                onValueChange = { name = it },
-                label = {
-                    Text(text = stringResource(R.string.name))
+                label = { Text(text = stringResource(R.string.name)) },
+                supportingText = {
+                    val msgRes = if (name.isNotEmpty() && nameAlreadyExists) R.string.error_category_exists else R.string.information_required_plain
+                    Text(text = stringResource(msgRes))
                },
+                isError = name.isNotEmpty() && nameAlreadyExists,
                singleLine = true,
            )
        },
@ -70,18 +79,25 @@ fun CategoryCreateDialog(
 fun CategoryRenameDialog(
    onDismissRequest: () -> Unit,
    onRename: (String) -> Unit,
+    categories: List<Category>,
    category: Category,
 ) {
    var name by remember { mutableStateOf(category.name) }
+    var valueHasChanged by remember { mutableStateOf(false) }
+
    val focusRequester = remember { FocusRequester() }
+    val nameAlreadyExists = remember(name) { categories.anyWithName(name) }

    AlertDialog(
        onDismissRequest = onDismissRequest,
        confirmButton = {
-            TextButton(onClick = {
-                onRename(name)
-                onDismissRequest()
-            },) {
+            TextButton(
+                enabled = valueHasChanged && !nameAlreadyExists,
+                onClick = {
+                    onRename(name)
+                    onDismissRequest()
+                },
+            ) {
                Text(text = stringResource(android.R.string.ok))
            }
        },
@ -95,13 +111,18 @@ fun CategoryRenameDialog(
        },
        text = {
            OutlinedTextField(
-                modifier = Modifier
-                    .focusRequester(focusRequester),
+                modifier = Modifier.focusRequester(focusRequester),
                value = name,
-                onValueChange = { name = it },
-                label = {
-                    Text(text = stringResource(R.string.name))
+                onValueChange = {
+                    valueHasChanged = name != it
+                    name = it
                },
+                label = { Text(text = stringResource(R.string.name)) },
+                supportingText = {
+                    val msgRes = if (valueHasChanged && nameAlreadyExists) R.string.error_category_exists else R.string.information_required_plain
+                    Text(text = stringResource(msgRes))
+                },
+                isError = valueHasChanged && nameAlreadyExists,
                singleLine = true,
            )
        },
--- a/app/src/main/java/eu/kanade/presentation/components/DeleteLibraryMangaDialog.kt
+++ b/app/src/main/java/eu/kanade/presentation/components/DeleteLibraryMangaDialog.kt
@ -44,6 +44,7 @@ fun DeleteLibraryMangaDialog(
        },
        confirmButton = {
            TextButton(
+                enabled = list.any { it.isChecked },
                onClick = {
                    onDismissRequest()
                    onConfirm(
--- a/app/src/main/java/eu/kanade/presentation/manga/components/DownloadCustomChaptersDialog.kt
+++ b/app/src/main/java/eu/kanade/presentation/manga/components/DownloadCustomChaptersDialog.kt
@ -42,6 +42,7 @@ fun DownloadCustomAmountDialog(
        },
        confirmButton = {
            TextButton(
+                enabled = amount != 0,
                onClick = {
                    onDismissRequest()
                    onConfirm(amount.coerceIn(0, maxAmount))
--- a/app/src/main/java/eu/kanade/presentation/more/settings/screen/SettingsAdvancedScreen.kt
+++ b/app/src/main/java/eu/kanade/presentation/more/settings/screen/SettingsAdvancedScreen.kt
@ -275,10 +275,6 @@ object SettingsAdvancedScreen : SearchableSettings {
                    pref = userAgentPref,
                    title = stringResource(R.string.pref_user_agent_string),
                    onValueChanged = {
-                        if (it.isBlank()) {
-                            context.toast(R.string.error_user_agent_string_blank)
-                            return@EditTextPreference false
-                        }
                        try {
                            // OkHttp checks for valid values internally
                            Headers.Builder().add("User-Agent", it)
--- a/app/src/main/java/eu/kanade/presentation/more/settings/screen/SettingsLibraryScreen.kt
+++ b/app/src/main/java/eu/kanade/presentation/more/settings/screen/SettingsLibraryScreen.kt
@ -315,7 +315,10 @@ object SettingsLibraryScreen : SearchableSettings {
                }
            },
            confirmButton = {
-                TextButton(onClick = { onValueChanged(portraitValue, landscapeValue) }) {
+                TextButton(
+                    enabled = portraitValue != initialPortrait || landscapeValue != initialLandscape,
+                    onClick = { onValueChanged(portraitValue, landscapeValue) },
+                ) {
                    Text(text = stringResource(android.R.string.ok))
                }
            },
--- a/app/src/main/java/eu/kanade/presentation/more/settings/screen/SettingsTrackingScreen.kt
+++ b/app/src/main/java/eu/kanade/presentation/more/settings/screen/SettingsTrackingScreen.kt
@ -222,7 +222,7 @@ object SettingsTrackingScreen : SearchableSettings {
                        label = { Text(text = stringResource(uNameStringRes)) },
                        keyboardOptions = KeyboardOptions(imeAction = ImeAction.Next),
                        singleLine = true,
-                        isError = inputError && username.text.isEmpty(),
+                        isError = inputError && !processing,
                    )

                    var hidePassword by remember { mutableStateOf(true) }
@ -253,21 +253,16 @@ object SettingsTrackingScreen : SearchableSettings {
                            imeAction = ImeAction.Done,
                        ),
                        singleLine = true,
-                        isError = inputError && password.text.isEmpty(),
+                        isError = inputError && !processing,
                    )
                }
            },
            confirmButton = {
                Button(
                    modifier = Modifier.fillMaxWidth(),
-                    enabled = !processing,
+                    enabled = !processing && username.text.isNotBlank() && password.text.isNotBlank(),
                    onClick = {
-                        if (username.text.isEmpty() || password.text.isEmpty()) {
-                            inputError = true
-                            return@Button
-                        }
                        scope.launchIO {
-                            inputError = false
                            processing = true
                            val result = checkLogin(
                                context = context,
@ -275,6 +270,7 @@ object SettingsTrackingScreen : SearchableSettings {
                                username = username.text,
                                password = password.text,
                            )
+                            inputError = !result
                            if (result) onDismissRequest()
                            processing = false
                        }
--- a/app/src/main/java/eu/kanade/presentation/more/settings/widget/EditTextPreferenceWidget.kt
+++ b/app/src/main/java/eu/kanade/presentation/more/settings/widget/EditTextPreferenceWidget.kt
@ -1,7 +1,12 @@
 package eu.kanade.presentation.more.settings.widget

 import androidx.compose.foundation.layout.fillMaxWidth
+import androidx.compose.material.icons.Icons
+import androidx.compose.material.icons.filled.Cancel
+import androidx.compose.material.icons.filled.Error
 import androidx.compose.material3.AlertDialog
+import androidx.compose.material3.Icon
+import androidx.compose.material3.IconButton
 import androidx.compose.material3.OutlinedTextField
 import androidx.compose.material3.Text
 import androidx.compose.material3.TextButton
@ -50,6 +55,16 @@ fun EditTextPreferenceWidget(
                OutlinedTextField(
                    value = textFieldValue,
                    onValueChange = { textFieldValue = it },
+                    trailingIcon = {
+                        if (textFieldValue.text.isBlank()) {
+                            Icon(imageVector = Icons.Filled.Error, contentDescription = null)
+                        } else {
+                            IconButton(onClick = { textFieldValue = TextFieldValue("") }) {
+                                Icon(imageVector = Icons.Filled.Cancel, contentDescription = null)
+                            }
+                        }
+                    },
+                    isError = textFieldValue.text.isBlank(),
                    singleLine = true,
                    modifier = Modifier.fillMaxWidth(),
                )
@ -59,6 +74,7 @@ fun EditTextPreferenceWidget(
            ),
            confirmButton = {
                TextButton(
+                    enabled = textFieldValue.text != value && textFieldValue.text.isNotBlank(),
                    onClick = {
                        scope.launch {
                            if (onConfirm(textFieldValue.text)) {
--- a/app/src/main/java/eu/kanade/tachiyomi/ui/category/CategoryScreen.kt
+++ b/app/src/main/java/eu/kanade/tachiyomi/ui/category/CategoryScreen.kt
@ -52,13 +52,15 @@ class CategoryScreen : Screen {
            CategoryDialog.Create -> {
                CategoryCreateDialog(
                    onDismissRequest = screenModel::dismissDialog,
-                    onCreate = { screenModel.createCategory(it) },
+                    onCreate = screenModel::createCategory,
+                    categories = successState.categories,
                )
            }
            is CategoryDialog.Rename -> {
                CategoryRenameDialog(
                    onDismissRequest = screenModel::dismissDialog,
                    onRename = { screenModel.renameCategory(dialog.category, it) },
+                    categories = successState.categories,
                    category = dialog.category,
                )
            }
--- a/app/src/main/java/eu/kanade/tachiyomi/ui/category/CategoryScreenModel.kt
+++ b/app/src/main/java/eu/kanade/tachiyomi/ui/category/CategoryScreenModel.kt
@ -47,7 +47,6 @@ class CategoryScreenModel(
        coroutineScope.launch {
            when (createCategoryWithName.await(name)) {
                is CreateCategoryWithName.Result.InternalError -> _events.send(CategoryEvent.InternalError)
-                CreateCategoryWithName.Result.NameAlreadyExistsError -> _events.send(CategoryEvent.CategoryWithNameAlreadyExists)
                else -> {}
            }
        }
@ -84,7 +83,6 @@ class CategoryScreenModel(
        coroutineScope.launch {
            when (renameCategory.await(category, name)) {
                is RenameCategory.Result.InternalError -> _events.send(CategoryEvent.InternalError)
-                RenameCategory.Result.NameAlreadyExistsError -> _events.send(CategoryEvent.CategoryWithNameAlreadyExists)
                else -> {}
            }
        }
@ -117,7 +115,6 @@ sealed class CategoryDialog {

 sealed class CategoryEvent {
    sealed class LocalizedMessage(@StringRes val stringRes: Int) : CategoryEvent()
-    object CategoryWithNameAlreadyExists : LocalizedMessage(R.string.error_category_exists)
    object InternalError : LocalizedMessage(R.string.internal_error)
 }

--- a/i18n/src/main/res/values/strings.xml
+++ b/i18n/src/main/res/values/strings.xml
@ -882,6 +882,7 @@
    <string name="information_empty_category">You have no categories. Tap the plus button to create one for organizing your library.</string>
    <string name="information_empty_category_dialog">You don\'t have any categories yet.</string>
    <string name="information_cloudflare_bypass_failure">Failed to bypass Cloudflare</string>
+    <string name="information_required_plain">*required</string>
    <!-- Do not translate "WebView" -->
    <string name="information_webview_required">WebView is required for Tachiyomi</string>
    <!-- Do not translate "WebView" -->