From f2a2c164642078f996875d898e44f4cf9fd2b4b1 Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Sun, 15 Sep 2024 17:32:19 +0200 Subject: [PATCH] Better refreshing state handling for AccountViewModel (#4680) Closes #4353 by addressing the last edge case: The swipe to refresh won't be enabled before something is loaded, preventing multiple loads at the same time. After the first load, the swipe to refresh itself prevents multiple loads: It can be swiped only once. --------- Co-authored-by: Goooler --- .../components/account/AccountActivity.kt | 12 ++++++---- .../components/account/AccountViewModel.kt | 22 +++++++------------ 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/account/AccountActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/account/AccountActivity.kt index dc813fe01..ee2bd0e4c 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/account/AccountActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/account/AccountActivity.kt @@ -387,7 +387,10 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvide viewModel.accountData.collect { if (it == null) return@collect when (it) { - is Success -> onAccountChanged(it.data) + is Success -> { + onAccountChanged(it.data) + binding.swipeToRefreshLayout.isEnabled = true + } is Error -> { Snackbar.make( binding.accountCoordinatorLayout, @@ -396,6 +399,7 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvide ) .setAction(R.string.action_retry) { viewModel.refresh() } .show() + binding.swipeToRefreshLayout.isEnabled = true } is Loading -> { } } @@ -438,10 +442,11 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvide * Setup swipe to refresh layout */ private fun setupRefreshLayout() { + binding.swipeToRefreshLayout.isEnabled = false // will only be enabled after the first load completed binding.swipeToRefreshLayout.setOnRefreshListener { onRefresh() } lifecycleScope.launch { - viewModel.isRefreshing.collect { isRefreshing -> - binding.swipeToRefreshLayout.isRefreshing = isRefreshing == true + viewModel.isRefreshing.collect { + binding.swipeToRefreshLayout.isRefreshing = it } } } @@ -1030,7 +1035,6 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvide return true } R.id.action_refresh -> { - binding.swipeToRefreshLayout.isRefreshing = true onRefresh() return true } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/account/AccountViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/account/AccountViewModel.kt index 8716ee826..c9626da8b 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/account/AccountViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/account/AccountViewModel.kt @@ -22,13 +22,9 @@ import com.keylesspalace.tusky.util.getDomain import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.coroutines.Job -import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch @@ -48,10 +44,8 @@ class AccountViewModel @Inject constructor( private val _noteSaved = MutableStateFlow(false) val noteSaved: StateFlow = _noteSaved.asStateFlow() - private val _isRefreshing = MutableSharedFlow(1, onBufferOverflow = BufferOverflow.DROP_OLDEST) - val isRefreshing: SharedFlow = _isRefreshing.asSharedFlow() - - private var isDataLoading = false + private val _isRefreshing = MutableStateFlow(false) + val isRefreshing: StateFlow = _isRefreshing.asStateFlow() lateinit var accountId: String var isSelf = false @@ -78,7 +72,9 @@ class AccountViewModel @Inject constructor( private fun obtainAccount(reload: Boolean = false) { if (_accountData.value == null || reload) { - isDataLoading = true + if (reload) { + _isRefreshing.value = true + } _accountData.value = Loading() viewModelScope.launch { @@ -89,14 +85,12 @@ class AccountViewModel @Inject constructor( isFromOwnDomain = domain == activeAccount.domain _accountData.value = Success(account) - isDataLoading = false - _isRefreshing.emit(false) + _isRefreshing.value = false }, { t -> Log.w(TAG, "failed obtaining account", t) _accountData.value = Error(cause = t) - isDataLoading = false - _isRefreshing.emit(false) + _isRefreshing.value = false } ) } @@ -318,7 +312,7 @@ class AccountViewModel @Inject constructor( } private fun reload(isReload: Boolean = false) { - if (isDataLoading) { + if (_isRefreshing.value) { return } accountId.let {