From 8651a4f66b3cea897ea56054a265b8208064a8fd Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Wed, 28 Aug 2024 13:11:42 +0530 Subject: [PATCH] refactor: lazily query saved and read state in UI Having this always be read from the UI avoids values going stale inside data models Fixes #641 --- CHANGELOG.md | 4 ++ .../android/paging/LobstersPagingSource.kt | 25 +------- .../claw/android/paging/SearchPagingSource.kt | 26 +------- .../dev/msfjarvis/claw/android/ui/ext.kt | 4 ++ .../claw/android/ui/lists/DatabasePosts.kt | 2 +- .../claw/android/ui/lists/LobstersListItem.kt | 9 +-- .../claw/android/ui/lists/NetworkPosts.kt | 6 +- .../claw/android/viewmodel/ClawViewModel.kt | 8 +++ .../android/viewmodel/SavedPostsRepository.kt | 3 +- .../claw/common/posts/LobstersCard.kt | 62 ++++++++----------- .../claw/common/posts/PostActions.kt | 4 ++ .../kotlin/dev/msfjarvis/claw/model/UIPost.kt | 3 - 12 files changed, 56 insertions(+), 100 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55780bd1..6960911b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Opening posts you have previously seen will show the number of new comments since last visit +### Fixed + +- Saving posts no longer triggers a page refresh that invalidates scroll position + ### Changed - Change submitter text to 'authored' when applicable diff --git a/android/src/main/kotlin/dev/msfjarvis/claw/android/paging/LobstersPagingSource.kt b/android/src/main/kotlin/dev/msfjarvis/claw/android/paging/LobstersPagingSource.kt index 6487ab44..5b7dea52 100644 --- a/android/src/main/kotlin/dev/msfjarvis/claw/android/paging/LobstersPagingSource.kt +++ b/android/src/main/kotlin/dev/msfjarvis/claw/android/paging/LobstersPagingSource.kt @@ -14,16 +14,12 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import dev.msfjarvis.claw.android.ui.toError -import dev.msfjarvis.claw.android.viewmodel.ReadPostsRepository -import dev.msfjarvis.claw.android.viewmodel.SavedPostsRepository import dev.msfjarvis.claw.core.injection.IODispatcher -import dev.msfjarvis.claw.database.local.SavedPost import dev.msfjarvis.claw.model.LobstersPost import dev.msfjarvis.claw.model.UIPost import dev.msfjarvis.claw.model.toUIPost import java.io.IOException import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext class LobstersPagingSource @@ -31,33 +27,14 @@ class LobstersPagingSource constructor( @Assisted private val remoteFetcher: RemoteFetcher, @IODispatcher private val ioDispatcher: CoroutineDispatcher, - private val savedPostsRepository: SavedPostsRepository, - private val readPostsRepository: ReadPostsRepository, ) : PagingSource() { - private lateinit var savedPosts: List - private lateinit var readPosts: List - override suspend fun load(params: LoadParams): LoadResult { - if (!::savedPosts.isInitialized) { - savedPosts = savedPostsRepository.savedPosts.first().map(SavedPost::shortId) - } - if (!::readPosts.isInitialized) { - readPosts = readPostsRepository.readPosts.first() - } val page = params.key ?: STARTING_PAGE_INDEX return when (val result = withContext(ioDispatcher) { remoteFetcher.getItemsAtPage(page) }) { is Success -> LoadResult.Page( itemsBefore = (page - 1) * PAGE_SIZE, - data = - result.value.map { - it - .toUIPost() - .copy( - isSaved = savedPosts.contains(it.shortId), - isRead = readPosts.contains(it.shortId), - ) - }, + data = result.value.map(LobstersPost::toUIPost), prevKey = if (page == STARTING_PAGE_INDEX) null else page - 1, nextKey = page + 1, ) diff --git a/android/src/main/kotlin/dev/msfjarvis/claw/android/paging/SearchPagingSource.kt b/android/src/main/kotlin/dev/msfjarvis/claw/android/paging/SearchPagingSource.kt index 32132b72..3e404e9a 100644 --- a/android/src/main/kotlin/dev/msfjarvis/claw/android/paging/SearchPagingSource.kt +++ b/android/src/main/kotlin/dev/msfjarvis/claw/android/paging/SearchPagingSource.kt @@ -15,16 +15,13 @@ import dagger.assisted.AssistedInject import dev.msfjarvis.claw.android.paging.LobstersPagingSource.Companion.PAGE_SIZE import dev.msfjarvis.claw.android.paging.LobstersPagingSource.Companion.STARTING_PAGE_INDEX import dev.msfjarvis.claw.android.ui.toError -import dev.msfjarvis.claw.android.viewmodel.ReadPostsRepository -import dev.msfjarvis.claw.android.viewmodel.SavedPostsRepository import dev.msfjarvis.claw.api.LobstersSearchApi import dev.msfjarvis.claw.core.injection.IODispatcher -import dev.msfjarvis.claw.database.local.SavedPost +import dev.msfjarvis.claw.model.LobstersPost import dev.msfjarvis.claw.model.UIPost import dev.msfjarvis.claw.model.toUIPost import java.io.IOException import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext /** @@ -39,19 +36,8 @@ constructor( private val searchApi: LobstersSearchApi, @Assisted private val queryProvider: () -> String, @IODispatcher private val ioDispatcher: CoroutineDispatcher, - private val savedPostsRepository: SavedPostsRepository, - private val readPostsRepository: ReadPostsRepository, ) : PagingSource() { - private lateinit var savedPosts: List - private lateinit var readPosts: List - override suspend fun load(params: LoadParams): LoadResult { - if (!::savedPosts.isInitialized) { - savedPosts = savedPostsRepository.savedPosts.first().map(SavedPost::shortId) - } - if (!::readPosts.isInitialized) { - readPosts = readPostsRepository.readPosts.first() - } val searchQuery = queryProvider() // If there is no query, we don't need to call the API at all. if (searchQuery.isEmpty()) { @@ -66,15 +52,7 @@ constructor( val nextKey = if (result.value.isEmpty()) null else page + 1 LoadResult.Page( itemsBefore = (page - 1) * PAGE_SIZE, - data = - result.value.map { - it - .toUIPost() - .copy( - isSaved = savedPosts.contains(it.shortId), - isRead = readPosts.contains(it.shortId), - ) - }, + data = result.value.map(LobstersPost::toUIPost), prevKey = if (page == STARTING_PAGE_INDEX) null else page - 1, nextKey = nextKey, ) diff --git a/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/ext.kt b/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/ext.kt index 8101629a..356cfe87 100644 --- a/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/ext.kt +++ b/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/ext.kt @@ -70,6 +70,10 @@ fun rememberPostActions( context.startActivity(shareIntent) } + override fun isPostRead(post: UIPost): Boolean = viewModel.isPostRead(post) + + override fun isPostSaved(post: UIPost): Boolean = viewModel.isPostSaved(post) + override suspend fun getComments(postId: String): UIPost { return viewModel.getPostComments(postId) } diff --git a/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/DatabasePosts.kt b/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/DatabasePosts.kt index 7a62df20..48e920fa 100644 --- a/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/DatabasePosts.kt +++ b/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/DatabasePosts.kt @@ -54,7 +54,7 @@ fun DatabasePosts( items.forEach { (month, posts) -> stickyHeader(contentType = "month-header") { MonthHeader(label = month) } items(items = posts, key = { it.shortId }, contentType = { "LobstersItem" }) { item -> - LobstersListItem(item = item, refresh = {}, postActions = postActions) + LobstersListItem(item = item, postActions = postActions) HorizontalDivider() } } diff --git a/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/LobstersListItem.kt b/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/LobstersListItem.kt index 7f16a856..e2fd4dc6 100644 --- a/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/LobstersListItem.kt +++ b/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/LobstersListItem.kt @@ -20,12 +20,7 @@ import me.saket.swipe.SwipeAction import me.saket.swipe.SwipeableActionsBox @Composable -fun LobstersListItem( - item: UIPost, - postActions: PostActions, - refresh: () -> Unit, - modifier: Modifier = Modifier, -) { +fun LobstersListItem(item: UIPost, postActions: PostActions, modifier: Modifier = Modifier) { val commentsAction = SwipeAction( icon = rememberVectorPainter(Icons.AutoMirrored.Filled.Reply), @@ -39,6 +34,6 @@ fun LobstersListItem( onSwipe = { postActions.share(item) }, ) SwipeableActionsBox(startActions = listOf(shareAction), endActions = listOf(commentsAction)) { - LobstersCard(post = item, postActions = postActions, refresh = refresh, modifier = modifier) + LobstersCard(post = item, postActions = postActions, modifier = modifier) } } diff --git a/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/NetworkPosts.kt b/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/NetworkPosts.kt index dbb58222..2c749657 100644 --- a/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/NetworkPosts.kt +++ b/android/src/main/kotlin/dev/msfjarvis/claw/android/ui/lists/NetworkPosts.kt @@ -73,11 +73,7 @@ fun NetworkPosts( ) { index -> val item = lazyPagingItems[index] if (item != null) { - LobstersListItem( - item = item, - postActions = postActions, - refresh = { lazyPagingItems.refresh() }, - ) + LobstersListItem(item = item, postActions = postActions) HorizontalDivider() } } diff --git a/android/src/main/kotlin/dev/msfjarvis/claw/android/viewmodel/ClawViewModel.kt b/android/src/main/kotlin/dev/msfjarvis/claw/android/viewmodel/ClawViewModel.kt index d1cfb3e1..e95acfe0 100644 --- a/android/src/main/kotlin/dev/msfjarvis/claw/android/viewmodel/ClawViewModel.kt +++ b/android/src/main/kotlin/dev/msfjarvis/claw/android/viewmodel/ClawViewModel.kt @@ -133,6 +133,14 @@ constructor( } } + fun isPostRead(post: UIPost): Boolean { + return _readPosts.contains(post.shortId) + } + + fun isPostSaved(post: UIPost): Boolean { + return _savedPosts.contains(post.shortId) + } + suspend fun getPostComments(postId: String) = withContext(ioDispatcher) { when (val result = api.getPostDetails(postId)) { diff --git a/android/src/main/kotlin/dev/msfjarvis/claw/android/viewmodel/SavedPostsRepository.kt b/android/src/main/kotlin/dev/msfjarvis/claw/android/viewmodel/SavedPostsRepository.kt index 9e8e9ba9..6901af87 100644 --- a/android/src/main/kotlin/dev/msfjarvis/claw/android/viewmodel/SavedPostsRepository.kt +++ b/android/src/main/kotlin/dev/msfjarvis/claw/android/viewmodel/SavedPostsRepository.kt @@ -16,6 +16,7 @@ import dev.msfjarvis.claw.model.toSavedPost import io.github.aakira.napier.Napier import javax.inject.Inject import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.withContext class SavedPostsRepository @@ -27,7 +28,7 @@ constructor( val savedPosts = savedPostQueries.selectAllPosts().asFlow().mapToList(dbDispatcher) suspend fun toggleSave(post: UIPost) { - if (post.isSaved) { + if (savedPosts.firstOrNull().orEmpty().any { it.shortId == post.shortId }) { Napier.d(tag = TAG) { "Removing post: ${post.shortId}" } withContext(dbDispatcher) { savedPostQueries.deletePost(post.shortId) } } else { diff --git a/common/src/main/kotlin/dev/msfjarvis/claw/common/posts/LobstersCard.kt b/common/src/main/kotlin/dev/msfjarvis/claw/common/posts/LobstersCard.kt index d9be14bf..4e15cbfd 100644 --- a/common/src/main/kotlin/dev/msfjarvis/claw/common/posts/LobstersCard.kt +++ b/common/src/main/kotlin/dev/msfjarvis/claw/common/posts/LobstersCard.kt @@ -38,6 +38,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.material3.minimumInteractiveComponentSize import androidx.compose.runtime.Composable +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -60,23 +61,14 @@ import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.toImmutableList @Composable -fun LobstersCard( - post: UIPost, - postActions: PostActions, - refresh: () -> Unit, - modifier: Modifier = Modifier, -) { - var localReadState by remember(post) { mutableStateOf(post.isRead) } - var localSavedState by remember(post) { mutableStateOf(post.isSaved) } +fun LobstersCard(post: UIPost, postActions: PostActions, modifier: Modifier = Modifier) { + val readState by remember(post.shortId) { derivedStateOf { postActions.isPostRead(post) } } + val savedState by remember(post.shortId) { derivedStateOf { postActions.isPostSaved(post) } } Box( modifier = modifier .fillMaxWidth() - .clickable { - postActions.viewPost(post.shortId, post.url, post.commentsUrl) - localReadState = true - refresh() - } + .clickable { postActions.viewPost(post.shortId, post.url, post.commentsUrl) } .background(MaterialTheme.colorScheme.background) .padding(start = 8.dp, top = 2.dp, bottom = 2.dp) ) { @@ -86,7 +78,7 @@ fun LobstersCard( ) { PostDetails( post = post, - isRead = localReadState, + isRead = { readState }, singleLineTitle = true, modifier = Modifier.weight(1f), ) @@ -94,25 +86,14 @@ fun LobstersCard( modifier = Modifier.wrapContentHeight(), horizontalAlignment = Alignment.CenterHorizontally, ) { - SaveButton( - isSaved = localSavedState, - modifier = - Modifier.clickable(role = Role.Button) { - localSavedState = !localSavedState - postActions.toggleSave(post) - }, - ) + SaveButton(isSaved = { savedState }, onClick = { postActions.toggleSave(post) }) HorizontalDivider(modifier = Modifier.width(48.dp)) CommentsButton( commentCount = post.commentCount, modifier = Modifier.clickable( role = Role.Button, - onClick = { - postActions.viewComments(post.shortId) - localReadState = true - refresh() - }, + onClick = { postActions.viewComments(post.shortId) }, ), ) } @@ -123,12 +104,12 @@ fun LobstersCard( @Composable fun PostDetails( post: UIPost, - isRead: Boolean, + isRead: () -> Boolean, singleLineTitle: Boolean, modifier: Modifier = Modifier, ) { Column(modifier = modifier, verticalArrangement = Arrangement.spacedBy(8.dp)) { - PostTitle(title = post.title, isRead = isRead, singleLineTitle = singleLineTitle) + PostTitle(title = post.title, isRead = isRead(), singleLineTitle = singleLineTitle) TagRow(tags = post.tags.toImmutableList()) Spacer(Modifier.height(4.dp)) Submitter( @@ -189,14 +170,19 @@ internal fun Submitter( } @Composable -private fun SaveButton(isSaved: Boolean, modifier: Modifier = Modifier) { - Crossfade(targetState = isSaved, label = "save-button") { saved -> +private fun SaveButton(isSaved: () -> Boolean, onClick: () -> Unit, modifier: Modifier = Modifier) { + var localSavedState by remember { mutableStateOf(isSaved()) } + Crossfade(targetState = localSavedState, label = "save-button") { saved -> Box(modifier = modifier.minimumInteractiveComponentSize()) { Icon( imageVector = if (saved) Icons.Filled.Favorite else Icons.Filled.FavoriteBorder, tint = MaterialTheme.colorScheme.secondary, contentDescription = if (saved) "Remove from saved posts" else "Add to saved posts", - modifier = Modifier.align(Alignment.Center).testTag("save_button"), + modifier = + Modifier.align(Alignment.Center).testTag("save_button").clickable(role = Role.Button) { + onClick() + localSavedState = !localSavedState + }, ) } } @@ -266,6 +252,14 @@ val TEST_POST_ACTIONS = override fun share(post: UIPost) {} + override fun isPostRead(post: UIPost): Boolean { + return true + } + + override fun isPostSaved(post: UIPost): Boolean { + return true + } + override suspend fun getComments(postId: String): UIPost { return UIPost( shortId = "ooga", @@ -298,12 +292,10 @@ val TEST_POST = submitter = "Haki", tags = listOf("databases", "apis"), description = "", - isSaved = true, - isRead = true, ) @ThemePreviews @Composable private fun LobstersCardPreview() { - LobstersTheme { LobstersCard(post = TEST_POST, postActions = TEST_POST_ACTIONS, refresh = {}) } + LobstersTheme { LobstersCard(post = TEST_POST, postActions = TEST_POST_ACTIONS) } } diff --git a/common/src/main/kotlin/dev/msfjarvis/claw/common/posts/PostActions.kt b/common/src/main/kotlin/dev/msfjarvis/claw/common/posts/PostActions.kt index 852321de..d8f976b8 100644 --- a/common/src/main/kotlin/dev/msfjarvis/claw/common/posts/PostActions.kt +++ b/common/src/main/kotlin/dev/msfjarvis/claw/common/posts/PostActions.kt @@ -22,6 +22,10 @@ interface PostActions { fun share(post: UIPost) + fun isPostRead(post: UIPost): Boolean + + fun isPostSaved(post: UIPost): Boolean + suspend fun getComments(postId: String): UIPost suspend fun getLinkMetadata(url: String): LinkMetadata diff --git a/model/src/main/kotlin/dev/msfjarvis/claw/model/UIPost.kt b/model/src/main/kotlin/dev/msfjarvis/claw/model/UIPost.kt index 0471e8f3..e032a4ae 100644 --- a/model/src/main/kotlin/dev/msfjarvis/claw/model/UIPost.kt +++ b/model/src/main/kotlin/dev/msfjarvis/claw/model/UIPost.kt @@ -27,8 +27,6 @@ data class UIPost( @SerialName("submitter_user") val submitter: String, val tags: List, val comments: List = emptyList(), - val isSaved: Boolean = false, - val isRead: Boolean = false, val userIsAuthor: Boolean = false, ) { @KonvertFrom( @@ -37,7 +35,6 @@ data class UIPost( [ Mapping(target = "submitter", expression = "it.submitterName"), Mapping(target = "commentCount", expression = "it.commentCount ?: 0"), - Mapping(target = "isSaved", expression = "true"), ], ) companion object