280 fix problematic toasts (bitfireAT/davx5#283)

* When renaming show a toast when account with new name exists already

* Replace toasts with snackbars

* Apply code styling hints

* Move lambdas out of parentheses

* Inject application instead of context

* Replace Toast with Snackbar

* Shorten messages shown in snackbars

* Show Toast in UI instead of model; use AndroidViewModel

* Move account name check out of lock; don't close activity if not successful

* Duplicate account name check: only check for our account type

---------

Co-authored-by: Ricki Hirner <hirner@bitfire.at>
This commit is contained in:
Sunik Kupfer 2023-06-09 13:41:15 +02:00 committed by Ricki Hirner
parent caf7be5e11
commit 1b3fde0854
5 changed files with 99 additions and 79 deletions

View file

@ -10,10 +10,10 @@ import android.content.Intent
import android.net.Uri
import android.view.Menu
import android.view.MenuItem
import android.widget.Toast
import androidx.annotation.CallSuper
import at.bitfire.davdroid.BuildConfig
import at.bitfire.davdroid.R
import com.google.android.material.snackbar.Snackbar
/**
* Default menu items control
@ -36,15 +36,10 @@ abstract class BaseAccountsDrawerHandler: AccountsDrawerHandler {
when (item.itemId) {
R.id.nav_about ->
activity.startActivity(Intent(activity, AboutActivity::class.java))
R.id.nav_beta_feedback ->
if (!UiUtils.launchUri(
activity,
Uri.parse(BETA_FEEDBACK_URI),
Intent.ACTION_SENDTO,
false
)
)
Toast.makeText(activity, R.string.install_email_client, Toast.LENGTH_LONG).show()
R.id.nav_beta_feedback -> item.actionView?.let { view ->
if (!UiUtils.launchUri(activity, Uri.parse(BETA_FEEDBACK_URI), Intent.ACTION_SENDTO, false))
Snackbar.make(view, R.string.install_email_client, Snackbar.LENGTH_LONG).show()
}
R.id.nav_app_settings ->
activity.startActivity(Intent(activity, AppSettingsActivity::class.java))
}

View file

@ -50,37 +50,40 @@ class PermissionsFragment: Fragment() {
model.checkPermissions()
}
model.needAutoResetPermission.observe(viewLifecycleOwner, { keepPermissions ->
model.needAutoResetPermission.observe(viewLifecycleOwner) { keepPermissions ->
if (keepPermissions == true && model.haveAutoResetPermission.value == false) {
Toast.makeText(requireActivity(), R.string.permissions_autoreset_instruction, Toast.LENGTH_LONG).show()
startActivity(Intent(Intent.ACTION_AUTO_REVOKE_PERMISSIONS, Uri.fromParts("package", BuildConfig.APPLICATION_ID, null)))
startActivity(Intent(
Intent.ACTION_AUTO_REVOKE_PERMISSIONS,
Uri.fromParts("package", BuildConfig.APPLICATION_ID, null)
))
}
})
model.needContactsPermissions.observe(viewLifecycleOwner, { needContacts ->
}
model.needContactsPermissions.observe(viewLifecycleOwner) { needContacts ->
if (needContacts && model.haveContactsPermissions.value == false)
requestPermission.launch(CONTACT_PERMISSIONS)
})
model.needCalendarPermissions.observe(viewLifecycleOwner, { needCalendars ->
}
model.needCalendarPermissions.observe(viewLifecycleOwner) { needCalendars ->
if (needCalendars && model.haveCalendarPermissions.value == false)
requestPermission.launch(CALENDAR_PERMISSIONS)
})
model.needNotificationPermissions.observe(viewLifecycleOwner, { needNotifications ->
}
model.needNotificationPermissions.observe(viewLifecycleOwner) { needNotifications ->
if (needNotifications == true && model.haveNotificationPermissions.value == false)
requestPermission.launch(arrayOf(Manifest.permission.POST_NOTIFICATIONS))
})
model.needOpenTasksPermissions.observe(viewLifecycleOwner, { needOpenTasks ->
}
model.needOpenTasksPermissions.observe(viewLifecycleOwner) { needOpenTasks ->
if (needOpenTasks == true && model.haveOpenTasksPermissions.value == false)
requestPermission.launch(TaskProvider.PERMISSIONS_OPENTASKS)
})
model.needTasksOrgPermissions.observe(viewLifecycleOwner, { needTasksOrg ->
}
model.needTasksOrgPermissions.observe(viewLifecycleOwner) { needTasksOrg ->
if (needTasksOrg == true && model.haveTasksOrgPermissions.value == false)
requestPermission.launch(TaskProvider.PERMISSIONS_TASKS_ORG)
})
model.needJtxPermissions.observe(viewLifecycleOwner, { needJtx ->
}
model.needJtxPermissions.observe(viewLifecycleOwner) { needJtx ->
if (needJtx == true && model.haveJtxPermissions.value == false)
requestPermission.launch(TaskProvider.PERMISSIONS_JTX)
})
model.needAllPermissions.observe(viewLifecycleOwner, { needAll ->
}
model.needAllPermissions.observe(viewLifecycleOwner) { needAll ->
if (needAll && model.haveAllPermissions.value == false) {
val all = mutableSetOf(*CONTACT_PERMISSIONS, *CALENDAR_PERMISSIONS, Manifest.permission.POST_NOTIFICATIONS)
if (model.haveOpenTasksPermissions.value != null)
@ -91,7 +94,7 @@ class PermissionsFragment: Fragment() {
all.addAll(TaskProvider.PERMISSIONS_JTX)
requestPermission.launch(all.toTypedArray())
}
})
}
binding.appSettings.setOnClickListener {
PermissionUtils.showAppSettings(requireActivity())

View file

@ -8,9 +8,9 @@ import android.Manifest
import android.accounts.Account
import android.accounts.AccountManager
import android.annotation.SuppressLint
import android.app.Application
import android.app.Dialog
import android.content.ContentResolver
import android.content.Context
import android.content.DialogInterface
import android.content.pm.PackageManager
import android.os.Bundle
@ -24,9 +24,8 @@ import androidx.annotation.WorkerThread
import androidx.core.content.ContextCompat
import androidx.fragment.app.DialogFragment
import androidx.fragment.app.viewModels
import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.Observer
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import at.bitfire.davdroid.*
import at.bitfire.davdroid.db.AppDatabase
@ -41,7 +40,6 @@ import at.bitfire.ical4android.TaskProvider
import com.google.android.material.dialog.MaterialAlertDialogBuilder
import dagger.hilt.android.AndroidEntryPoint
import dagger.hilt.android.lifecycle.HiltViewModel
import dagger.hilt.android.qualifiers.ApplicationContext
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.NonCancellable
import kotlinx.coroutines.launch
@ -81,9 +79,14 @@ class RenameAccountFragment: DialogFragment() {
layout.setPadding(8*density, 8*density, 8*density, 8*density)
layout.addView(editText)
model.finished.observe(this, Observer {
this@RenameAccountFragment.requireActivity().finish()
})
model.errorMessage.observe(this) { msg ->
// we use a Toast to show the error message because a Snackbar is not usable for the input dialog fragment
Toast.makeText(requireActivity(), msg, Toast.LENGTH_LONG).show()
}
model.finishActivity.observe(this) {
requireActivity().finish()
}
return MaterialAlertDialogBuilder(requireActivity())
.setTitle(R.string.account_rename)
@ -95,8 +98,6 @@ class RenameAccountFragment: DialogFragment() {
return@OnClickListener
model.renameAccount(oldAccount, newName)
requireActivity().finish()
})
.setNegativeButton(android.R.string.cancel) { _, _ -> }
.create()
@ -105,19 +106,28 @@ class RenameAccountFragment: DialogFragment() {
@HiltViewModel
class Model @Inject constructor(
@ApplicationContext val context: Context,
application: Application,
val db: AppDatabase
): ViewModel() {
): AndroidViewModel(application) {
val finished = MutableLiveData<Boolean>()
val errorMessage = MutableLiveData<String>()
val finishActivity = MutableLiveData<Boolean>()
/**
* Will try to rename the given account to given string
*
* @param oldAccount the account to be renamed
* @param newName the new name
*/
fun renameAccount(oldAccount: Account, newName: String) {
val context: Application = getApplication()
// remember sync intervals
val oldSettings = try {
AccountSettings(context, oldAccount)
} catch (e: InvalidAccountException) {
Toast.makeText(context, R.string.account_invalid, Toast.LENGTH_LONG).show()
finished.value = true
errorMessage.postValue(context.getString(R.string.account_invalid))
finishActivity.value = true
return
}
@ -129,6 +139,13 @@ class RenameAccountFragment: DialogFragment() {
val syncIntervals = authorities.map { Pair(it, oldSettings.getSyncInterval(it)) }
val accountManager = AccountManager.get(context)
// check whether name is already taken
if (accountManager.getAccountsByType(context.getString(R.string.account_type)).map { it.name }.contains(newName)) {
Logger.log.log(Level.WARNING, "Account with name \"$newName\" already exists")
errorMessage.postValue(context.getString(R.string.account_rename_exists_already))
return
}
try {
/* https://github.com/bitfireAT/davx5/issues/135
Lock accounts cleanup so that the AccountsCleanupWorker doesn't run while we rename the account
@ -139,6 +156,7 @@ class RenameAccountFragment: DialogFragment() {
3. Now the services would be renamed, but they're not here anymore. */
AccountsCleanupWorker.lockAccountsCleanup()
// Renaming account
accountManager.renameAccount(oldAccount, newName, @MainThread {
if (it.result?.name == newName /* account has new name -> success */)
viewModelScope.launch(Dispatchers.Default + NonCancellable) {
@ -151,10 +169,13 @@ class RenameAccountFragment: DialogFragment() {
} else
// release AccountsCleanupWorker mutex now
AccountsCleanupWorker.unlockAccountsCleanup()
// close AccountActivity with old name
finishActivity.postValue(true)
}, null)
} catch (e: Exception) {
Logger.log.log(Level.WARNING, "Couldn't rename account", e)
Toast.makeText(context, R.string.account_rename_couldnt_rename, Toast.LENGTH_LONG).show()
errorMessage.postValue(context.getString(R.string.account_rename_couldnt_rename))
}
}
@ -163,6 +184,7 @@ class RenameAccountFragment: DialogFragment() {
fun onAccountRenamed(accountManager: AccountManager, oldAccount: Account, newName: String, syncIntervals: List<Pair<String, Long?>>) {
// account has now been renamed
Logger.log.info("Updating account name references")
val context: Application = getApplication()
// cancel maybe running synchronization
SyncWorker.cancelSync(context, oldAccount)
@ -173,8 +195,8 @@ class RenameAccountFragment: DialogFragment() {
try {
db.serviceDao().renameAccount(oldAccount.name, newName)
} catch (e: Exception) {
Toast.makeText(context, R.string.account_rename_couldnt_rename, Toast.LENGTH_LONG).show()
Logger.log.log(Level.SEVERE, "Couldn't update service DB", e)
errorMessage.postValue(context.getString(R.string.account_rename_couldnt_rename))
return
}

View file

@ -6,7 +6,7 @@ package at.bitfire.davdroid.ui.account
import android.accounts.Account
import android.annotation.SuppressLint
import android.content.Context
import android.app.Application
import android.content.Intent
import android.os.Build
import android.os.Bundle
@ -42,7 +42,6 @@ import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import dagger.hilt.android.AndroidEntryPoint
import dagger.hilt.android.qualifiers.ApplicationContext
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
@ -121,7 +120,7 @@ class SettingsActivity: AppCompatActivity() {
private fun initSettings() {
// preference group: sync
findPreference<ListPreference>(getString(R.string.settings_sync_interval_contacts_key))!!.let {
model.syncIntervalContacts.observe(viewLifecycleOwner, { interval: Long? ->
model.syncIntervalContacts.observe(viewLifecycleOwner) { interval: Long? ->
if (interval != null) {
it.isEnabled = true
it.isVisible = true
@ -137,10 +136,10 @@ class SettingsActivity: AppCompatActivity() {
}
} else
it.isVisible = false
})
}
}
findPreference<ListPreference>(getString(R.string.settings_sync_interval_calendars_key))!!.let {
model.syncIntervalCalendars.observe(viewLifecycleOwner, { interval: Long? ->
model.syncIntervalCalendars.observe(viewLifecycleOwner) { interval: Long? ->
if (interval != null) {
it.isEnabled = true
it.isVisible = true
@ -156,10 +155,10 @@ class SettingsActivity: AppCompatActivity() {
}
} else
it.isVisible = false
})
}
}
findPreference<ListPreference>(getString(R.string.settings_sync_interval_tasks_key))!!.let {
model.syncIntervalTasks.observe(viewLifecycleOwner, { interval: Long? ->
model.syncIntervalTasks.observe(viewLifecycleOwner) { interval: Long? ->
val provider = model.tasksProvider
if (provider != null && interval != null) {
it.isEnabled = true
@ -176,25 +175,25 @@ class SettingsActivity: AppCompatActivity() {
}
} else
it.isVisible = false
})
}
}
findPreference<SwitchPreferenceCompat>(getString(R.string.settings_sync_wifi_only_key))!!.let {
model.syncWifiOnly.observe(viewLifecycleOwner, { wifiOnly ->
model.syncWifiOnly.observe(viewLifecycleOwner) { wifiOnly ->
it.isEnabled = !settings.containsKey(AccountSettings.KEY_WIFI_ONLY)
it.isChecked = wifiOnly
it.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, wifiOnly ->
model.updateSyncWifiOnly(wifiOnly as Boolean)
false
}
})
}
}
findPreference<EditTextPreference>(getString(R.string.settings_sync_wifi_only_ssids_key))!!.let {
model.syncWifiOnly.observe(viewLifecycleOwner, { wifiOnly ->
model.syncWifiOnly.observe(viewLifecycleOwner) { wifiOnly ->
it.isEnabled = wifiOnly && settings.isWritable(AccountSettings.KEY_WIFI_ONLY_SSIDS)
})
model.syncWifiOnlySSIDs.observe(viewLifecycleOwner, { onlySSIDs ->
}
model.syncWifiOnlySSIDs.observe(viewLifecycleOwner) { onlySSIDs ->
checkWifiPermissions()
if (onlySSIDs != null) {
@ -214,14 +213,14 @@ class SettingsActivity: AppCompatActivity() {
model.updateSyncWifiOnlySSIDs(newOnlySSIDs)
false
}
})
}
}
// preference group: authentication
val prefUserName = findPreference<EditTextPreference>("username")!!
val prefPassword = findPreference<EditTextPreference>("password")!!
val prefCertAlias = findPreference<Preference>("certificate_alias")!!
model.credentials.observe(viewLifecycleOwner, { credentials ->
model.credentials.observe(viewLifecycleOwner) { credentials ->
prefUserName.summary = credentials.userName
prefUserName.text = credentials.userName
prefUserName.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newUserName ->
@ -245,10 +244,10 @@ class SettingsActivity: AppCompatActivity() {
}, null, null, null, -1, credentials.certificateAlias)
true
}
})
}
// preference group: CalDAV
model.hasCalDav.observe(viewLifecycleOwner, { hasCalDav ->
model.hasCalDav.observe(viewLifecycleOwner) { hasCalDav ->
if (!hasCalDav)
findPreference<PreferenceGroup>(getString(R.string.settings_caldav_key))!!.isVisible = false
else {
@ -260,7 +259,7 @@ class SettingsActivity: AppCompatActivity() {
findPreference<EditTextPreference>(getString(R.string.settings_sync_time_range_past_key))!!.let { pref ->
if (hasCalendars)
model.timeRangePastDays.observe(viewLifecycleOwner, { pastDays ->
model.timeRangePastDays.observe(viewLifecycleOwner) { pastDays ->
if (model.syncIntervalCalendars.value != null) {
pref.isVisible = true
if (pastDays != null) {
@ -281,14 +280,14 @@ class SettingsActivity: AppCompatActivity() {
}
} else
pref.isVisible = false
})
}
else
pref.isVisible = false
}
findPreference<EditTextPreference>(getString(R.string.settings_key_default_alarm))!!.let { pref ->
if (hasCalendars)
model.defaultAlarmMinBefore.observe(viewLifecycleOwner, { minBefore ->
model.defaultAlarmMinBefore.observe(viewLifecycleOwner) { minBefore ->
pref.isVisible = true
if (minBefore != null) {
pref.text = minBefore.toString()
@ -306,25 +305,25 @@ class SettingsActivity: AppCompatActivity() {
model.updateDefaultAlarm(minBefore)
false
}
})
}
else
pref.isVisible = false
}
findPreference<SwitchPreferenceCompat>(getString(R.string.settings_manage_calendar_colors_key))!!.let {
model.manageCalendarColors.observe(viewLifecycleOwner, { manageCalendarColors ->
model.manageCalendarColors.observe(viewLifecycleOwner) { manageCalendarColors ->
it.isEnabled = !settings.containsKey(AccountSettings.KEY_MANAGE_CALENDAR_COLORS)
it.isChecked = manageCalendarColors
it.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue ->
model.updateManageCalendarColors(newValue as Boolean)
false
}
})
}
}
findPreference<SwitchPreferenceCompat>(getString(R.string.settings_event_colors_key))!!.let { pref ->
if (hasCalendars)
model.eventColors.observe(viewLifecycleOwner, { eventColors ->
model.eventColors.observe(viewLifecycleOwner) { eventColors ->
pref.isVisible = true
pref.isEnabled = !settings.containsKey(AccountSettings.KEY_EVENT_COLORS)
pref.isChecked = eventColors
@ -332,22 +331,22 @@ class SettingsActivity: AppCompatActivity() {
model.updateEventColors(newValue as Boolean)
false
}
})
}
else
pref.isVisible = false
}
}
})
}
// preference group: CardDAV
model.syncIntervalContacts.observe(viewLifecycleOwner, { contactsSyncInterval ->
model.syncIntervalContacts.observe(viewLifecycleOwner) { contactsSyncInterval ->
val hasCardDav = contactsSyncInterval != null
if (!hasCardDav)
findPreference<PreferenceGroup>(getString(R.string.settings_carddav_key))!!.isVisible = false
else {
findPreference<PreferenceGroup>(getString(R.string.settings_carddav_key))!!.isVisible = true
findPreference<ListPreference>(getString(R.string.settings_contact_group_method_key))!!.let {
model.contactGroupMethod.observe(viewLifecycleOwner, { groupMethod ->
model.contactGroupMethod.observe(viewLifecycleOwner) { groupMethod ->
if (model.syncIntervalContacts.value != null) {
it.isVisible = true
it.value = groupMethod.name
@ -363,10 +362,10 @@ class SettingsActivity: AppCompatActivity() {
}
} else
it.isVisible = false
})
}
}
}
})
}
}
@SuppressLint("WrongConstant")
@ -385,7 +384,7 @@ class SettingsActivity: AppCompatActivity() {
class Model @AssistedInject constructor(
@ApplicationContext val context: Context,
val application: Application,
val settings: SettingsManager,
@Assisted val account: Account
): ViewModel(), SettingsManager.OnChangeListener {
@ -400,7 +399,7 @@ class SettingsActivity: AppCompatActivity() {
// settings
val syncIntervalContacts = MutableLiveData<Long>()
val syncIntervalCalendars = MutableLiveData<Long>()
val tasksProvider = TaskUtils.currentProvider(context)
val tasksProvider = TaskUtils.currentProvider(application)
val syncIntervalTasks = MutableLiveData<Long>()
val hasCalDav = object: MediatorLiveData<Boolean>() {
init {
@ -426,7 +425,7 @@ class SettingsActivity: AppCompatActivity() {
init {
accountSettings = AccountSettings(context, account)
accountSettings = AccountSettings(application, account)
settings.addOnChangeListener(this)
@ -446,7 +445,7 @@ class SettingsActivity: AppCompatActivity() {
fun reload() {
val accountSettings = accountSettings ?: return
syncIntervalContacts.postValue(accountSettings.getSyncInterval(context.getString(R.string.address_books_authority)))
syncIntervalContacts.postValue(accountSettings.getSyncInterval(application.getString(R.string.address_books_authority)))
syncIntervalCalendars.postValue(accountSettings.getSyncInterval(CalendarContract.AUTHORITY))
syncIntervalTasks.postValue(tasksProvider?.let { accountSettings.getSyncInterval(it.authority) })
@ -523,7 +522,7 @@ class SettingsActivity: AppCompatActivity() {
accountSettings?.setGroupMethod(groupMethod)
reload()
resync(context.getString(R.string.address_books_authority), fullResync = true)
resync(application.getString(R.string.address_books_authority), fullResync = true)
}
/**
@ -550,7 +549,7 @@ class SettingsActivity: AppCompatActivity() {
*/
private fun resync(authority: String, fullResync: Boolean) {
val resync = if (fullResync) SyncWorker.FULL_RESYNC else SyncWorker.RESYNC
SyncWorker.enqueue(context, account, authority, resync)
SyncWorker.enqueue(application, account, authority, resync)
}
}

View file

@ -246,6 +246,7 @@
<string name="account_rename">Rename account</string>
<string name="account_rename_new_name">Unsaved local data may be dismissed. Re-synchronization is required after renaming. New account name:</string>
<string name="account_rename_rename">Rename</string>
<string name="account_rename_exists_already">Account name already taken</string>
<string name="account_rename_couldnt_rename">Couldn\'t rename account</string>
<string name="account_delete">Delete account</string>
<string name="account_delete_confirmation_title">Really delete account?</string>