Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better refreshing state handling for AccountViewModel #4680

Merged
merged 11 commits into from
Sep 15, 2024
Merged

Better refreshing state handling for AccountViewModel #4680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4f4dab9
Revert "fix swipe-refresh spinner showing forever when refreshing Acc…
Goooler Apr 2, 2024
b932917
Set _isRefreshing to true first
Goooler Apr 2, 2024
2d44f3c
Remove redundant isDataLoading
Goooler Apr 2, 2024
8925269
Single source
Goooler Apr 2, 2024
195be77
Merge branch 'refs/heads/develop' into better-fix-for-is-refreshing
Goooler Apr 5, 2024
3d283bd
Use refreshing state
Goooler Apr 5, 2024
965dc2e
Update app/src/main/java/com/keylesspalace/tusky/components/account/A…
Goooler Apr 11, 2024
26d19e2
Merge remote-tracking branch 'refs/remotes/origin/develop' into bette…
Goooler Apr 11, 2024
6e48b6a
Revert "Use refreshing state"
Goooler Apr 11, 2024
7c38b14
Merge branch 'develop' into better-fix-for-is-refreshing
connyduck Sep 13, 2024
593ad4b
prevent refreshing before something actually loaded
connyduck Sep 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions app/src/main/java/com/keylesspalace/tusky/components/account/AccountActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -396,6 +399,7 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvide
)
.setAction(R.string.action_retry) { viewModel.refresh() }
.show()
binding.swipeToRefreshLayout.isEnabled = true
}
is Loading -> { }
}
Expand Down Expand Up @@ -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
}
}
}
Expand Down Expand Up @@ -1030,7 +1035,6 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, MenuProvide
return true
}
R.id.action_refresh -> {
binding.swipeToRefreshLayout.isRefreshing = true
onRefresh()
return true
}
Expand Down
22 changes: 8 additions & 14 deletions app/src/main/java/com/keylesspalace/tusky/components/account/AccountViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -48,10 +44,8 @@ class AccountViewModel @Inject constructor(
private val _noteSaved = MutableStateFlow(false)
val noteSaved: StateFlow<Boolean> = _noteSaved.asStateFlow()

private val _isRefreshing = MutableSharedFlow<Boolean>(1, onBufferOverflow = BufferOverflow.DROP_OLDEST)
val isRefreshing: SharedFlow<Boolean> = _isRefreshing.asSharedFlow()

private var isDataLoading = false
private val _isRefreshing = MutableStateFlow(false)
val isRefreshing: StateFlow<Boolean> = _isRefreshing.asStateFlow()

lateinit var accountId: String
var isSelf = false
Expand All @@ -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 {
Expand All @@ -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
}
)
}
Expand Down Expand Up @@ -318,7 +312,7 @@ class AccountViewModel @Inject constructor(
}

private fun reload(isReload: Boolean = false) {
if (isDataLoading) {
if (_isRefreshing.value) {
return
}
accountId.let {
Expand Down
Loading