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.

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

[Tags Feed] Lazy load tag rows #20790

Merged
merged 5 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,17 @@ package org.wordpress.android.ui.reader.repository
import com.android.volley.VolleyError
import com.wordpress.rest.RestRequest
import dagger.Reusable
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.suspendCancellableCoroutine
import kotlinx.coroutines.withContext
import org.json.JSONObject
import org.wordpress.android.WordPress.Companion.getRestClientUtilsV1_2
import org.wordpress.android.datasets.ReaderPostTable
import org.wordpress.android.datasets.ReaderTagTable
import org.wordpress.android.models.ReaderPostList
import org.wordpress.android.models.ReaderTag
import org.wordpress.android.models.ReaderTagType
import org.wordpress.android.modules.IO_THREAD
import org.wordpress.android.ui.reader.ReaderConstants
import org.wordpress.android.ui.reader.actions.ReaderActions
import org.wordpress.android.ui.reader.actions.ReaderActions.UpdateResultListener
Expand All @@ -23,20 +26,22 @@ import org.wordpress.android.util.LocaleManagerWrapper
import org.wordpress.android.util.UrlUtils
import java.util.Locale
import javax.inject.Inject
import javax.inject.Named
import kotlin.coroutines.resume
import kotlin.coroutines.resumeWithException

@Reusable
class ReaderPostRepository @Inject constructor(
private val localeManagerWrapper: LocaleManagerWrapper,
private val localSource: ReaderPostLocalSource,
@Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher,
) {
/**
* Fetches and returns the most recent posts for the passed tag, respecting the maxPosts limit.
* It always fetches the most recent posts, saves them to the local DB and returns the latest from that cache.
*/
suspend fun fetchNewerPostsForTag(tag: ReaderTag, maxPosts: Int = 10): ReaderPostList {
return suspendCancellableCoroutine { cont ->
suspend fun fetchNewerPostsForTag(tag: ReaderTag, maxPosts: Int = 10): ReaderPostList = withContext(ioDispatcher) {
suspendCancellableCoroutine { cont ->
val resultListener = UpdateResultListener { result ->
if (result == ReaderActions.UpdateResult.FAILED) {
cont.resumeWithException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class ReaderTagsFeedUiStateMapper @Inject constructor(
onPostCardClick: (TagsFeedPostItem) -> Unit,
onPostLikeClick: (TagsFeedPostItem) -> Unit,
onPostMoreMenuClick: () -> Unit,
onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit,
) = ReaderTagsFeedViewModel.TagFeedItem(
tagChip = ReaderTagsFeedViewModel.TagChip(
tag = tag,
Expand Down Expand Up @@ -52,13 +53,15 @@ class ReaderTagsFeedUiStateMapper @Inject constructor(
)
}
),
onItemEnteredView = onItemEnteredView,
)

fun mapErrorTagFeedItem(
tag: ReaderTag,
errorType: ReaderTagsFeedViewModel.ErrorType,
onTagClick: (ReaderTag) -> Unit,
onRetryClick: () -> Unit,
onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit,
): ReaderTagsFeedViewModel.TagFeedItem =
ReaderTagsFeedViewModel.TagFeedItem(
tagChip = ReaderTagsFeedViewModel.TagChip(
Expand All @@ -69,11 +72,13 @@ class ReaderTagsFeedUiStateMapper @Inject constructor(
type = errorType,
onRetryClick = onRetryClick,
),
onItemEnteredView = onItemEnteredView,
)

fun mapLoadingPostsUiState(
fun mapInitialPostsUiState(
tags: List<ReaderTag>,
onTagClick: (ReaderTag) -> Unit,
onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit,
): ReaderTagsFeedViewModel.UiState.Loaded =
ReaderTagsFeedViewModel.UiState.Loaded(
tags.map { tag ->
Expand All @@ -82,8 +87,23 @@ class ReaderTagsFeedUiStateMapper @Inject constructor(
tag = tag,
onTagClick = onTagClick,
),
postList = ReaderTagsFeedViewModel.PostList.Loading,
postList = ReaderTagsFeedViewModel.PostList.Initial,
onItemEnteredView = onItemEnteredView,
)
}
)

fun mapLoadingTagFeedItem(
tag: ReaderTag,
onTagClick: (ReaderTag) -> Unit,
onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit,
): ReaderTagsFeedViewModel.TagFeedItem =
ReaderTagsFeedViewModel.TagFeedItem(
tagChip = ReaderTagsFeedViewModel.TagChip(
tag = tag,
onTagClick = onTagClick,
),
postList = ReaderTagsFeedViewModel.PostList.Loading,
onItemEnteredView = onItemEnteredView,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,7 @@ class ReaderTagsFeedViewModel @Inject constructor(

// Initially add all tags to the list with the posts loading UI
_uiStateFlow.update {
readerTagsFeedUiStateMapper.mapLoadingPostsUiState(tags, ::onTagClick)
}
// Fetch all posts and update the posts loading UI to either loaded or error when the request finishes
launch {
tags.forEach {
fetchTag(it)
}
readerTagsFeedUiStateMapper.mapInitialPostsUiState(tags, ::onTagClick, ::onItemEnteredView)
}
}

Expand All @@ -98,74 +92,97 @@ class ReaderTagsFeedViewModel @Inject constructor(
/**
* Fetch posts for a single tag. This method will emit a new state to [uiStateFlow] for different [UiState]s:
* [UiState.Initial], [UiState.Loaded], [UiState.Loading], [UiState.Empty], but only for the tag being fetched.
*
* Can be used for retrying a failed fetch, for instance.
*/
@Suppress("SwallowedException")
private suspend fun fetchTag(tag: ReaderTag) {
val updatedLoadedData = getUpdatedLoadedData()
// At this point, all tag feed items already exist in the UI with the loading status.
// We need it's index to update it to either Loaded or Error when the request is finished.
val existingIndex = updatedLoadedData.indexOfFirst { it.tagChip.tag == tag }
// Remove the current row of this tag (which is loading). This will be used to later add an updated item with
// either Loaded or Error status, depending on the result of the request.
updatedLoadedData.removeAll { it.tagChip.tag == tag }
try {
// Set the tag to loading state
updateTagFeedItem(
readerTagsFeedUiStateMapper.mapLoadingTagFeedItem(
tag = tag,
onTagClick = ::onTagClick,
onItemEnteredView = ::onItemEnteredView,
)
)

val updatedItem: TagFeedItem = try {
// Fetch posts for tag
val posts = readerPostRepository.fetchNewerPostsForTag(tag)
if (posts.isNotEmpty()) {
updatedLoadedData.add(
existingIndex,
readerTagsFeedUiStateMapper.mapLoadedTagFeedItem(
tag = tag,
posts = posts,
onTagClick = ::onTagClick,
onSiteClick = ::onSiteClick,
onPostCardClick = ::onPostCardClick,
onPostLikeClick = ::onPostLikeClick,
onPostMoreMenuClick = ::onPostMoreMenuClick,
)
readerTagsFeedUiStateMapper.mapLoadedTagFeedItem(
tag = tag,
posts = posts,
onTagClick = ::onTagClick,
onSiteClick = ::onSiteClick,
onPostCardClick = ::onPostCardClick,
onPostLikeClick = ::onPostLikeClick,
onPostMoreMenuClick = ::onPostMoreMenuClick,
onItemEnteredView = ::onItemEnteredView,
)
} else {
updatedLoadedData.add(
existingIndex,
readerTagsFeedUiStateMapper.mapErrorTagFeedItem(
tag = tag,
errorType = ErrorType.NoContent,
onTagClick = ::onTagClick,
onRetryClick = ::onRetryClick,
)
)
}
} catch (e: ReaderPostFetchException) {
updatedLoadedData.add(
existingIndex,
readerTagsFeedUiStateMapper.mapErrorTagFeedItem(
tag = tag,
errorType = ErrorType.Default,
errorType = ErrorType.NoContent,
onTagClick = ::onTagClick,
onRetryClick = ::onRetryClick,
onItemEnteredView = ::onItemEnteredView,
)
}
} catch (e: ReaderPostFetchException) {
readerTagsFeedUiStateMapper.mapErrorTagFeedItem(
tag = tag,
errorType = ErrorType.Default,
onTagClick = ::onTagClick,
onRetryClick = ::onRetryClick,
onItemEnteredView = ::onItemEnteredView,
)
}
_uiStateFlow.update { UiState.Loaded(updatedLoadedData) }

updateTagFeedItem(updatedItem)
}

private fun getUpdatedLoadedData(): MutableList<TagFeedItem> {
private fun getLoadedData(uiState: UiState): MutableList<TagFeedItem> {
val updatedLoadedData = mutableListOf<TagFeedItem>()
val currentUiState = _uiStateFlow.value
if (currentUiState is UiState.Loaded) {
val currentLoadedData = currentUiState.data
updatedLoadedData.addAll(currentLoadedData)
if (uiState is UiState.Loaded) {
updatedLoadedData.addAll(uiState.data)
}
return updatedLoadedData
}

// Update the UI state for a single feed item, making sure to do it atomically so we don't lose any updates.
private fun updateTagFeedItem(updatedItem: TagFeedItem) {
_uiStateFlow.update { uiState ->
val updatedLoadedData = getLoadedData(uiState)

// At this point, all tag feed items already exist in the UI.
// We need it's index to update it and keep it in the same place.
updatedLoadedData.indexOfFirst { it.tagChip.tag == updatedItem.tagChip.tag }
.takeIf { it >= 0 }
?.let { existingIndex ->
// Update item
updatedLoadedData[existingIndex] = updatedItem
}

UiState.Loaded(updatedLoadedData)
}
}

@VisibleForTesting
fun onItemEnteredView(item: TagFeedItem) {
if (item.postList != PostList.Initial) {
// do nothing as it's still loading or already loaded
return
}

launch {
fetchTag(item.tagChip.tag)
}
}

private fun onOpenTagsListClick() {
// TODO
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
@VisibleForTesting
fun onTagClick(readerTag: ReaderTag) {
_actionEvents.value = ActionEvent.OpenTagPostsFeed(readerTag)
}
Expand All @@ -174,7 +191,7 @@ class ReaderTagsFeedViewModel @Inject constructor(
// TODO
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
@VisibleForTesting
fun onSiteClick(postItem: TagsFeedPostItem) {
launch {
findPost(postItem.postId, postItem.blogId)?.let {
Expand Down Expand Up @@ -334,28 +351,35 @@ class ReaderTagsFeedViewModel @Inject constructor(
}

sealed class UiState {
object Initial : UiState()
data object Initial : UiState()
data class Loaded(val data: List<TagFeedItem>) : UiState()

object Loading : UiState()
data object Loading : UiState()

data class Empty(val onOpenTagsListClick: () -> Unit) : UiState()
}

data class TagFeedItem(
val tagChip: TagChip,
val postList: PostList,
)
private val onItemEnteredView: (TagFeedItem) -> Unit = {},
) {
fun onEnteredView() {
onItemEnteredView(this)
}
}

data class TagChip(
val tag: ReaderTag,
val onTagClick: (ReaderTag) -> Unit,
)

sealed class PostList {
data object Initial : PostList()

data class Loaded(val items: List<TagsFeedPostItem>) : PostList()

object Loading : PostList()
data object Loading : PostList()

data class Error(
val type: ErrorType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import androidx.compose.material.ripple.rememberRipple
import androidx.compose.material3.Icon
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
Expand Down Expand Up @@ -84,7 +85,14 @@ private fun Loaded(uiState: UiState.Loaded) {
) {
items(
items = uiState.data,
) { (tagChip, postList) ->
) { item ->
val tagChip = item.tagChip
val postList = item.postList

LaunchedEffect(Unit) {
item.onEnteredView()
}

val backgroundColor = if (isSystemInDarkTheme()) {
AppColor.White.copy(alpha = 0.12F)
} else {
Expand All @@ -103,7 +111,7 @@ private fun Loaded(uiState: UiState.Loaded) {
Spacer(modifier = Modifier.height(Margin.Large.value))
// Posts list UI
when (postList) {
is PostList.Loading -> PostListLoading()
is PostList.Initial, is PostList.Loading -> PostListLoading()
is PostList.Loaded -> PostListLoaded(postList, tagChip, backgroundColor)
is PostList.Error -> PostListError(backgroundColor, tagChip, postList)
}
Expand Down Expand Up @@ -531,7 +539,7 @@ fun ReaderTagsFeedLoaded() {
),
TagFeedItem(
tagChip = TagChip(readerTag, {}),
postList = PostList.Loading,
postList = PostList.Initial,
),
TagFeedItem(
tagChip = TagChip(readerTag, {}),
Expand Down
Loading
Loading