From c650ca9362a6e23dacf49e250ba35ff09f088a94 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Mon, 9 Jan 2023 21:31:31 +0100 Subject: [PATCH] Improve the actual and perceived speed of thread loading (#3118) * Improve the actual and perceived speed of thread loading To improve the actual speed, note that if the user has opened a thread from their home timeline then the initial status is cached in the database. Other statuses in the same thread may be cached as well. So try and load the initial status from the database, falling back to the network if it's not present (e.g., the user has opened a thread from the local or federated timelines, or a search). Introduce a new loading state to deal with this case. In typical cases this allows the UI to display the initial status immediately with no need to show a progress indicator. To improve the perceived speed, delay showing the initial loading circular progress indicators by 500ms. If loading the initial status completes within that time no spinner is shown and the user will perceive the action as close-to-immediate (https://www.nngroup.com/articles/response-times-3-important-limits/). Additionally, introduce an extra indeterminate progress indicator. The new indicator is linear, anchored to the bottom of the screen, and shows progress loading ancestor/descendant statuses. Like the other indicator is also delayed 500ms from when ancestor/descendant status information is fetched, and if the fetch completes in that time it will not be shown. * Mark `getStatus` as suspend so it doesn't run on the main thread * Save an allocation, use an isDetailed parameter to TimelineStatusWithAccount.toViewData Rename Status.toViewData's "detailed" parameter to "isDetailed" for consistency with other uses. * Ensure suspend functions run to completion when testing * Delay-load the status from the network even if it's cached This speeds up the UI while ensuring it will eventually contain accurate data from the remote. * Load the network status before updating the list Avoids excess animations if the network copy has changes * Fix UI flicker when loading reblogged statuses * Lint * Fixup tests --- .../timeline/TimelineTypeMappers.kt | 5 +- .../components/viewthread/ThreadAdapter.kt | 1 + .../viewthread/ViewThreadFragment.kt | 79 +++++++-- .../viewthread/ViewThreadViewModel.kt | 161 +++++++++++++----- .../viewthread/edits/ViewEditsFragment.kt | 6 +- .../com/keylesspalace/tusky/db/TimelineDao.kt | 22 +++ .../layout-sw640dp/fragment_view_thread.xml | 29 +++- .../main/res/layout/fragment_view_thread.xml | 30 +++- app/src/main/res/values/strings.xml | 1 + .../viewthread/ViewThreadViewModelTest.kt | 101 ++++++++--- 10 files changed, 328 insertions(+), 107 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineTypeMappers.kt b/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineTypeMappers.kt index a02e9966c..735f0492e 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineTypeMappers.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineTypeMappers.kt @@ -149,7 +149,7 @@ fun Status.toEntity( ) } -fun TimelineStatusWithAccount.toViewData(gson: Gson): StatusViewData { +fun TimelineStatusWithAccount.toViewData(gson: Gson, isDetailed: Boolean = false): StatusViewData { if (this.status.authorServerId == null) { return StatusViewData.Placeholder(this.status.serverId, this.status.expanded) } @@ -261,6 +261,7 @@ fun TimelineStatusWithAccount.toViewData(gson: Gson): StatusViewData { status = status, isExpanded = this.status.expanded, isShowingContent = this.status.contentShowing, - isCollapsed = this.status.contentCollapsed + isCollapsed = this.status.contentCollapsed, + isDetailed = isDetailed ) } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ThreadAdapter.kt b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ThreadAdapter.kt index 9e0903b06..7f900de6c 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ThreadAdapter.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ThreadAdapter.kt @@ -62,6 +62,7 @@ class ThreadAdapter( } companion object { + private const val TAG = "ThreadAdapter" private const val VIEW_TYPE_STATUS = 0 private const val VIEW_TYPE_STATUS_DETAILED = 1 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 d90c976ba..f524e932b 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 @@ -21,6 +21,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.widget.LinearLayout +import androidx.annotation.CheckResult import androidx.fragment.app.commit import androidx.fragment.app.viewModels import androidx.lifecycle.lifecycleScope @@ -50,6 +51,9 @@ import com.keylesspalace.tusky.util.show import com.keylesspalace.tusky.util.viewBinding import com.keylesspalace.tusky.viewdata.AttachmentViewData.Companion.list import com.keylesspalace.tusky.viewdata.StatusViewData +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.delay import kotlinx.coroutines.launch import java.io.IOException import javax.inject.Inject @@ -142,24 +146,50 @@ class ViewThreadFragment : SFragment(), OnRefreshListener, StatusActionListener, (binding.recyclerView.itemAnimator as SimpleItemAnimator).supportsChangeAnimations = false + var initialProgressBar = getProgressBarJob(binding.initialProgressBar, 500) + var threadProgressBar = getProgressBarJob(binding.threadProgressBar, 500) + viewLifecycleOwner.lifecycleScope.launch { viewModel.uiState.collect { uiState -> when (uiState) { is ThreadUiState.Loading -> { updateRevealButton(RevealButtonState.NO_BUTTON) + binding.recyclerView.hide() binding.statusView.hide() - binding.progressBar.show() + + initialProgressBar = getProgressBarJob(binding.initialProgressBar, 500) + initialProgressBar.start() + } + is ThreadUiState.LoadingThread -> { + if (uiState.statusViewDatum == null) { + // no detailed statuses available, e.g. because author is blocked + activity?.finish() + return@collect + } + + initialProgressBar.cancel() + threadProgressBar = getProgressBarJob(binding.threadProgressBar, 500) + threadProgressBar.start() + + adapter.submitList(listOf(uiState.statusViewDatum)) + + updateRevealButton(uiState.revealButton) + binding.swipeRefreshLayout.isRefreshing = false + + binding.recyclerView.show() + binding.statusView.hide() } is ThreadUiState.Error -> { Log.w(TAG, "failed to load status", uiState.throwable) + initialProgressBar.cancel() + threadProgressBar.cancel() updateRevealButton(RevealButtonState.NO_BUTTON) binding.swipeRefreshLayout.isRefreshing = false binding.recyclerView.hide() binding.statusView.show() - binding.progressBar.hide() if (uiState.throwable is IOException) { binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network) { @@ -172,28 +202,21 @@ class ViewThreadFragment : SFragment(), OnRefreshListener, StatusActionListener, } } is ThreadUiState.Success -> { - if (uiState.statuses.none { viewData -> viewData.isDetailed }) { - // no detailed statuses available, e.g. because author is blocked - activity?.finish() - return@collect - } + threadProgressBar.cancel() - adapter.submitList(uiState.statuses) { - if (viewModel.isInitialLoad) { - viewModel.isInitialLoad = false - val detailedPosition = adapter.currentList.indexOfFirst { viewData -> - viewData.isDetailed - } - binding.recyclerView.scrollToPosition(detailedPosition) - } + adapter.submitList(uiState.statusViewData) { + // Ensure the top of the status is visible + (binding.recyclerView.layoutManager as LinearLayoutManager).scrollToPositionWithOffset(uiState.detailedStatusPosition, 0) } updateRevealButton(uiState.revealButton) - binding.swipeRefreshLayout.isRefreshing = uiState.refreshing + binding.swipeRefreshLayout.isRefreshing = false binding.recyclerView.show() binding.statusView.hide() - binding.progressBar.hide() + } + is ThreadUiState.Refreshing -> { + threadProgressBar.cancel() } } } @@ -213,6 +236,28 @@ class ViewThreadFragment : SFragment(), OnRefreshListener, StatusActionListener, viewModel.loadThread(thisThreadsStatusId) } + /** + * Create a job to implement a delayed-visible progress bar. + * + * Delaying the visibility of the progress bar can improve user perception of UI speed because + * fewer UI elements are appearing and disappearing. + * + * When started the job will wait `delayMs` then show `view`. If the job is cancelled at + * any time `view` is hidden. + */ + @CheckResult() + private fun getProgressBarJob(view: View, delayMs: Long) = viewLifecycleOwner.lifecycleScope.launch( + start = CoroutineStart.LAZY + ) { + try { + delay(delayMs) + view.show() + awaitCancellation() + } finally { + view.hide() + } + } + private fun updateRevealButton(state: RevealButtonState) { val menuItem = binding.toolbar.menu.findItem(R.id.action_reveal) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadViewModel.kt index 91cdc62e9..b6ff76ffd 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadViewModel.kt @@ -20,6 +20,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import at.connyduck.calladapter.networkresult.fold import at.connyduck.calladapter.networkresult.getOrElse +import com.google.gson.Gson import com.keylesspalace.tusky.appstore.BlockEvent import com.keylesspalace.tusky.appstore.BookmarkEvent import com.keylesspalace.tusky.appstore.EventHub @@ -28,8 +29,10 @@ import com.keylesspalace.tusky.appstore.PinEvent import com.keylesspalace.tusky.appstore.ReblogEvent import com.keylesspalace.tusky.appstore.StatusComposedEvent import com.keylesspalace.tusky.appstore.StatusDeletedEvent +import com.keylesspalace.tusky.components.timeline.toViewData import com.keylesspalace.tusky.components.timeline.util.ifExpected import com.keylesspalace.tusky.db.AccountManager +import com.keylesspalace.tusky.db.AppDatabase import com.keylesspalace.tusky.entity.Filter import com.keylesspalace.tusky.entity.Status import com.keylesspalace.tusky.network.FilterModel @@ -54,7 +57,9 @@ class ViewThreadViewModel @Inject constructor( private val filterModel: FilterModel, private val timelineCases: TimelineCases, eventHub: EventHub, - accountManager: AccountManager + accountManager: AccountManager, + private val db: AppDatabase, + private val gson: Gson ) : ViewModel() { private val _uiState: MutableStateFlow = MutableStateFlow(ThreadUiState.Loading) @@ -65,8 +70,6 @@ class ViewThreadViewModel @Inject constructor( val errors: Flow get() = _errors - var isInitialLoad: Boolean = true - private val alwaysShowSensitiveMedia: Boolean private val alwaysOpenSpoiler: Boolean @@ -95,36 +98,70 @@ class ViewThreadViewModel @Inject constructor( } fun loadThread(id: String) { + _uiState.value = ThreadUiState.Loading + viewModelScope.launch { + Log.d(TAG, "Finding status with: $id") val contextCall = async { api.statusContext(id) } - val statusCall = async { api.statusAsync(id) } + val timelineStatus = db.timelineDao().getStatus(id) + + var detailedStatus = if (timelineStatus != null) { + Log.d(TAG, "Loaded status from local timeline") + val viewData = timelineStatus.toViewData( + gson, + isDetailed = true + ) as StatusViewData.Concrete + + // Return the correct status, depending on which one matched. If you do not do + // this the status IDs will be different between the status that's displayed with + // ThreadUiState.LoadingThread and ThreadUiState.Success, even though the apparent + // status content is the same. Then the status flickers as it is drawn twice. + if (viewData.actionableId == id) { + viewData.actionable.toViewData(isDetailed = true) + } else { + viewData + } + } else { + Log.d(TAG, "Loaded status from network") + val result = api.status(id).getOrElse { exception -> + _uiState.value = ThreadUiState.Error(exception) + return@launch + } + result.toViewData(isDetailed = true) + } - val contextResult = contextCall.await() - val statusResult = statusCall.await() + _uiState.value = ThreadUiState.LoadingThread( + statusViewDatum = detailedStatus, + revealButton = detailedStatus.getRevealButtonState() + ) - val status = statusResult.getOrElse { exception -> - _uiState.value = ThreadUiState.Error(exception) - return@launch + // If the detailedStatus was loaded from the database it might be out-of-date + // compared to the remote one. Now the user has a working UI do a background fetch + // for the status. Ignore errors, the user still has a functioning UI if the fetch + // failed. + if (timelineStatus != null) { + val viewData = api.status(id).getOrNull()?.toViewData(isDetailed = true) + if (viewData != null) { detailedStatus = viewData } } - contextResult.fold({ statusContext -> + val contextResult = contextCall.await() + contextResult.fold({ statusContext -> val ancestors = statusContext.ancestors.map { status -> status.toViewData() }.filter() - val detailedStatus = status.toViewData(true) val descendants = statusContext.descendants.map { status -> status.toViewData() }.filter() val statuses = ancestors + detailedStatus + descendants _uiState.value = ThreadUiState.Success( - statuses = statuses, - revealButton = statuses.getRevealButtonState(), - refreshing = false + statusViewData = statuses, + detailedStatusPosition = ancestors.size, + revealButton = statuses.getRevealButtonState() ) }, { throwable -> _errors.emit(throwable) _uiState.value = ThreadUiState.Success( - statuses = listOf(status.toViewData(true)), + statusViewData = listOf(detailedStatus), + detailedStatusPosition = 0, revealButton = RevealButtonState.NO_BUTTON, - refreshing = false ) }) } @@ -136,14 +173,12 @@ class ViewThreadViewModel @Inject constructor( } fun refresh(id: String) { - updateSuccess { uiState -> - uiState.copy(refreshing = true) - } + _uiState.value = ThreadUiState.Refreshing loadThread(id) } fun detailedStatus(): StatusViewData.Concrete? { - return (_uiState.value as ThreadUiState.Success?)?.statuses?.find { status -> + return (_uiState.value as ThreadUiState.Success?)?.statusViewData?.find { status -> status.isDetailed } } @@ -201,14 +236,14 @@ class ViewThreadViewModel @Inject constructor( fun removeStatus(statusToRemove: StatusViewData.Concrete) { updateSuccess { uiState -> uiState.copy( - statuses = uiState.statuses.filterNot { status -> status == statusToRemove } + statusViewData = uiState.statusViewData.filterNot { status -> status == statusToRemove } ) } } fun changeExpanded(expanded: Boolean, status: StatusViewData.Concrete) { updateSuccess { uiState -> - val statuses = uiState.statuses.map { viewData -> + val statuses = uiState.statusViewData.map { viewData -> if (viewData.id == status.id) { viewData.copy(isExpanded = expanded) } else { @@ -216,7 +251,7 @@ class ViewThreadViewModel @Inject constructor( } } uiState.copy( - statuses = statuses, + statusViewData = statuses, revealButton = statuses.getRevealButtonState() ) } @@ -261,7 +296,7 @@ class ViewThreadViewModel @Inject constructor( private fun removeAllByAccountId(accountId: String) { updateSuccess { uiState -> uiState.copy( - statuses = uiState.statuses.filter { viewData -> + statusViewData = uiState.statusViewData.filter { viewData -> viewData.status.account.id != accountId } ) @@ -271,7 +306,7 @@ class ViewThreadViewModel @Inject constructor( private fun handleStatusComposedEvent(event: StatusComposedEvent) { val eventStatus = event.status updateSuccess { uiState -> - val statuses = uiState.statuses + val statuses = uiState.statusViewData val detailedIndex = statuses.indexOfFirst { status -> status.isDetailed } val repliedIndex = statuses.indexOfFirst { status -> eventStatus.inReplyToId == status.id } if (detailedIndex != -1 && repliedIndex >= detailedIndex) { @@ -279,7 +314,7 @@ class ViewThreadViewModel @Inject constructor( val newStatuses = statuses.subList(0, repliedIndex + 1) + eventStatus.toViewData() + statuses.subList(repliedIndex + 1, statuses.size) - uiState.copy(statuses = newStatuses) + uiState.copy(statusViewData = newStatuses) } else { uiState } @@ -289,7 +324,7 @@ class ViewThreadViewModel @Inject constructor( private fun handleStatusDeletedEvent(event: StatusDeletedEvent) { updateSuccess { uiState -> uiState.copy( - statuses = uiState.statuses.filter { status -> + statusViewData = uiState.statusViewData.filter { status -> status.id != event.statusId } ) @@ -300,13 +335,13 @@ class ViewThreadViewModel @Inject constructor( updateSuccess { uiState -> when (uiState.revealButton) { RevealButtonState.HIDE -> uiState.copy( - statuses = uiState.statuses.map { viewData -> + statusViewData = uiState.statusViewData.map { viewData -> viewData.copy(isExpanded = false) }, revealButton = RevealButtonState.REVEAL ) RevealButtonState.REVEAL -> uiState.copy( - statuses = uiState.statuses.map { viewData -> + statusViewData = uiState.statusViewData.map { viewData -> viewData.copy(isExpanded = true) }, revealButton = RevealButtonState.HIDE @@ -316,16 +351,11 @@ class ViewThreadViewModel @Inject constructor( } } - private fun List.getRevealButtonState(): RevealButtonState { - val hasWarnings = any { viewData -> - viewData.status.spoilerText.isNotEmpty() - } + private fun StatusViewData.Concrete.getRevealButtonState(): RevealButtonState { + val hasWarnings = status.spoilerText.isNotEmpty() return if (hasWarnings) { - val allExpanded = none { viewData -> - !viewData.isExpanded - } - if (allExpanded) { + if (isExpanded) { RevealButtonState.HIDE } else { RevealButtonState.REVEAL @@ -335,6 +365,31 @@ class ViewThreadViewModel @Inject constructor( } } + /** + * Get the reveal button state based on the state of all the statuses in the list. + * + * - If any status sets it to REVEAL, use REVEAL + * - If no status sets it to REVEAL, but at least one uses HIDE, use HIDE + * - Otherwise use NO_BUTTON + */ + private fun List.getRevealButtonState(): RevealButtonState { + var seenHide = false + + forEach { + when (val state = it.getRevealButtonState()) { + RevealButtonState.NO_BUTTON -> return@forEach + RevealButtonState.REVEAL -> return state + RevealButtonState.HIDE -> seenHide = true + } + } + + if (seenHide) { + return RevealButtonState.HIDE + } + + return RevealButtonState.NO_BUTTON + } + private fun loadFilters() { viewModelScope.launch { val filters = api.getFilters().getOrElse { @@ -349,9 +404,9 @@ class ViewThreadViewModel @Inject constructor( ) updateSuccess { uiState -> - val statuses = uiState.statuses.filter() + val statuses = uiState.statusViewData.filter() uiState.copy( - statuses = statuses, + statusViewData = statuses, revealButton = statuses.getRevealButtonState() ) } @@ -365,14 +420,14 @@ class ViewThreadViewModel @Inject constructor( } private fun Status.toViewData( - detailed: Boolean = false + isDetailed: Boolean = false ): StatusViewData.Concrete { - val oldStatus = (_uiState.value as? ThreadUiState.Success)?.statuses?.find { it.id == this.id } + val oldStatus = (_uiState.value as? ThreadUiState.Success)?.statusViewData?.find { it.id == this.id } return toViewData( isShowingContent = oldStatus?.isShowingContent ?: (alwaysShowSensitiveMedia || !actionableStatus.sensitive), isExpanded = oldStatus?.isExpanded ?: alwaysOpenSpoiler, - isCollapsed = oldStatus?.isCollapsed ?: !detailed, - isDetailed = oldStatus?.isDetailed ?: detailed + isCollapsed = oldStatus?.isCollapsed ?: !isDetailed, + isDetailed = oldStatus?.isDetailed ?: isDetailed ) } @@ -389,7 +444,7 @@ class ViewThreadViewModel @Inject constructor( private fun updateStatusViewData(statusId: String, updater: (StatusViewData.Concrete) -> StatusViewData.Concrete) { updateSuccess { uiState -> uiState.copy( - statuses = uiState.statuses.map { viewData -> + statusViewData = uiState.statusViewData.map { viewData -> if (viewData.id == statusId) { updater(viewData) } else { @@ -414,13 +469,27 @@ class ViewThreadViewModel @Inject constructor( } sealed interface ThreadUiState { + /** The initial load of the detailed status for this thread */ object Loading : ThreadUiState + + /** Loading the detailed status has completed, now loading ancestors/descendants */ + data class LoadingThread( + val statusViewDatum: StatusViewData.Concrete?, + val revealButton: RevealButtonState + ) : ThreadUiState + + /** An error occurred at any point */ class Error(val throwable: Throwable) : ThreadUiState + + /** Successfully loaded the full thread */ data class Success( - val statuses: List, + val statusViewData: List, val revealButton: RevealButtonState, - val refreshing: Boolean + val detailedStatusPosition: Int ) : ThreadUiState + + /** Refreshing the thread with a swipe */ + object Refreshing : ThreadUiState } enum class RevealButtonState { 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 d1487fefe..d02d017d5 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 @@ -82,14 +82,14 @@ class ViewEditsFragment : Fragment(R.layout.fragment_view_thread), LinkListener, EditsUiState.Loading -> { binding.recyclerView.hide() binding.statusView.hide() - binding.progressBar.show() + binding.initialProgressBar.show() } is EditsUiState.Error -> { Log.w(TAG, "failed to load edits", uiState.throwable) binding.recyclerView.hide() binding.statusView.show() - binding.progressBar.hide() + binding.initialProgressBar.hide() if (uiState.throwable is IOException) { binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network) { @@ -104,7 +104,7 @@ class ViewEditsFragment : Fragment(R.layout.fragment_view_thread), LinkListener, is EditsUiState.Success -> { binding.recyclerView.show() binding.statusView.hide() - binding.progressBar.hide() + binding.initialProgressBar.hide() binding.recyclerView.adapter = ViewEditsAdapter( edits = uiState.edits, diff --git a/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt b/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt index 3b4ba16bc..3387c8a54 100644 --- a/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt +++ b/app/src/main/java/com/keylesspalace/tusky/db/TimelineDao.kt @@ -53,6 +53,28 @@ ORDER BY LENGTH(s.serverId) DESC, s.serverId DESC""" ) abstract fun getStatuses(account: Long): PagingSource + @Query( + """ +SELECT s.serverId, s.url, s.timelineUserId, +s.authorServerId, s.inReplyToId, s.inReplyToAccountId, s.createdAt, s.editedAt, +s.emojis, s.reblogsCount, s.favouritesCount, s.repliesCount, s.reblogged, s.favourited, s.bookmarked, s.sensitive, +s.spoilerText, s.visibility, s.mentions, s.tags, s.application, s.reblogServerId,s.reblogAccountId, +s.content, s.attachments, s.poll, s.card, s.muted, s.expanded, s.contentShowing, s.contentCollapsed, s.pinned, s.language, +a.serverId as 'a_serverId', a.timelineUserId as 'a_timelineUserId', +a.localUsername as 'a_localUsername', a.username as 'a_username', +a.displayName as 'a_displayName', a.url as 'a_url', a.avatar as 'a_avatar', +a.emojis as 'a_emojis', a.bot as 'a_bot', +rb.serverId as 'rb_serverId', rb.timelineUserId 'rb_timelineUserId', +rb.localUsername as 'rb_localUsername', rb.username as 'rb_username', +rb.displayName as 'rb_displayName', rb.url as 'rb_url', rb.avatar as 'rb_avatar', +rb.emojis as 'rb_emojis', rb.bot as 'rb_bot' +FROM TimelineStatusEntity s +LEFT JOIN TimelineAccountEntity a ON (s.timelineUserId = a.timelineUserId AND s.authorServerId = a.serverId) +LEFT JOIN TimelineAccountEntity rb ON (s.timelineUserId = rb.timelineUserId AND s.reblogAccountId = rb.serverId) +WHERE s.serverId = :statusId OR s.reblogServerId = :statusId""" + ) + abstract suspend fun getStatus(statusId: String): TimelineStatusWithAccount? + @Query( """DELETE FROM TimelineStatusEntity WHERE timelineUserId = :accountId AND (LENGTH(serverId) < LENGTH(:maxId) OR LENGTH(serverId) == LENGTH(:maxId) AND serverId <= :maxId) diff --git a/app/src/main/res/layout-sw640dp/fragment_view_thread.xml b/app/src/main/res/layout-sw640dp/fragment_view_thread.xml index 69163b8c7..9248c542b 100644 --- a/app/src/main/res/layout-sw640dp/fragment_view_thread.xml +++ b/app/src/main/res/layout-sw640dp/fragment_view_thread.xml @@ -27,20 +27,37 @@ android:layout_gravity="center_horizontal|top" app:layout_behavior="com.google.android.material.appbar.AppBarLayout$ScrollingViewBehavior"> - + android:orientation="vertical"> + + + + + android:visibility="gone" + android:indeterminate="true" + android:layout_gravity="center" + android:contentDescription="@string/a11y_label_loading_thread" /> - + android:orientation="vertical"> + + + + + android:visibility="gone" + android:indeterminate="true" + android:layout_gravity="center" + android:contentDescription="@string/a11y_label_loading_thread" /> %1$s created %2$s + Loading thread diff --git a/app/src/test/java/com/keylesspalace/tusky/components/viewthread/ViewThreadViewModelTest.kt b/app/src/test/java/com/keylesspalace/tusky/components/viewthread/ViewThreadViewModelTest.kt index e1d690a14..d48322b2c 100644 --- a/app/src/test/java/com/keylesspalace/tusky/components/viewthread/ViewThreadViewModelTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/components/viewthread/ViewThreadViewModelTest.kt @@ -1,8 +1,12 @@ package com.keylesspalace.tusky.components.viewthread import android.os.Looper.getMainLooper +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.room.Room import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry import at.connyduck.calladapter.networkresult.NetworkResult +import com.google.gson.Gson import com.keylesspalace.tusky.appstore.BookmarkEvent import com.keylesspalace.tusky.appstore.EventHub import com.keylesspalace.tusky.appstore.FavoriteEvent @@ -11,14 +15,18 @@ import com.keylesspalace.tusky.components.timeline.mockStatus import com.keylesspalace.tusky.components.timeline.mockStatusViewData import com.keylesspalace.tusky.db.AccountEntity import com.keylesspalace.tusky.db.AccountManager +import com.keylesspalace.tusky.db.AppDatabase +import com.keylesspalace.tusky.db.Converters import com.keylesspalace.tusky.entity.StatusContext import com.keylesspalace.tusky.network.FilterModel import com.keylesspalace.tusky.network.MastodonApi import com.keylesspalace.tusky.usecase.TimelineCases import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking +import org.junit.After import org.junit.Assert.assertEquals import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.kotlin.doReturn @@ -35,9 +43,38 @@ class ViewThreadViewModelTest { private lateinit var api: MastodonApi private lateinit var eventHub: EventHub private lateinit var viewModel: ViewThreadViewModel + private lateinit var db: AppDatabase private val threadId = "1234" + /** + * Execute each task synchronously. + * + * If you do not do this, and you have code like this under test: + * + * ``` + * fun someFunc() = viewModelScope.launch { + * _uiState.value = "initial value" + * // ... + * call_a_suspend_fun() + * // ... + * _uiState.value = "new value" + * } + * ``` + * + * and a test like: + * + * ``` + * someFunc() + * assertEquals("new value", viewModel.uiState.value) + * ``` + * + * The test will fail, because someFunc() yields at the `call_a_suspend_func()` point, + * and control returns to the test before `_uiState.value` has been changed. + */ + @get:Rule + val instantTaskRule = InstantTaskExecutorRule() + @Before fun setup() { shadowOf(getMainLooper()).idle() @@ -56,7 +93,19 @@ class ViewThreadViewModelTest { isActive = true ) } - viewModel = ViewThreadViewModel(api, filterModel, timelineCases, eventHub, accountManager) + val context = InstrumentationRegistry.getInstrumentation().targetContext + db = Room.inMemoryDatabaseBuilder(context, AppDatabase::class.java) + .addTypeConverter(Converters(Gson())) + .allowMainThreadQueries() + .build() + + val gson = Gson() + viewModel = ViewThreadViewModel(api, filterModel, timelineCases, eventHub, accountManager, db, gson) + } + + @After + fun closeDb() { + db.close() } @Test @@ -68,13 +117,13 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "1", spoilerText = "Test"), mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true, spoilerText = "Test"), mockStatusViewData(id = "3", inReplyToId = "2", inReplyToAccountId = "1", spoilerText = "Test") ), - revealButton = RevealButtonState.REVEAL, - refreshing = false + detailedStatusPosition = 1, + revealButton = RevealButtonState.REVEAL ), viewModel.uiState.first() ) @@ -84,7 +133,7 @@ class ViewThreadViewModelTest { @Test fun `should emit status even if context fails to load`() { api.stub { - onBlocking { statusAsync(threadId) } doReturn NetworkResult.success(mockStatus(id = "2", inReplyToId = "1", inReplyToAccountId = "1")) + onBlocking { status(threadId) } doReturn NetworkResult.success(mockStatus(id = "2", inReplyToId = "1", inReplyToAccountId = "1")) onBlocking { statusContext(threadId) } doReturn NetworkResult.failure(IOException()) } @@ -93,11 +142,11 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true) ), + detailedStatusPosition = 0, revealButton = RevealButtonState.NO_BUTTON, - refreshing = false ), viewModel.uiState.first() ) @@ -107,7 +156,7 @@ class ViewThreadViewModelTest { @Test fun `should emit error when status and context fail to load`() { api.stub { - onBlocking { statusAsync(threadId) } doReturn NetworkResult.failure(IOException()) + onBlocking { status(threadId) } doReturn NetworkResult.failure(IOException()) onBlocking { statusContext(threadId) } doReturn NetworkResult.failure(IOException()) } @@ -124,7 +173,7 @@ class ViewThreadViewModelTest { @Test fun `should emit error when status fails to load`() { api.stub { - onBlocking { statusAsync(threadId) } doReturn NetworkResult.failure(IOException()) + onBlocking { status(threadId) } doReturn NetworkResult.failure(IOException()) onBlocking { statusContext(threadId) } doReturn NetworkResult.success( StatusContext( ancestors = listOf(mockStatus(id = "1")), @@ -153,13 +202,13 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "1", spoilerText = "Test", isExpanded = true), mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true, spoilerText = "Test", isExpanded = true), mockStatusViewData(id = "3", inReplyToId = "2", inReplyToAccountId = "1", spoilerText = "Test", isExpanded = true) ), + detailedStatusPosition = 1, revealButton = RevealButtonState.HIDE, - refreshing = false ), viewModel.uiState.first() ) @@ -177,13 +226,13 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "1", spoilerText = "Test", favourited = false), mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true, spoilerText = "Test"), mockStatusViewData(id = "3", inReplyToId = "2", inReplyToAccountId = "1", spoilerText = "Test") ), + detailedStatusPosition = 1, revealButton = RevealButtonState.REVEAL, - refreshing = false ), viewModel.uiState.first() ) @@ -201,13 +250,13 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "1", spoilerText = "Test"), mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true, spoilerText = "Test", reblogged = true), mockStatusViewData(id = "3", inReplyToId = "2", inReplyToAccountId = "1", spoilerText = "Test") ), + detailedStatusPosition = 1, revealButton = RevealButtonState.REVEAL, - refreshing = false ), viewModel.uiState.first() ) @@ -225,13 +274,13 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "1", spoilerText = "Test"), mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true, spoilerText = "Test"), mockStatusViewData(id = "3", inReplyToId = "2", inReplyToAccountId = "1", spoilerText = "Test", bookmarked = false) ), + detailedStatusPosition = 1, revealButton = RevealButtonState.REVEAL, - refreshing = false ), viewModel.uiState.first() ) @@ -249,12 +298,12 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "1", spoilerText = "Test"), mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true, spoilerText = "Test") ), + detailedStatusPosition = 1, revealButton = RevealButtonState.REVEAL, - refreshing = false ), viewModel.uiState.first() ) @@ -275,13 +324,13 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "1", spoilerText = "Test"), mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true, spoilerText = "Test", isExpanded = true), mockStatusViewData(id = "3", inReplyToId = "2", inReplyToAccountId = "1", spoilerText = "Test") ), + detailedStatusPosition = 1, revealButton = RevealButtonState.REVEAL, - refreshing = false ), viewModel.uiState.first() ) @@ -302,13 +351,13 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "1", spoilerText = "Test"), mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true, spoilerText = "Test", isCollapsed = true), mockStatusViewData(id = "3", inReplyToId = "2", inReplyToAccountId = "1", spoilerText = "Test") ), + detailedStatusPosition = 1, revealButton = RevealButtonState.REVEAL, - refreshing = false ), viewModel.uiState.first() ) @@ -329,13 +378,13 @@ class ViewThreadViewModelTest { runBlocking { assertEquals( ThreadUiState.Success( - statuses = listOf( + statusViewData = listOf( mockStatusViewData(id = "1", spoilerText = "Test"), mockStatusViewData(id = "2", inReplyToId = "1", inReplyToAccountId = "1", isDetailed = true, spoilerText = "Test", isShowingContent = true), mockStatusViewData(id = "3", inReplyToId = "2", inReplyToAccountId = "1", spoilerText = "Test") ), + detailedStatusPosition = 1, revealButton = RevealButtonState.REVEAL, - refreshing = false ), viewModel.uiState.first() ) @@ -344,7 +393,7 @@ class ViewThreadViewModelTest { private fun mockSuccessResponses() { api.stub { - onBlocking { statusAsync(threadId) } doReturn NetworkResult.success(mockStatus(id = "2", inReplyToId = "1", inReplyToAccountId = "1", spoilerText = "Test")) + onBlocking { status(threadId) } doReturn NetworkResult.success(mockStatus(id = "2", inReplyToId = "1", inReplyToAccountId = "1", spoilerText = "Test")) onBlocking { statusContext(threadId) } doReturn NetworkResult.success( StatusContext( ancestors = listOf(mockStatus(id = "1", spoilerText = "Test")),