From 23381d45d7f4009cd754e528ecf07175d2403183 Mon Sep 17 00:00:00 2001 From: UlrichKu Date: Thu, 30 Mar 2023 19:29:42 +0200 Subject: [PATCH] 3430: Make list refresh/retry consistent (#3474) * 3430: Make list refresh/retry consistent * 3430: Add swipe-to-refresh and use states in filter lists --- .../com/keylesspalace/tusky/ListsActivity.kt | 4 ++ .../accountlist/AccountListFragment.kt | 8 ++- .../conversation/ConversationsFragment.kt | 8 ++- .../components/filters/FiltersActivity.kt | 67 +++++++++++-------- .../components/filters/FiltersViewModel.kt | 29 +++++--- .../components/timeline/TimelineFragment.kt | 8 ++- app/src/main/res/layout/activity_filters.xml | 25 ++++--- app/src/main/res/layout/activity_lists.xml | 12 +++- .../main/res/layout/fragment_account_list.xml | 15 +++-- 9 files changed, 117 insertions(+), 59 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/ListsActivity.kt b/app/src/main/java/com/keylesspalace/tusky/ListsActivity.kt index e5c043d7a..ee0187986 100644 --- a/app/src/main/java/com/keylesspalace/tusky/ListsActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/ListsActivity.kt @@ -101,6 +101,9 @@ class ListsActivity : BaseActivity(), Injectable, HasAndroidInjector { DividerItemDecoration(this, DividerItemDecoration.VERTICAL) ) + binding.swipeRefreshLayout.setOnRefreshListener { viewModel.retryLoading() } + binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue) + lifecycleScope.launch { viewModel.state.collect(this@ListsActivity::update) } @@ -166,6 +169,7 @@ class ListsActivity : BaseActivity(), Injectable, HasAndroidInjector { private fun update(state: ListsViewModel.State) { adapter.submitList(state.lists) binding.progressBar.visible(state.loadingState == LOADING) + binding.swipeRefreshLayout.isRefreshing = state.loadingState == LOADING when (state.loadingState) { INITIAL, LOADING -> binding.messageView.hide() ERROR_NETWORK -> { 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 d18ae4fd5..552744586 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 @@ -89,9 +89,11 @@ class AccountListFragment : Fragment(R.layout.fragment_account_list), AccountAct val layoutManager = LinearLayoutManager(view.context) binding.recyclerView.layoutManager = layoutManager (binding.recyclerView.itemAnimator as SimpleItemAnimator).supportsChangeAnimations = false - binding.recyclerView.addItemDecoration(DividerItemDecoration(view.context, DividerItemDecoration.VERTICAL)) + binding.swipeRefreshLayout.setOnRefreshListener { fetchAccounts() } + binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue) + val pm = PreferenceManager.getDefaultSharedPreferences(view.context) val animateAvatar = pm.getBoolean(PrefKeys.ANIMATE_GIF_AVATARS, false) val animateEmojis = pm.getBoolean(PrefKeys.ANIMATE_CUSTOM_EMOJIS, false) @@ -287,6 +289,7 @@ class AccountListFragment : Fragment(R.layout.fragment_account_list), AccountAct return } fetching = true + binding.swipeRefreshLayout.isRefreshing = true if (fromId != null) { binding.recyclerView.post { adapter.setBottomLoading(true) } @@ -295,6 +298,7 @@ class AccountListFragment : Fragment(R.layout.fragment_account_list), AccountAct viewLifecycleOwner.lifecycleScope.launch { try { val response = getFetchCallByListType(fromId) + if (!response.isSuccessful) { onFetchAccountsFailure(Exception(response.message())) return@launch @@ -317,6 +321,7 @@ class AccountListFragment : Fragment(R.layout.fragment_account_list), AccountAct private fun onFetchAccountsSuccess(accounts: List, linkHeader: String?) { adapter.setBottomLoading(false) + binding.swipeRefreshLayout.isRefreshing = false val links = HttpHeaderLink.parse(linkHeader) val next = HttpHeaderLink.findByRelationType(links, "next") @@ -366,6 +371,7 @@ class AccountListFragment : Fragment(R.layout.fragment_account_list), AccountAct private fun onFetchAccountsFailure(throwable: Throwable) { fetching = false + binding.swipeRefreshLayout.isRefreshing = false Log.e(TAG, "Fetch failure", throwable) if (adapter.itemCount == 0) { 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 0f17a7486..303ec6b03 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 @@ -141,9 +141,13 @@ class ConversationsFragment : binding.statusView.show() if ((loadState.refresh as LoadState.Error).error is IOException) { - binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network, null) + binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network) { + refreshContent() + } } else { - binding.statusView.setup(R.drawable.elephant_error, R.string.error_generic, null) + binding.statusView.setup(R.drawable.elephant_error, R.string.error_generic) { + refreshContent() + } } } is LoadState.Loading -> { diff --git a/app/src/main/java/com/keylesspalace/tusky/components/filters/FiltersActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/filters/FiltersActivity.kt index e56a82535..b61a4faf4 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/filters/FiltersActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/filters/FiltersActivity.kt @@ -12,8 +12,8 @@ import com.keylesspalace.tusky.entity.Filter import com.keylesspalace.tusky.util.hide import com.keylesspalace.tusky.util.show import com.keylesspalace.tusky.util.viewBinding +import com.keylesspalace.tusky.util.visible import kotlinx.coroutines.launch -import java.io.IOException import javax.inject.Inject class FiltersActivity : BaseActivity(), FiltersListener { @@ -33,10 +33,14 @@ class FiltersActivity : BaseActivity(), FiltersListener { setDisplayHomeAsUpEnabled(true) setDisplayShowHomeEnabled(true) } + binding.addFilterButton.setOnClickListener { launchEditFilterActivity() } + binding.swipeRefreshLayout.setOnRefreshListener { loadFilters() } + binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue) + setTitle(R.string.pref_title_timeline_filters) } @@ -48,41 +52,46 @@ class FiltersActivity : BaseActivity(), FiltersListener { private fun observeViewModel() { lifecycleScope.launch { - viewModel.filters.collect { filters -> - binding.filtersView.show() - binding.addFilterButton.show() - binding.filterProgressBar.hide() - refreshFilterDisplay(filters) - } - } + viewModel.state.collect { state -> + binding.progressBar.visible(state.loadingState == FiltersViewModel.LoadingState.LOADING) + binding.swipeRefreshLayout.isRefreshing = state.loadingState == FiltersViewModel.LoadingState.LOADING + binding.addFilterButton.visible(state.loadingState == FiltersViewModel.LoadingState.LOADED) - lifecycleScope.launch { - viewModel.error.collect { error -> - if (error is IOException) { - binding.filterMessageView.setup( - R.drawable.elephant_offline, - R.string.error_network - ) { loadFilters() } - } else { - binding.filterMessageView.setup( - R.drawable.elephant_error, - R.string.error_generic - ) { loadFilters() } + when (state.loadingState) { + FiltersViewModel.LoadingState.INITIAL, FiltersViewModel.LoadingState.LOADING -> binding.messageView.hide() + FiltersViewModel.LoadingState.ERROR_NETWORK -> { + binding.messageView.setup(R.drawable.elephant_offline, R.string.error_network) { + loadFilters() + } + binding.messageView.show() + } + FiltersViewModel.LoadingState.ERROR_OTHER -> { + binding.messageView.setup(R.drawable.elephant_error, R.string.error_generic) { + loadFilters() + } + binding.messageView.show() + } + FiltersViewModel.LoadingState.LOADED -> { + if (state.filters.isEmpty()) { + binding.messageView.setup( + R.drawable.elephant_friend_empty, + R.string.message_empty, + null + ) + binding.messageView.show() + } else { + binding.messageView.hide() + binding.filtersList.adapter = FiltersAdapter(this@FiltersActivity, state.filters) + binding.filtersList.show() + } + } } } } } - private fun refreshFilterDisplay(filters: List) { - binding.filtersView.adapter = FiltersAdapter(this, filters) - } - private fun loadFilters() { - binding.filterMessageView.hide() - binding.filtersView.hide() - binding.addFilterButton.hide() - binding.filterProgressBar.show() - + binding.filtersList.hide() viewModel.load() } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/filters/FiltersViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/filters/FiltersViewModel.kt index f4791d145..e28d251b8 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/filters/FiltersViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/filters/FiltersViewModel.kt @@ -9,6 +9,7 @@ import com.keylesspalace.tusky.appstore.EventHub import com.keylesspalace.tusky.appstore.PreferenceChangedEvent import com.keylesspalace.tusky.entity.Filter import com.keylesspalace.tusky.network.MastodonApi +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch import retrofit2.HttpException @@ -18,27 +19,39 @@ class FiltersViewModel @Inject constructor( private val api: MastodonApi, private val eventHub: EventHub ) : ViewModel() { - val filters: MutableStateFlow> = MutableStateFlow(listOf()) - val error: MutableStateFlow = MutableStateFlow(null) + + enum class LoadingState { + INITIAL, LOADING, LOADED, ERROR_NETWORK, ERROR_OTHER + } + + data class State(val filters: List, val loadingState: LoadingState) + + val state: Flow get() = _state + private val _state = MutableStateFlow(State(emptyList(), LoadingState.INITIAL)) fun load() { + this@FiltersViewModel._state.value = _state.value.copy(loadingState = LoadingState.LOADING) + viewModelScope.launch { api.getFilters().fold( { filters -> - this@FiltersViewModel.filters.value = filters + this@FiltersViewModel._state.value = State(filters, LoadingState.LOADED) }, { throwable -> if (throwable is HttpException && throwable.code() == 404) { api.getFiltersV1().fold( { filters -> - this@FiltersViewModel.filters.value = filters.map { it.toFilter() } + this@FiltersViewModel._state.value = State(filters.map { it.toFilter() }, LoadingState.LOADED) }, { throwable -> - error.value = throwable + // TODO log errors (also below) + + this@FiltersViewModel._state.value = _state.value.copy(loadingState = LoadingState.ERROR_OTHER) } ) + this@FiltersViewModel._state.value = _state.value.copy(loadingState = LoadingState.ERROR_OTHER) } else { - error.value = throwable + this@FiltersViewModel._state.value = _state.value.copy(loadingState = LoadingState.ERROR_NETWORK) } } ) @@ -49,7 +62,7 @@ class FiltersViewModel @Inject constructor( viewModelScope.launch { api.deleteFilter(filter.id).fold( { - filters.value = filters.value.filter { it.id != filter.id } + this@FiltersViewModel._state.value = State(this@FiltersViewModel._state.value.filters.filter { it.id != filter.id }, LoadingState.LOADED) for (context in filter.context) { eventHub.dispatch(PreferenceChangedEvent(context)) } @@ -58,7 +71,7 @@ class FiltersViewModel @Inject constructor( if (throwable is HttpException && throwable.code() == 404) { api.deleteFilterV1(filter.id).fold( { - filters.value = filters.value.filter { it.id != filter.id } + this@FiltersViewModel._state.value = State(this@FiltersViewModel._state.value.filters.filter { it.id != filter.id }, LoadingState.LOADED) }, { Snackbar.make(parent, "Error deleting filter '${filter.title}'", Snackbar.LENGTH_SHORT).show() 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 d1e891d00..c5998f1d3 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 @@ -245,9 +245,13 @@ class TimelineFragment : binding.statusView.show() if ((loadState.refresh as LoadState.Error).error is IOException) { - binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network) + binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network) { + onRefresh() + } } else { - binding.statusView.setup(R.drawable.elephant_error, R.string.error_generic) + binding.statusView.setup(R.drawable.elephant_error, R.string.error_generic) { + onRefresh() + } } } is LoadState.Loading -> { diff --git a/app/src/main/res/layout/activity_filters.xml b/app/src/main/res/layout/activity_filters.xml index 4e5d9558b..a2ae06c18 100644 --- a/app/src/main/res/layout/activity_filters.xml +++ b/app/src/main/res/layout/activity_filters.xml @@ -10,21 +10,27 @@ android:id="@+id/includedToolbar" layout="@layout/toolbar_basic" /> - + android:layout_height="match_parent"> + + + @@ -36,8 +42,7 @@ android:layout_margin="16dp" android:contentDescription="@string/filter_addition_title" android:src="@drawable/ic_plus_24dp" - app:layout_anchor="@id/filtersView" + app:layout_anchor="@id/filtersList" app:layout_anchorGravity="bottom|end" /> - - \ No newline at end of file + diff --git a/app/src/main/res/layout/activity_lists.xml b/app/src/main/res/layout/activity_lists.xml index d1e3931c3..7e297c23c 100644 --- a/app/src/main/res/layout/activity_lists.xml +++ b/app/src/main/res/layout/activity_lists.xml @@ -13,14 +13,20 @@ android:id="@+id/includedToolbar" layout="@layout/toolbar_basic" /> - + app:layout_constraintTop_toBottomOf="@id/includedToolbar"> + + + - + android:layout_height="match_parent"> + + + + - \ No newline at end of file +