From 33a221971692c1662dc883a7bac9fdcc7b843d35 Mon Sep 17 00:00:00 2001 From: zbue <108109611+zbue@users.noreply.github.com> Date: Sun, 15 Jan 2023 07:24:57 +0800 Subject: [PATCH] Enable `confirmButton` only when needed to respond to user input (#8848) * Enable `confirmButton` when appropriate * Show error in dialog instead * Follow M3 guidelines --- .../interactor/CreateCategoryWithName.kt | 6 -- .../category/interactor/RenameCategory.kt | 7 --- .../category/components/CategoryDialogs.kt | 55 +++++++++++++------ .../components/DeleteLibraryMangaDialog.kt | 1 + .../DownloadCustomChaptersDialog.kt | 1 + .../settings/screen/SettingsAdvancedScreen.kt | 4 -- .../settings/screen/SettingsLibraryScreen.kt | 5 +- .../settings/screen/SettingsTrackingScreen.kt | 12 ++-- .../widget/EditTextPreferenceWidget.kt | 16 ++++++ .../tachiyomi/ui/category/CategoryScreen.kt | 4 +- .../ui/category/CategoryScreenModel.kt | 3 - i18n/src/main/res/values/strings.xml | 1 + 12 files changed, 68 insertions(+), 47 deletions(-) diff --git a/app/src/main/java/eu/kanade/domain/category/interactor/CreateCategoryWithName.kt b/app/src/main/java/eu/kanade/domain/category/interactor/CreateCategoryWithName.kt index be3a4adcd..7daafb71b 100644 --- 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() } } diff --git a/app/src/main/java/eu/kanade/domain/category/interactor/RenameCategory.kt b/app/src/main/java/eu/kanade/domain/category/interactor/RenameCategory.kt index ef531599b..b298ac020 100644 --- 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() } } diff --git a/app/src/main/java/eu/kanade/presentation/category/components/CategoryDialogs.kt b/app/src/main/java/eu/kanade/presentation/category/components/CategoryDialogs.kt index ca3efb421..bd0596289 100644 --- 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, ) { 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, ) { 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, ) }, diff --git a/app/src/main/java/eu/kanade/presentation/components/DeleteLibraryMangaDialog.kt b/app/src/main/java/eu/kanade/presentation/components/DeleteLibraryMangaDialog.kt index 1411a5755..52ee61559 100644 --- 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( diff --git a/app/src/main/java/eu/kanade/presentation/manga/components/DownloadCustomChaptersDialog.kt b/app/src/main/java/eu/kanade/presentation/manga/components/DownloadCustomChaptersDialog.kt index 5b2b5b6cb..53d594434 100644 --- 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)) diff --git 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 index 92f2053cd..37a2fc56c 100644 --- 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) diff --git 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 index bae00429a..80ced0194 100644 --- 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)) } }, diff --git 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 index 91ecac4df..2962c9087 100644 --- 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 } diff --git 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 index 75c416826..6191206ec 100644 --- 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)) { diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/category/CategoryScreen.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/category/CategoryScreen.kt index e71f43640..245c3e37c 100644 --- 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, ) } diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/category/CategoryScreenModel.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/category/CategoryScreenModel.kt index 1ecb89bbe..643362c4b 100644 --- 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) } diff --git a/i18n/src/main/res/values/strings.xml b/i18n/src/main/res/values/strings.xml index cd541c66d..e7afd2658 100644 --- a/i18n/src/main/res/values/strings.xml +++ b/i18n/src/main/res/values/strings.xml @@ -882,6 +882,7 @@ You have no categories. Tap the plus button to create one for organizing your library. You don\'t have any categories yet. Failed to bypass Cloudflare + *required WebView is required for Tachiyomi