From 153a9ad9c24402c6b826e932209260cc0eca361e Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Mon, 19 Jun 2023 23:49:20 +0200 Subject: [PATCH] Simplify repeated code that shows errors (#3762) Instead of repeating the same if/else check on the error type when setting up the background message, move this in to BackgroundMessageView. Provide different `setup()` variants, including one that just takes a throwable and a handler, and figures out the correct drawables and error message. Update and simplify call sites. --- .../tusky/AccountsInListFragment.kt | 16 +--------------- .../account/list/ListsForAccountFragment.kt | 12 +----------- .../account/media/AccountMediaFragment.kt | 8 +------- .../accountlist/AccountListFragment.kt | 13 +++---------- .../conversation/ConversationsFragment.kt | 12 +----------- .../followedtags/FollowedTagsActivity.kt | 7 +------ .../fragment/InstanceListFragment.kt | 14 +++----------- .../scheduled/ScheduledStatusActivity.kt | 11 +---------- .../components/timeline/TimelineFragment.kt | 12 +----------- .../components/viewthread/ViewThreadFragment.kt | 17 +---------------- .../viewthread/edits/ViewEditsFragment.kt | 14 +------------- .../tusky/util/ThrowableExtensions.kt | 16 ++++++++++++++++ .../tusky/view/BackgroundMessageView.kt | 16 ++++++++++++++-- app/src/main/res/values/strings.xml | 2 +- 14 files changed, 46 insertions(+), 124 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/AccountsInListFragment.kt b/app/src/main/java/com/keylesspalace/tusky/AccountsInListFragment.kt index 7821630b..a20526e9 100644 --- a/app/src/main/java/com/keylesspalace/tusky/AccountsInListFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/AccountsInListFragment.kt @@ -46,7 +46,6 @@ import com.keylesspalace.tusky.util.viewBinding import com.keylesspalace.tusky.viewmodel.AccountsInListViewModel import com.keylesspalace.tusky.viewmodel.State import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject private typealias AccountInfo = Pair @@ -146,23 +145,10 @@ class AccountsInListFragment : DialogFragment(), Injectable { private fun handleError(error: Throwable) { binding.messageView.show() - val retryAction = { _: View -> + binding.messageView.setup(error) { _: View -> binding.messageView.hide() viewModel.load(listId) } - if (error is IOException) { - binding.messageView.setup( - R.drawable.elephant_offline, - R.string.error_network, - retryAction - ) - } else { - binding.messageView.setup( - R.drawable.elephant_error, - R.string.error_generic, - retryAction - ) - } } private fun onRemoveFromList(accountId: String) { diff --git a/app/src/main/java/com/keylesspalace/tusky/components/account/list/ListsForAccountFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/account/list/ListsForAccountFragment.kt index d527f613..08c93756 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/account/list/ListsForAccountFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/account/list/ListsForAccountFragment.kt @@ -40,7 +40,6 @@ import com.keylesspalace.tusky.util.viewBinding import com.keylesspalace.tusky.util.visible import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject class ListsForAccountFragment : DialogFragment(), Injectable { @@ -103,16 +102,7 @@ class ListsForAccountFragment : DialogFragment(), Injectable { binding.listsView.hide() binding.messageView.apply { show() - - if (error is IOException) { - setup(R.drawable.elephant_offline, R.string.error_network) { - load() - } - } else { - setup(R.drawable.elephant_error, R.string.error_generic) { - load() - } - } + setup(error) { load() } } } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaFragment.kt index df87372e..b39a8b5b 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaFragment.kt @@ -51,7 +51,6 @@ import com.mikepenz.iconics.utils.colorInt import com.mikepenz.iconics.utils.sizeDp import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject /** @@ -133,12 +132,7 @@ class AccountMediaFragment : } is LoadState.Error -> { binding.statusView.show() - - if ((loadState.refresh as LoadState.Error).error is IOException) { - binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network, null) - } else { - binding.statusView.setup(R.drawable.elephant_error, R.string.error_generic, null) - } + binding.statusView.setup((loadState.refresh as LoadState.Error).error) } is LoadState.Loading -> { binding.progressBar.show() diff --git a/app/src/main/java/com/keylesspalace/tusky/components/accountlist/AccountListFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/accountlist/AccountListFragment.kt index dfa1726a..68eba3c7 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/accountlist/AccountListFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/accountlist/AccountListFragment.kt @@ -393,16 +393,9 @@ class AccountListFragment : if (adapter.itemCount == 0) { binding.messageView.show() - if (throwable is IOException) { - binding.messageView.setup(R.drawable.elephant_offline, R.string.error_network) { - binding.messageView.hide() - this.fetchAccounts(null) - } - } else { - binding.messageView.setup(R.drawable.elephant_error, R.string.error_generic) { - binding.messageView.hide() - this.fetchAccounts(null) - } + binding.messageView.setup(throwable) { + binding.messageView.hide() + this.fetchAccounts(null) } } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsFragment.kt index 303ec6b0..48d1e87b 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsFragment.kt @@ -64,7 +64,6 @@ import com.mikepenz.iconics.utils.sizeDp import kotlinx.coroutines.delay import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject import kotlin.time.DurationUnit import kotlin.time.toDuration @@ -139,16 +138,7 @@ class ConversationsFragment : } is LoadState.Error -> { binding.statusView.show() - - if ((loadState.refresh as LoadState.Error).error is IOException) { - binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network) { - refreshContent() - } - } else { - binding.statusView.setup(R.drawable.elephant_error, R.string.error_generic) { - refreshContent() - } - } + binding.statusView.setup((loadState.refresh as LoadState.Error).error) { refreshContent() } } is LoadState.Loading -> { binding.progressBar.show() diff --git a/app/src/main/java/com/keylesspalace/tusky/components/followedtags/FollowedTagsActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/followedtags/FollowedTagsActivity.kt index 94a0b47e..b6b56d4a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/followedtags/FollowedTagsActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/followedtags/FollowedTagsActivity.kt @@ -31,7 +31,6 @@ import com.keylesspalace.tusky.util.viewBinding import com.keylesspalace.tusky.util.visible import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject class FollowedTagsActivity : @@ -108,11 +107,7 @@ class FollowedTagsActivity : binding.followedTagsView.hide() binding.followedTagsMessageView.show() val errorState = loadState.refresh as LoadState.Error - if (errorState.error is IOException) { - binding.followedTagsMessageView.setup(R.drawable.elephant_offline, R.string.error_network) { retry() } - } else { - binding.followedTagsMessageView.setup(R.drawable.elephant_error, R.string.error_generic) { retry() } - } + binding.followedTagsMessageView.setup(errorState.error) { retry() } Log.w(TAG, "error loading followed hashtags", errorState.error) } else { binding.followedTagsView.show() diff --git a/app/src/main/java/com/keylesspalace/tusky/components/instancemute/fragment/InstanceListFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/instancemute/fragment/InstanceListFragment.kt index 1e4925a5..1da0a2b7 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/instancemute/fragment/InstanceListFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/instancemute/fragment/InstanceListFragment.kt @@ -26,7 +26,6 @@ import com.keylesspalace.tusky.util.viewBinding import com.keylesspalace.tusky.view.EndlessOnScrollListener import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject class InstanceListFragment : Fragment(R.layout.fragment_instance_list), Injectable, InstanceActionListener { @@ -146,16 +145,9 @@ class InstanceListFragment : Fragment(R.layout.fragment_instance_list), Injectab if (adapter.itemCount == 0) { binding.messageView.show() - if (throwable is IOException) { - binding.messageView.setup(R.drawable.elephant_offline, R.string.error_network) { - binding.messageView.hide() - this.fetchInstances(null) - } - } else { - binding.messageView.setup(R.drawable.elephant_error, R.string.error_generic) { - binding.messageView.hide() - this.fetchInstances(null) - } + binding.messageView.setup(throwable) { + binding.messageView.hide() + this.fetchInstances(null) } } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusActivity.kt index 102e67be..a53c0892 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusActivity.kt @@ -47,7 +47,6 @@ import com.mikepenz.iconics.utils.colorInt import com.mikepenz.iconics.utils.sizeDp import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject class ScheduledStatusActivity : @@ -102,15 +101,7 @@ class ScheduledStatusActivity : binding.errorMessageView.show() val errorState = loadState.refresh as LoadState.Error - if (errorState.error is IOException) { - binding.errorMessageView.setup(R.drawable.elephant_offline, R.string.error_network) { - refreshStatuses() - } - } else { - binding.errorMessageView.setup(R.drawable.elephant_error, R.string.error_generic) { - refreshStatuses() - } - } + binding.errorMessageView.setup(errorState.error) { refreshStatuses() } } if (loadState.refresh != LoadState.Loading) { binding.swipeRefreshLayout.isRefreshing = false diff --git a/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt index bae257a9..ef5d9085 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt @@ -79,7 +79,6 @@ import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers import io.reactivex.rxjava3.core.Observable import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch -import java.io.IOException import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -242,16 +241,7 @@ class TimelineFragment : } is LoadState.Error -> { binding.statusView.show() - - if ((loadState.refresh as LoadState.Error).error is IOException) { - binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network) { - onRefresh() - } - } else { - binding.statusView.setup(R.drawable.elephant_error, R.string.error_generic) { - onRefresh() - } - } + binding.statusView.setup((loadState.refresh as LoadState.Error).error) { onRefresh() } } is LoadState.Loading -> { binding.progressBar.show() diff --git a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadFragment.kt index 1a3777f9..0d4ba3b3 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadFragment.kt @@ -60,7 +60,6 @@ import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.delay import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject class ViewThreadFragment : @@ -201,21 +200,7 @@ class ViewThreadFragment : binding.recyclerView.hide() binding.statusView.show() - if (uiState.throwable is IOException) { - binding.statusView.setup( - R.drawable.elephant_offline, - R.string.error_network - ) { - viewModel.retry(thisThreadsStatusId) - } - } else { - binding.statusView.setup( - R.drawable.elephant_error, - R.string.error_generic - ) { - viewModel.retry(thisThreadsStatusId) - } - } + binding.statusView.setup(uiState.throwable) { viewModel.retry(thisThreadsStatusId) } } is ThreadUiState.Success -> { if (uiState.statusViewData.none { viewData -> viewData.isDetailed }) { diff --git a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/edits/ViewEditsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/edits/ViewEditsFragment.kt index 9fa5a30f..95a0b96d 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/edits/ViewEditsFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/edits/ViewEditsFragment.kt @@ -53,7 +53,6 @@ import com.mikepenz.iconics.typeface.library.googlematerial.GoogleMaterial import com.mikepenz.iconics.utils.colorInt import com.mikepenz.iconics.utils.sizeDp import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject class ViewEditsFragment : @@ -112,14 +111,6 @@ class ViewEditsFragment : binding.initialProgressBar.hide() when (uiState.throwable) { - is IOException -> { - binding.statusView.setup( - R.drawable.elephant_offline, - R.string.error_network - ) { - viewModel.loadEdits(statusId, force = true) - } - } is ViewEditsViewModel.MissingEditsException -> { binding.statusView.setup( R.drawable.elephant_friend_empty, @@ -127,10 +118,7 @@ class ViewEditsFragment : ) } else -> { - binding.statusView.setup( - R.drawable.elephant_error, - R.string.error_generic - ) { + binding.statusView.setup(uiState.throwable) { viewModel.loadEdits(statusId, force = true) } } diff --git a/app/src/main/java/com/keylesspalace/tusky/util/ThrowableExtensions.kt b/app/src/main/java/com/keylesspalace/tusky/util/ThrowableExtensions.kt index 26f96255..a3811a35 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/ThrowableExtensions.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/ThrowableExtensions.kt @@ -1,8 +1,11 @@ package com.keylesspalace.tusky.util +import android.content.Context +import com.keylesspalace.tusky.R import org.json.JSONException import org.json.JSONObject import retrofit2.HttpException +import java.io.IOException /** * checks if this throwable indicates an error causes by a 4xx/5xx server response and @@ -24,3 +27,16 @@ fun Throwable.getServerErrorMessage(): String? { } return null } + +/** @return A drawable resource to accompany the error message for this throwable */ +fun Throwable.getDrawableRes(): Int = when (this) { + is IOException -> R.drawable.elephant_offline + is HttpException -> R.drawable.elephant_offline + else -> R.drawable.elephant_error +} + +/** @return A string error message for this throwable */ +fun Throwable.getErrorString(context: Context): String = getServerErrorMessage() ?: when (this) { + is IOException -> context.getString(R.string.error_network) + else -> context.getString(R.string.error_generic) +} diff --git a/app/src/main/java/com/keylesspalace/tusky/view/BackgroundMessageView.kt b/app/src/main/java/com/keylesspalace/tusky/view/BackgroundMessageView.kt index 97078d50..650d92cc 100644 --- a/app/src/main/java/com/keylesspalace/tusky/view/BackgroundMessageView.kt +++ b/app/src/main/java/com/keylesspalace/tusky/view/BackgroundMessageView.kt @@ -13,6 +13,8 @@ import androidx.annotation.StringRes import com.keylesspalace.tusky.R import com.keylesspalace.tusky.databinding.ViewBackgroundMessageBinding import com.keylesspalace.tusky.util.addDrawables +import com.keylesspalace.tusky.util.getDrawableRes +import com.keylesspalace.tusky.util.getErrorString import com.keylesspalace.tusky.util.visible /** @@ -35,16 +37,26 @@ class BackgroundMessageView @JvmOverloads constructor( } } + fun setup(throwable: Throwable, listener: ((v: View) -> Unit)? = null) { + setup(throwable.getDrawableRes(), throwable.getErrorString(context), listener) + } + + fun setup( + @DrawableRes imageRes: Int, + @StringRes messageRes: Int, + clickListener: ((v: View) -> Unit)? = null + ) = setup(imageRes, context.getString(messageRes), clickListener) + /** * Setup image, message and button. * If [clickListener] is `null` then the button will be hidden. */ fun setup( @DrawableRes imageRes: Int, - @StringRes messageRes: Int, + message: String, clickListener: ((v: View) -> Unit)? = null ) { - binding.messageTextView.setText(messageRes) + binding.messageTextView.text = message binding.messageTextView.movementMethod = LinkMovementMethod.getInstance() binding.imageView.setImageResource(imageRes) binding.button.setOnClickListener(clickListener) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 7b4ab8c9..158ecfae 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -18,7 +18,7 @@ An error occurred. - A network error occurred! Please check your connection and try again! + A network error occurred. Please check your connection and try again. This cannot be empty. Invalid domain entered Failed authenticating with that instance. If this persists, try "Login in Browser" from the menu.