From 68eaefbc9cc6356c25a81be738feac1e0e4296aa Mon Sep 17 00:00:00 2001 From: David Langley Date: Fri, 27 Aug 2021 15:48:11 +0100 Subject: [PATCH] Show error dialog rather than toast. Add validation to keyword field for slashes and dots. --- .../app/core/preference/KeywordPreference.kt | 61 ++++++++++++++----- .../settings/VectorSettingsBaseFragment.kt | 16 ++++- ...dMentionsNotificationPreferenceFragment.kt | 27 ++++---- ...sPushRuleNotificationPreferenceFragment.kt | 6 +- vector/src/main/res/values/strings.xml | 2 + 5 files changed, 78 insertions(+), 34 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/preference/KeywordPreference.kt b/vector/src/main/java/im/vector/app/core/preference/KeywordPreference.kt index a3cab1fb9e..b57bb27671 100644 --- a/vector/src/main/java/im/vector/app/core/preference/KeywordPreference.kt +++ b/vector/src/main/java/im/vector/app/core/preference/KeywordPreference.kt @@ -17,6 +17,7 @@ package im.vector.app.core.preference import android.content.Context +import android.text.Editable import android.util.AttributeSet import android.view.inputmethod.EditorInfo import android.widget.Button @@ -25,7 +26,10 @@ import androidx.core.view.children import androidx.preference.PreferenceViewHolder import com.google.android.material.chip.Chip import com.google.android.material.chip.ChipGroup +import com.google.android.material.textfield.TextInputLayout import im.vector.app.R +import im.vector.app.core.epoxy.addTextChangedListenerOnce +import im.vector.app.core.platform.SimpleTextWatcher class KeywordPreference : VectorPreference { @@ -36,6 +40,7 @@ class KeywordPreference : VectorPreference { } private var keywordsEnabled = true + private var isCurrentKeywordValid = true private var _keywords: LinkedHashSet = linkedSetOf() @@ -78,6 +83,7 @@ class KeywordPreference : VectorPreference { val chipEditText = holder.findViewById(R.id.chipEditText) as? EditText ?: return val chipGroup = holder.findViewById(R.id.chipGroup) as? ChipGroup ?: return val addKeywordButton = holder.findViewById(R.id.addKeywordButton) as? Button ?: return + val chipTextInputLayout = holder.findViewById(R.id.chipTextInputLayout) as? TextInputLayout ?: return chipEditText.text = null chipGroup.removeAllViews() @@ -90,30 +96,57 @@ class KeywordPreference : VectorPreference { chipGroup.isEnabled = keywordsEnabled chipGroup.children.forEach { it.isEnabled = keywordsEnabled } - fun addKeyword(): Boolean { - val keyword = chipEditText.text.toString().trim() - if (keyword.isEmpty()) { - return false - } - listener?.didAddKeyword(keyword) - onPreferenceChangeListener?.onPreferenceChange(this, _keywords) - notifyChanged() - chipEditText.text = null - return true - } - + chipEditText.addTextChangedListenerOnce(onTextChangeListener(chipTextInputLayout, addKeywordButton)) chipEditText.setOnEditorActionListener { _, actionId, _ -> if (actionId != EditorInfo.IME_ACTION_DONE) { return@setOnEditorActionListener false } - return@setOnEditorActionListener addKeyword() + return@setOnEditorActionListener addKeyword(chipEditText) } chipEditText.setOnFocusChangeListener { _, hasFocus -> listener?.onFocusDidChange(hasFocus) } addKeywordButton.setOnClickListener { - addKeyword() + addKeyword(chipEditText) + } + } + + private fun addKeyword(chipEditText: EditText): Boolean { + val keyword = chipEditText.text.toString().trim() + + if (!isCurrentKeywordValid || keyword.isEmpty()) { + return false + } + + listener?.didAddKeyword(keyword) + onPreferenceChangeListener?.onPreferenceChange(this, _keywords) + notifyChanged() + chipEditText.text = null + return true + } + + private fun onTextChangeListener(chipTextInputLayout: TextInputLayout, addKeywordButton: Button) = object : SimpleTextWatcher() { + override fun afterTextChanged(s: Editable) { + val keyword = s.toString().trim() + val errorMessage = when { + keyword.startsWith(".") -> { + context.getString(R.string.settings_notification_keyword_contains_dot) + } + keyword.contains("\\") -> { + context.getString(R.string.settings_notification_keyword_contains_invalid_character, "\\") + } + keyword.contains("/") -> { + context.getString(R.string.settings_notification_keyword_contains_invalid_character, "/") + } + else -> null + } + + chipTextInputLayout.isErrorEnabled = errorMessage != null + chipTextInputLayout.error = errorMessage + val keywordValid = errorMessage == null + addKeywordButton.isEnabled = keywordsEnabled && keywordValid + this@KeywordPreference.isCurrentKeywordValid = keywordValid } } diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsBaseFragment.kt b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsBaseFragment.kt index 069216aaae..d57c89ca3b 100644 --- a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsBaseFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsBaseFragment.kt @@ -21,6 +21,7 @@ import android.os.Bundle import android.view.View import androidx.annotation.CallSuper import androidx.preference.PreferenceFragmentCompat +import com.google.android.material.dialog.MaterialAlertDialogBuilder import im.vector.app.R import im.vector.app.core.di.DaggerScreenComponent import im.vector.app.core.di.HasScreenInjector @@ -160,9 +161,22 @@ abstract class VectorSettingsBaseFragment : PreferenceFragmentCompat(), HasScree } activity?.runOnUiThread { if (errorMessage != null && errorMessage.isNotBlank()) { - activity?.toast(errorMessage) + displayErrorDialog(errorMessage) } hideLoadingView() } } + + protected fun displayErrorDialog(throwable: Throwable) { + displayErrorDialog(errorFormatter.toHumanReadable(throwable)) + } + + protected fun displayErrorDialog(errorMessage: String) { + MaterialAlertDialogBuilder(requireActivity()) + .setTitle(R.string.dialog_title_error) + .setMessage(errorMessage) + .setPositiveButton(R.string.ok, null) + .show() + } + } diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsKeywordAndMentionsNotificationPreferenceFragment.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsKeywordAndMentionsNotificationPreferenceFragment.kt index 1f697d7f5f..fc9083f9a4 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsKeywordAndMentionsNotificationPreferenceFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsKeywordAndMentionsNotificationPreferenceFragment.kt @@ -25,7 +25,6 @@ import im.vector.app.core.preference.KeywordPreference import im.vector.app.core.preference.VectorCheckboxPreference import im.vector.app.core.preference.VectorPreference import im.vector.app.core.preference.VectorPreferenceCategory -import im.vector.app.core.utils.toast import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -68,7 +67,7 @@ class VectorSettingsKeywordAndMentionsNotificationPreferenceFragment val footerPreference = findPreference("SETTINGS_KEYWORDS_FOOTER")!! footerPreference.isIconSpaceReserved = false - + val host = this keywordPreference.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue -> val keywords = editKeywordPreference.keywords val newChecked = newValue as Boolean @@ -82,17 +81,15 @@ class VectorSettingsKeywordAndMentionsNotificationPreferenceFragment keywordPreference.isChecked = newChecked editKeywordPreference.isEnabled = newChecked } - result.onFailure { + result.onFailure { failure -> refreshDisplay() - activity?.toast(errorFormatter.toHumanReadable(it)) + host.displayErrorDialog(failure) } } false } - val host = this editKeywordPreference.listener = object: KeywordPreference.Listener { - override fun onFocusDidChange(hasFocus: Boolean) { host.keywordsHasFocus = true } @@ -147,37 +144,35 @@ class VectorSettingsKeywordAndMentionsNotificationPreferenceFragment val newActions = standardAction.actions ?: return val newRule = PushRule(actions = newActions.toJson(), pattern = keyword, enabled = enabled, ruleId = keyword) displayLoadingView() + val host = this lifecycleScope.launch { val result = runCatching { session.addPushRule(RuleKind.CONTENT, newRule) } + hideLoadingView() if (!isAdded) { return@launch } - hideLoadingView() // Already added to UI, no-op on success - result.onFailure { - // Just display an error on failure, keywords have not been added to the UI - activity?.toast(errorFormatter.toHumanReadable(it)) - } + + result.onFailure(host::displayErrorDialog) } } fun removeKeyword(keyword: String) { displayLoadingView() + val host = this lifecycleScope.launch { val result = runCatching { session.removePushRule(RuleKind.CONTENT, keyword) } + hideLoadingView() if (!isAdded) { return@launch } - hideLoadingView() // Already added to UI, no-op on success - result.onFailure { - // Just display an error on failure, keywords have not been added to the UI - activity?.toast(errorFormatter.toHumanReadable(it)) - } + + result.onFailure(host::displayErrorDialog) } } diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationPreferenceFragment.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationPreferenceFragment.kt index 2b90baf3ba..cc87d101e8 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationPreferenceFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationPreferenceFragment.kt @@ -19,7 +19,6 @@ package im.vector.app.features.settings.notifications import androidx.lifecycle.lifecycleScope import androidx.preference.Preference import im.vector.app.core.preference.VectorCheckboxPreference -import im.vector.app.core.utils.toast import im.vector.app.features.settings.VectorSettingsBaseFragment import kotlinx.coroutines.launch import org.matrix.android.sdk.api.pushrules.RuleKind @@ -57,6 +56,7 @@ abstract class VectorSettingsPushRuleNotificationPreferenceFragment val newActions = standardAction.actions displayLoadingView() + val host = this lifecycleScope.launch { val result = runCatching { session.updatePushRuleActions(kind, @@ -64,17 +64,17 @@ abstract class VectorSettingsPushRuleNotificationPreferenceFragment enabled, newActions) } + hideLoadingView() if (!isAdded) { return@launch } - hideLoadingView() result.onSuccess { preference.isChecked = checked } result.onFailure { failure -> // Restore the previous value refreshDisplay() - activity?.toast(errorFormatter.toHumanReadable(failure)) + host.displayErrorDialog(failure) } } } diff --git a/vector/src/main/res/values/strings.xml b/vector/src/main/res/values/strings.xml index 3bd5e10023..1ba938169d 100644 --- a/vector/src/main/res/values/strings.xml +++ b/vector/src/main/res/values/strings.xml @@ -1097,6 +1097,8 @@ Notify me for Your keywords Add new keyword + Keywords cannot start with \'.\' + Keywords cannot contain \'%s\' Notification privacy Troubleshoot Notifications