From e4fd746755c652e92b620b4491bbe920bd5adabd Mon Sep 17 00:00:00 2001 From: "maarten.vercruysse" Date: Sun, 17 Sep 2023 00:15:13 +0200 Subject: [PATCH 1/5] Add missingComment logic --- app/src/main/java/com/jerboa/Utils.kt | 144 ++++++++--- .../java/com/jerboa/model/PostViewModel.kt | 3 +- .../ui/components/comment/CommentNode.kt | 223 ++++++++++++++---- .../ui/components/comment/CommentNodes.kt | 117 ++++++--- .../jerboa/ui/components/post/PostActivity.kt | 25 +- app/src/main/res/values/strings.xml | 1 + 6 files changed, 385 insertions(+), 128 deletions(-) diff --git a/app/src/main/java/com/jerboa/Utils.kt b/app/src/main/java/com/jerboa/Utils.kt index fa94d0c7f..281875b2c 100644 --- a/app/src/main/java/com/jerboa/Utils.kt +++ b/app/src/main/java/com/jerboa/Utils.kt @@ -49,6 +49,7 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewmodel.CreationExtras import androidx.navigation.NavController +import arrow.core.Either import arrow.core.compareTo import coil.annotation.ExperimentalCoilApi import coil.imageLoader @@ -80,6 +81,7 @@ import java.net.MalformedURLException import java.net.URL import java.text.DecimalFormat import java.util.* +import kotlin.collections.ArrayDeque import kotlin.math.abs import kotlin.math.pow @@ -238,64 +240,124 @@ data class InstantScores( val downvotes: Int, ) -data class CommentNodeData( - val commentView: CommentView, +@Stable +data class CommentNodeData( + val data: T, + val depth: Int, // Must use a SnapshotStateList and not a MutableList here, otherwise changes in the tree children won't trigger a UI update - val children: SnapshotStateList?, - var depth: Int, + val children: SnapshotStateList> = mutableStateListOf(), + var parent: CommentNodeData? = null, +) + +data class MissingCommentNode( + val commentId: Int, + val path: String, ) +typealias FullCommentNode = Either + fun commentsToFlatNodes( comments: List, -): ImmutableList { - return comments.map { c -> CommentNodeData(commentView = c, children = null, depth = 0) }.toImmutableList() +): ImmutableList> { + return comments.map { c -> CommentNodeData(data = Either.Right(c), depth = 0) }.toImmutableList() } fun buildCommentsTree( comments: List, - isCommentView: Boolean, -): ImmutableList { - val map = LinkedHashMap() + rootCommentId: Int?, // If it's in CommentView, then we need to know the root comment id +): ImmutableList> { + val isCommentView = rootCommentId != null + val map = LinkedHashMap>() val firstComment = comments.firstOrNull()?.comment - val depthOffset = if (!isCommentView) { - 0 + val depthOffset = if (isCommentView && firstComment != null) { + getCommentIdDepthFromPath(firstComment.path, rootCommentId!!) } else { - getDepthFromComment(firstComment) ?: 0 + 0 } comments.forEach { cv -> - val depth = getDepthFromComment(cv.comment)?.minus(depthOffset) ?: 0 - val node = CommentNodeData( - commentView = cv, - children = mutableStateListOf(), - depth, + val depth = getDepthFromComment(cv.comment).minus(depthOffset) + val node = CommentNodeData( + data = Either.Right(cv), + depth = depth, ) map[cv.comment.id] = node } - val tree = mutableListOf() + val tree = ArrayDeque>(comments.size) comments.forEach { cv -> val child = map[cv.comment.id] - child?.let { cChild -> - val parentId = getCommentParentId(cv.comment) - parentId?.let { cParentId -> - val parent = map[cParentId] - - // Necessary because blocked comment might not exist - parent?.let { cParent -> - cParent.children?.add(cChild) - } - } ?: run { - tree.add(cChild) - } + child?.let { + recCreateAndGenMissingCommentData(map, tree, cv.comment.path, it) } } + /** + * For commentView it will only receive partial comments, so we need to prune the missing nodes that it generates + * But only the ones that are at the top level, otherwise we'll prune nodes that are actually missing + */ + if (isCommentView) { + pruneMissingCommentNodesToComment(tree, rootCommentId!!) + } + return tree.toImmutableList() } +fun pruneMissingCommentNodesToComment(tree: MutableList>, rootCommentId: Int) { + while (tree.isNotEmpty()) { + val node = tree.first() + if (node.data.isLeft() && node.data.leftOrNull()?.commentId != rootCommentId) { + tree.removeFirst() + tree.addAll(node.children) + } else { + node.parent = null + return + } + } +} + +fun recCreateAndGenMissingCommentData( + map: LinkedHashMap>, + tree: MutableList>, + currCommentPath: String, + currCommentNodeData: CommentNodeData, +) { + val parentId = getCommentParentId(currCommentPath) + + // if no parent then add it to the root of the three + if (parentId != null) { + val parent = map[parentId] + // If the parent doesn't exist, then we need to add a placeholder node + + if (parent == null) { + val parentPath = getParentPath(currCommentPath) + val missingNode = CommentNodeData( + data = Either.Left( + MissingCommentNode( + parentId, + parentPath, + ), + ), + depth = currCommentNodeData.depth - 1, + ) + map[parentId] = missingNode + missingNode.children.add(currCommentNodeData) + currCommentNodeData.parent = missingNode + // The the missing parent needs to be correctly weaved into the tree + // It needs a parent, and it needs to be added to the parent's children + // The parent may also be missing, so we need to recursively call this function + recCreateAndGenMissingCommentData(map, tree, parentPath, missingNode) + } else { + currCommentNodeData.parent = parent + parent.children.add(currCommentNodeData) + } + } else { + tree.add(currCommentNodeData) + } +} + fun LazyListState.isScrolledToEnd(): Boolean { val totalItems = layoutInfo.totalItemsCount val lastItemVisible = layoutInfo.visibleItemsInfo.lastOrNull()?.index @@ -868,19 +930,29 @@ fun isSameInstance(url: String, instance: String): Boolean { return hostName(url) == instance } -fun getCommentParentId(comment: Comment?): Int? { - val split = comment?.path?.split(".")?.toMutableList() +fun getCommentParentId(comment: Comment): Int? = getCommentParentId(comment.path) +fun getCommentParentId(commentPath: String): Int? { + val split = commentPath.split(".").toMutableList() // remove the 0 - split?.removeFirst() - return if (split !== null && split.size > 1) { + split.removeFirst() + return if (split.size > 1) { split[split.size - 2].toInt() } else { null } } -fun getDepthFromComment(comment: Comment?): Int? { - return comment?.path?.split(".")?.size?.minus(2) +fun getParentPath(path: String) = path.split(".").dropLast(1).joinToString(".") + +fun getDepthFromComment(commentPath: String): Int { + return commentPath.split(".").size.minus(2) +} + +fun getDepthFromComment(comment: Comment): Int = getDepthFromComment(comment.path) + +fun getCommentIdDepthFromPath(commentPath: String, commentId: Int): Int { + val split = commentPath.split(".").toMutableList() + return split.indexOf(commentId.toString()).minus(2) } fun nsfwCheck(postView: PostView): Boolean { diff --git a/app/src/main/java/com/jerboa/model/PostViewModel.kt b/app/src/main/java/com/jerboa/model/PostViewModel.kt index 70f3d1a39..4c6dc2e09 100644 --- a/app/src/main/java/com/jerboa/model/PostViewModel.kt +++ b/app/src/main/java/com/jerboa/model/PostViewModel.kt @@ -110,8 +110,7 @@ class PostViewModel(val id: Either, account: Account) : ViewM }) commentsRes = ApiState.Loading - commentsRes = - apiWrapper(API.getInstance().getComments(commentsForm.serializeToMap())) + commentsRes = apiWrapper(API.getInstance().getComments(commentsForm.serializeToMap())) } } diff --git a/app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt b/app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt index 6e20ba163..636d57a3d 100644 --- a/app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt +++ b/app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt @@ -22,7 +22,6 @@ import androidx.compose.material.icons.filled.Bookmark import androidx.compose.material.icons.outlined.BookmarkBorder import androidx.compose.material.icons.outlined.Comment import androidx.compose.material.icons.outlined.MoreVert -import androidx.compose.material.icons.outlined.Person import androidx.compose.material3.Divider import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedButton @@ -39,11 +38,14 @@ import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontFamily +import androidx.compose.ui.text.font.FontStyle import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import com.jerboa.Border import com.jerboa.CommentNodeData +import com.jerboa.FullCommentNode import com.jerboa.InstantScores +import com.jerboa.MissingCommentNode import com.jerboa.R import com.jerboa.VoteType import com.jerboa.border @@ -169,7 +171,7 @@ fun CommentBodyPreview() { } fun LazyListScope.commentNodeItem( - node: CommentNodeData, + node: CommentNodeData, increaseLazyListIndexTracker: () -> Unit, addToParentIndexes: () -> Unit, isFlat: Boolean, @@ -204,7 +206,7 @@ fun LazyListScope.commentNodeItem( blurNSFW: Boolean, showScores: Boolean, ) { - val commentView = node.commentView + val commentView = node.data val commentId = commentView.comment.id val offset = calculateCommentOffset(node.depth, 4) // The ones with a border on @@ -218,8 +220,8 @@ fun LazyListScope.commentNodeItem( addToParentIndexes() } - val showMoreChildren = isExpanded(commentId) && node.children.isNullOrEmpty() && node - .commentView.counts.child_count > 0 && !isFlat + val showMoreChildren = isExpanded(commentId) && node.children.isEmpty() && + commentView.counts.child_count > 0 && !isFlat increaseLazyListIndexTracker() // TODO Needs a contentType @@ -284,7 +286,7 @@ fun LazyListScope.commentNodeItem( onLongClick = { onHeaderLongClick(commentView) }, - collapsedCommentsCount = node.commentView.counts.child_count, + collapsedCommentsCount = commentView.counts.child_count, isExpanded = isExpanded(commentId), showAvatar = showAvatar, showScores = showScores, @@ -374,44 +376,177 @@ fun LazyListScope.commentNodeItem( } } - node.children?.also { nodes -> - commentNodeItems( - nodes = nodes.toImmutableList(), - increaseLazyListIndexTracker = increaseLazyListIndexTracker, - addToParentIndexes = addToParentIndexes, - isFlat = isFlat, - toggleExpanded = toggleExpanded, - toggleActionBar = toggleActionBar, - isExpanded = isExpanded, - onUpvoteClick = onUpvoteClick, - onDownvoteClick = onDownvoteClick, - onSaveClick = onSaveClick, - onMarkAsReadClick = onMarkAsReadClick, - onCommentClick = onCommentClick, - onEditCommentClick = onEditCommentClick, - onDeleteCommentClick = onDeleteCommentClick, - onPersonClick = onPersonClick, - onHeaderClick = onHeaderClick, - onHeaderLongClick = onHeaderLongClick, - onCommunityClick = onCommunityClick, - onPostClick = onPostClick, - showPostAndCommunityContext = showPostAndCommunityContext, - onReportClick = onReportClick, - onCommentLinkClick = onCommentLinkClick, - onFetchChildrenClick = onFetchChildrenClick, - onReplyClick = onReplyClick, - onBlockCreatorClick = onBlockCreatorClick, - account = account, - isModerator = isModerator, - isCollapsedByParent = isCollapsedByParent || !isExpanded(commentId), - showCollapsedCommentContent = showCollapsedCommentContent, - showActionBar = showActionBar, - enableDownVotes = enableDownVotes, - showAvatar = showAvatar, - blurNSFW = blurNSFW, - showScores = showScores, - ) + commentNodeItems( + nodes = node.children.toImmutableList(), + increaseLazyListIndexTracker = increaseLazyListIndexTracker, + addToParentIndexes = addToParentIndexes, + isFlat = isFlat, + toggleExpanded = toggleExpanded, + toggleActionBar = toggleActionBar, + isExpanded = isExpanded, + onUpvoteClick = onUpvoteClick, + onDownvoteClick = onDownvoteClick, + onSaveClick = onSaveClick, + onMarkAsReadClick = onMarkAsReadClick, + onCommentClick = onCommentClick, + onEditCommentClick = onEditCommentClick, + onDeleteCommentClick = onDeleteCommentClick, + onPersonClick = onPersonClick, + onHeaderClick = onHeaderClick, + onHeaderLongClick = onHeaderLongClick, + onCommunityClick = onCommunityClick, + onPostClick = onPostClick, + showPostAndCommunityContext = showPostAndCommunityContext, + onReportClick = onReportClick, + onCommentLinkClick = onCommentLinkClick, + onFetchChildrenClick = onFetchChildrenClick, + onReplyClick = onReplyClick, + onBlockCreatorClick = onBlockCreatorClick, + account = account, + isModerator = isModerator, + isCollapsedByParent = isCollapsedByParent || !isExpanded(commentId), + showCollapsedCommentContent = showCollapsedCommentContent, + showActionBar = showActionBar, + enableDownVotes = enableDownVotes, + showAvatar = showAvatar, + blurNSFW = blurNSFW, + showScores = showScores, + ) +} + +fun LazyListScope.missingCommentNodeItem( + node: CommentNodeData, + increaseLazyListIndexTracker: () -> Unit, + addToParentIndexes: () -> Unit, + isFlat: Boolean, + isExpanded: (commentId: Int) -> Boolean, + toggleExpanded: (commentId: Int) -> Unit, + toggleActionBar: (commentId: Int) -> Unit, + isModerator: (Int) -> Boolean, + onUpvoteClick: (commentView: CommentView) -> Unit, + onDownvoteClick: (commentView: CommentView) -> Unit, + onReplyClick: (commentView: CommentView) -> Unit, + onSaveClick: (commentView: CommentView) -> Unit, + onMarkAsReadClick: (commentView: CommentView) -> Unit, + onCommentClick: (commentView: CommentView) -> Unit, + onEditCommentClick: (commentView: CommentView) -> Unit, + onDeleteCommentClick: (commentView: CommentView) -> Unit, + onPersonClick: (personId: Int) -> Unit, + onHeaderClick: (commentView: CommentView) -> Unit, + onHeaderLongClick: (commentView: CommentView) -> Unit, + onCommunityClick: (community: Community) -> Unit, + onPostClick: (postId: Int) -> Unit, + onReportClick: (commentView: CommentView) -> Unit, + onCommentLinkClick: (commentView: CommentView) -> Unit, + onBlockCreatorClick: (creator: Person) -> Unit, + onFetchChildrenClick: (commentView: CommentView) -> Unit, + showCollapsedCommentContent: Boolean, + showPostAndCommunityContext: Boolean = false, + account: Account, + isCollapsedByParent: Boolean, + showActionBar: (commentId: Int) -> Boolean, + enableDownVotes: Boolean, + showAvatar: Boolean, + blurNSFW: Boolean, + showScores: Boolean, +) { + val commentId = node.data.commentId + + val offset = calculateCommentOffset(node.depth, 4) // The ones with a border on + val offset2 = if (node.depth == 0) { + MEDIUM_PADDING + } else { + XXL_PADDING } + + if (node.depth == 0) { + addToParentIndexes() + } + + increaseLazyListIndexTracker() + // TODO Needs a contentType + // possibly "contentNodeItemL${node.depth}" + item(key = commentId) { + val backgroundColor = MaterialTheme.colorScheme.background + val borderColor = calculateBorderColor(backgroundColor, node.depth) + val border = Border(SMALL_PADDING, borderColor) + + AnimatedVisibility( + visible = !isCollapsedByParent, + enter = expandVertically(), + exit = shrinkVertically(), + ) { + Column( + modifier = Modifier + .padding( + start = offset, + ), + ) { + Column( + modifier = Modifier.border(start = border), + ) { + Divider(modifier = Modifier.padding(start = if (node.depth == 0) 0.dp else border.strokeWidth)) + Column( + modifier = Modifier.padding( + start = offset2, + end = MEDIUM_PADDING, + ), + ) { + AnimatedVisibility( + visible = isExpanded(commentId) || showCollapsedCommentContent, + enter = expandVertically(), + exit = shrinkVertically(), + ) { + Text( + text = stringResource(id = R.string.comment_gone), + fontStyle = FontStyle.Italic, + modifier = Modifier.padding(vertical = SMALL_PADDING), + ) + } + } + } + } + } + } + + increaseLazyListIndexTracker() + + commentNodeItems( + nodes = node.children.toImmutableList(), + increaseLazyListIndexTracker = increaseLazyListIndexTracker, + addToParentIndexes = addToParentIndexes, + isFlat = isFlat, + toggleExpanded = toggleExpanded, + toggleActionBar = toggleActionBar, + isExpanded = isExpanded, + onUpvoteClick = onUpvoteClick, + onDownvoteClick = onDownvoteClick, + onSaveClick = onSaveClick, + onMarkAsReadClick = onMarkAsReadClick, + onCommentClick = onCommentClick, + onEditCommentClick = onEditCommentClick, + onDeleteCommentClick = onDeleteCommentClick, + onPersonClick = onPersonClick, + onHeaderClick = onHeaderClick, + onHeaderLongClick = onHeaderLongClick, + onCommunityClick = onCommunityClick, + onPostClick = onPostClick, + showPostAndCommunityContext = showPostAndCommunityContext, + onReportClick = onReportClick, + onCommentLinkClick = onCommentLinkClick, + onFetchChildrenClick = onFetchChildrenClick, + onReplyClick = onReplyClick, + onBlockCreatorClick = onBlockCreatorClick, + account = account, + isModerator = isModerator, + isCollapsedByParent = isCollapsedByParent || !isExpanded(commentId), + showCollapsedCommentContent = showCollapsedCommentContent, + showActionBar = showActionBar, + enableDownVotes = enableDownVotes, + showAvatar = showAvatar, + blurNSFW = blurNSFW, + showScores = showScores, + ) } @Composable @@ -621,7 +756,7 @@ fun CommentNodesPreview() { sampleCommentView, sampleReplyCommentView, ) - val tree = buildCommentsTree(comments, false) + val tree = buildCommentsTree(comments, null) CommentNodes( nodes = tree, increaseLazyListIndexTracker = {}, diff --git a/app/src/main/java/com/jerboa/ui/components/comment/CommentNodes.kt b/app/src/main/java/com/jerboa/ui/components/comment/CommentNodes.kt index ac7905ae2..3f62f62df 100644 --- a/app/src/main/java/com/jerboa/ui/components/comment/CommentNodes.kt +++ b/app/src/main/java/com/jerboa/ui/components/comment/CommentNodes.kt @@ -8,7 +8,9 @@ import androidx.compose.foundation.lazy.LazyListState import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp +import arrow.core.Either import com.jerboa.CommentNodeData +import com.jerboa.FullCommentNode import com.jerboa.datatypes.types.CommentView import com.jerboa.datatypes.types.Community import com.jerboa.datatypes.types.Person @@ -17,7 +19,7 @@ import kotlinx.collections.immutable.ImmutableList @Composable fun CommentNodes( - nodes: ImmutableList, + nodes: ImmutableList>, increaseLazyListIndexTracker: () -> Unit, addToParentIndexes: () -> Unit, isFlat: Boolean, @@ -97,7 +99,7 @@ fun CommentNodes( } fun LazyListScope.commentNodeItems( - nodes: ImmutableList, + nodes: ImmutableList>, increaseLazyListIndexTracker: () -> Unit, addToParentIndexes: () -> Unit, isFlat: Boolean, @@ -133,41 +135,80 @@ fun LazyListScope.commentNodeItems( showScores: Boolean, ) { nodes.forEach { node -> - commentNodeItem( - node = node, - increaseLazyListIndexTracker = increaseLazyListIndexTracker, - addToParentIndexes = addToParentIndexes, - isFlat = isFlat, - isExpanded = isExpanded, - toggleExpanded = toggleExpanded, - toggleActionBar = toggleActionBar, - onUpvoteClick = onUpvoteClick, - onDownvoteClick = onDownvoteClick, - onReplyClick = onReplyClick, - onSaveClick = onSaveClick, - account = account, - isModerator = isModerator, - onMarkAsReadClick = onMarkAsReadClick, - onCommentClick = onCommentClick, - onPersonClick = onPersonClick, - onHeaderClick = onHeaderClick, - onHeaderLongClick = onHeaderLongClick, - onCommunityClick = onCommunityClick, - onPostClick = onPostClick, - onEditCommentClick = onEditCommentClick, - onDeleteCommentClick = onDeleteCommentClick, - onReportClick = onReportClick, - onCommentLinkClick = onCommentLinkClick, - onFetchChildrenClick = onFetchChildrenClick, - onBlockCreatorClick = onBlockCreatorClick, - showPostAndCommunityContext = showPostAndCommunityContext, - showCollapsedCommentContent = showCollapsedCommentContent, - isCollapsedByParent = isCollapsedByParent, - showActionBar = showActionBar, - enableDownVotes = enableDownVotes, - showAvatar = showAvatar, - blurNSFW = blurNSFW, - showScores = showScores, - ) + when (val data = node.data) { + is Either.Right -> commentNodeItem( + node = CommentNodeData(data.value, node.depth, node.children, node.parent), + increaseLazyListIndexTracker = increaseLazyListIndexTracker, + addToParentIndexes = addToParentIndexes, + isFlat = isFlat, + isExpanded = isExpanded, + toggleExpanded = toggleExpanded, + toggleActionBar = toggleActionBar, + onUpvoteClick = onUpvoteClick, + onDownvoteClick = onDownvoteClick, + onReplyClick = onReplyClick, + onSaveClick = onSaveClick, + account = account, + isModerator = isModerator, + onMarkAsReadClick = onMarkAsReadClick, + onCommentClick = onCommentClick, + onPersonClick = onPersonClick, + onHeaderClick = onHeaderClick, + onHeaderLongClick = onHeaderLongClick, + onCommunityClick = onCommunityClick, + onPostClick = onPostClick, + onEditCommentClick = onEditCommentClick, + onDeleteCommentClick = onDeleteCommentClick, + onReportClick = onReportClick, + onCommentLinkClick = onCommentLinkClick, + onFetchChildrenClick = onFetchChildrenClick, + onBlockCreatorClick = onBlockCreatorClick, + showPostAndCommunityContext = showPostAndCommunityContext, + showCollapsedCommentContent = showCollapsedCommentContent, + isCollapsedByParent = isCollapsedByParent, + showActionBar = showActionBar, + enableDownVotes = enableDownVotes, + showAvatar = showAvatar, + blurNSFW = blurNSFW, + showScores = showScores, + ) + + is Either.Left -> missingCommentNodeItem( + node = CommentNodeData(data.value, node.depth, node.children, node.parent), + increaseLazyListIndexTracker = increaseLazyListIndexTracker, + addToParentIndexes = addToParentIndexes, + isFlat = isFlat, + isExpanded = isExpanded, + toggleExpanded = toggleExpanded, + toggleActionBar = toggleActionBar, + onUpvoteClick = onUpvoteClick, + onDownvoteClick = onDownvoteClick, + onReplyClick = onReplyClick, + onSaveClick = onSaveClick, + account = account, + isModerator = isModerator, + onMarkAsReadClick = onMarkAsReadClick, + onCommentClick = onCommentClick, + onPersonClick = onPersonClick, + onHeaderClick = onHeaderClick, + onHeaderLongClick = onHeaderLongClick, + onCommunityClick = onCommunityClick, + onPostClick = onPostClick, + onEditCommentClick = onEditCommentClick, + onDeleteCommentClick = onDeleteCommentClick, + onReportClick = onReportClick, + onCommentLinkClick = onCommentLinkClick, + onFetchChildrenClick = onFetchChildrenClick, + onBlockCreatorClick = onBlockCreatorClick, + showPostAndCommunityContext = showPostAndCommunityContext, + showCollapsedCommentContent = showCollapsedCommentContent, + isCollapsedByParent = isCollapsedByParent, + showActionBar = showActionBar, + enableDownVotes = enableDownVotes, + showAvatar = showAvatar, + blurNSFW = blurNSFW, + showScores = showScores, + ) + } } } diff --git a/app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt b/app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt index fc6933fa3..a57acc2d9 100644 --- a/app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt +++ b/app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt @@ -491,15 +491,12 @@ fun PostActivity( is ApiState.Holder -> { val commentTree = buildCommentsTree( commentsRes.data.comments, - postViewModel.isCommentView(), + id.fold( + { null }, + { it }, + ), ) - val firstComment = - commentTree.firstOrNull()?.commentView?.comment - val depth = getDepthFromComment(firstComment) - val commentParentId = getCommentParentId(firstComment) - val showContextButton = depth != null && depth > 0 - val toggleExpanded: (Int) -> Unit = { commentId: Int -> if (unExpandedComments.contains(commentId)) { unExpandedComments.remove(commentId) @@ -518,10 +515,22 @@ fun PostActivity( item(key = "${postView.post.id}_is_comment_view", contentType = "contextButtons") { if (postViewModel.isCommentView()) { + val firstCommentNodeData = commentTree.firstOrNull() + + val firstCommentPath = when (val t = firstCommentNodeData?.data) { + is Either.Left -> t.value.path + is Either.Right -> t.value.comment.path + else -> null + } + + val hasParent = firstCommentPath != null && getDepthFromComment(firstCommentPath) > 0 + + val commentParentId = firstCommentPath?.let(::getCommentParentId) + ShowCommentContextButtons( postView.post.id, commentParentId = commentParentId, - showContextButton = showContextButton, + showContextButton = hasParent, onPostClick = appState::toPost, onCommentClick = appState::toComment, ) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 8fd86c896..3df275ee9 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -419,4 +419,5 @@ Posts failed loading, retry Failed to share media! Failed to parse datetime + There is no record of this comment From ae6a023315bf3f486c87c7955f9ff25e50d836de Mon Sep 17 00:00:00 2001 From: "maarten.vercruysse" Date: Sun, 1 Oct 2023 03:27:24 +0200 Subject: [PATCH 2/5] Remove prune, Simplify commentNodeData, Add tests --- app/src/main/java/com/jerboa/Utils.kt | 127 ++++++++++-------- .../ui/components/comment/CommentNode.kt | 11 +- .../ui/components/comment/CommentNodes.kt | 18 +-- .../jerboa/ui/components/post/PostActivity.kt | 6 +- app/src/test/java/com/jerboa/UtilsKtTest.kt | 65 +++++++++ 5 files changed, 151 insertions(+), 76 deletions(-) diff --git a/app/src/main/java/com/jerboa/Utils.kt b/app/src/main/java/com/jerboa/Utils.kt index 281875b2c..d2c49e635 100644 --- a/app/src/main/java/com/jerboa/Utils.kt +++ b/app/src/main/java/com/jerboa/Utils.kt @@ -49,7 +49,6 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewmodel.CreationExtras import androidx.navigation.NavController -import arrow.core.Either import arrow.core.compareTo import coil.annotation.ExperimentalCoilApi import coil.imageLoader @@ -240,34 +239,60 @@ data class InstantScores( val downvotes: Int, ) -@Stable -data class CommentNodeData( - val data: T, - val depth: Int, - // Must use a SnapshotStateList and not a MutableList here, otherwise changes in the tree children won't trigger a UI update - val children: SnapshotStateList> = mutableStateListOf(), - var parent: CommentNodeData? = null, -) - -data class MissingCommentNode( +data class MissingCommentView( val commentId: Int, val path: String, ) -typealias FullCommentNode = Either +sealed class CommentNodeData( + val depth: Int, + // Must use a SnapshotStateList and not a MutableList here, otherwise changes in the tree children won't trigger a UI update + val children: SnapshotStateList = mutableStateListOf(), + var parent: CommentNodeData? = null, +) { + abstract fun getId(): Int + abstract fun getPath(): String +} + +class CommentNode( + val commentView: CommentView, + depth: Int, + children: SnapshotStateList = mutableStateListOf(), + parent: CommentNodeData? = null, +) : CommentNodeData(depth, children, parent) { + override fun getId() = commentView.comment.id + override fun getPath() = commentView.comment.path +} + +class MissingCommentNode( + val missingCommentView: MissingCommentView, + depth: Int, + children: SnapshotStateList = mutableStateListOf(), + parent: CommentNodeData? = null, +) : CommentNodeData(depth, children, parent) { + override fun getId() = missingCommentView.commentId + override fun getPath() = missingCommentView.path +} fun commentsToFlatNodes( comments: List, -): ImmutableList> { - return comments.map { c -> CommentNodeData(data = Either.Right(c), depth = 0) }.toImmutableList() +): ImmutableList { + return comments.map { c -> CommentNode(c, depth = 0) }.toImmutableList() } +/** + * This function takes a list of comments and builds a tree from it + * + * In commentView it should be giving a id of the root comment + * Else it would generate a + */ fun buildCommentsTree( comments: List, rootCommentId: Int?, // If it's in CommentView, then we need to know the root comment id -): ImmutableList> { +): ImmutableList { val isCommentView = rootCommentId != null - val map = LinkedHashMap>() + + val map = LinkedHashMap() val firstComment = comments.firstOrNull()?.comment val depthOffset = if (isCommentView && firstComment != null) { @@ -278,51 +303,34 @@ fun buildCommentsTree( comments.forEach { cv -> val depth = getDepthFromComment(cv.comment).minus(depthOffset) - val node = CommentNodeData( - data = Either.Right(cv), - depth = depth, - ) + val node = CommentNode(cv, depth) map[cv.comment.id] = node } - val tree = ArrayDeque>(comments.size) + val tree = ArrayDeque(comments.size) comments.forEach { cv -> val child = map[cv.comment.id] child?.let { - recCreateAndGenMissingCommentData(map, tree, cv.comment.path, it) + recCreateAndGenMissingCommentData(map, tree, cv.comment.path, it, rootCommentId) } } - /** - * For commentView it will only receive partial comments, so we need to prune the missing nodes that it generates - * But only the ones that are at the top level, otherwise we'll prune nodes that are actually missing - */ - if (isCommentView) { - pruneMissingCommentNodesToComment(tree, rootCommentId!!) - } - return tree.toImmutableList() } -fun pruneMissingCommentNodesToComment(tree: MutableList>, rootCommentId: Int) { - while (tree.isNotEmpty()) { - val node = tree.first() - if (node.data.isLeft() && node.data.leftOrNull()?.commentId != rootCommentId) { - tree.removeFirst() - tree.addAll(node.children) - } else { - node.parent = null - return - } - } -} - +/** + * This function is given a node and adds it to the parent's children + * If the parent doesn't exist it is missing, then it creates a placeholder node + * and passes it to this function again so that it can be added to the parent's children (recursively) + */ +// TODO: Remove this once missing comments issue is fixed by Lemmy, see https://github.com/dessalines/jerboa/pull/1240 fun recCreateAndGenMissingCommentData( - map: LinkedHashMap>, - tree: MutableList>, + map: LinkedHashMap, + tree: MutableList, currCommentPath: String, - currCommentNodeData: CommentNodeData, + currCommentNodeData: CommentNodeData, + rootCommentId: Int?, ) { val parentId = getCommentParentId(currCommentPath) @@ -332,23 +340,25 @@ fun recCreateAndGenMissingCommentData( // If the parent doesn't exist, then we need to add a placeholder node if (parent == null) { + // Do not generate a parent if its the root comment (commentView starting with this comment) + if (currCommentNodeData.getId() == rootCommentId) { + tree.add(currCommentNodeData) + return + } + val parentPath = getParentPath(currCommentPath) - val missingNode = CommentNodeData( - data = Either.Left( - MissingCommentNode( - parentId, - parentPath, - ), - ), - depth = currCommentNodeData.depth - 1, + val missingNode = MissingCommentNode( + MissingCommentView(parentId, parentPath), + currCommentNodeData.depth - 1, ) + map[parentId] = missingNode missingNode.children.add(currCommentNodeData) currCommentNodeData.parent = missingNode // The the missing parent needs to be correctly weaved into the tree // It needs a parent, and it needs to be added to the parent's children // The parent may also be missing, so we need to recursively call this function - recCreateAndGenMissingCommentData(map, tree, parentPath, missingNode) + recCreateAndGenMissingCommentData(map, tree, parentPath, missingNode, rootCommentId) } else { currCommentNodeData.parent = parent parent.children.add(currCommentNodeData) @@ -794,6 +804,7 @@ fun siFormat(num: Int): String { formattedNumber } } + fun imageInputStreamFromUri(ctx: Context, uri: Uri): InputStream { return ctx.contentResolver.openInputStream(uri)!! } @@ -952,7 +963,7 @@ fun getDepthFromComment(comment: Comment): Int = getDepthFromComment(comment.pat fun getCommentIdDepthFromPath(commentPath: String, commentId: Int): Int { val split = commentPath.split(".").toMutableList() - return split.indexOf(commentId.toString()).minus(2) + return split.indexOf(commentId.toString()).minus(1) } fun nsfwCheck(postView: PostView): Boolean { @@ -1228,6 +1239,7 @@ fun calculateCommentOffset(depth: Int, multiplier: Int): Dp { (abs((depth.minus(1) * multiplier)).dp + SMALL_PADDING) } } + fun findAndUpdatePost(posts: List, updatedPostView: PostView): List { val foundIndex = posts.indexOfFirst { it.post.id == updatedPostView.post.id @@ -1370,6 +1382,7 @@ fun LocaleListCompat.convertToLanguageRange(): MutableList } return l } + inline fun > getEnumFromIntSetting( appSettings: LiveData, getter: (AppSettings) -> Int, @@ -1414,6 +1427,7 @@ fun matchLoginErrorMsgToStringRes(ctx: Context, e: Throwable): String { "registration_denied" -> ctx.getString(R.string.login_view_model_registration_denied) "registration_application_pending", "registration_application_is_pending" -> ctx.getString(R.string.login_view_model_registration_pending) + "missing_totp_token" -> ctx.getString(R.string.login_view_model_missing_totp) "incorrect_totp_token" -> ctx.getString(R.string.login_view_model_incorrect_totp) else -> { @@ -1465,6 +1479,7 @@ fun Context.getInputStream(url: String): InputStream { val videoRgx = Regex( pattern = "(http)?s?:?(//[^\"']*\\.(?:mp4|mp3|ogg|flv|m4a|3gp|mkv|mpeg|mov))", ) + fun isVideo(url: String): Boolean { return url.matches(videoRgx) } diff --git a/app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt b/app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt index 636d57a3d..b1fd980fb 100644 --- a/app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt +++ b/app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt @@ -42,8 +42,7 @@ import androidx.compose.ui.text.font.FontStyle import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import com.jerboa.Border -import com.jerboa.CommentNodeData -import com.jerboa.FullCommentNode +import com.jerboa.CommentNode import com.jerboa.InstantScores import com.jerboa.MissingCommentNode import com.jerboa.R @@ -171,7 +170,7 @@ fun CommentBodyPreview() { } fun LazyListScope.commentNodeItem( - node: CommentNodeData, + node: CommentNode, increaseLazyListIndexTracker: () -> Unit, addToParentIndexes: () -> Unit, isFlat: Boolean, @@ -206,7 +205,7 @@ fun LazyListScope.commentNodeItem( blurNSFW: Boolean, showScores: Boolean, ) { - val commentView = node.data + val commentView = node.commentView val commentId = commentView.comment.id val offset = calculateCommentOffset(node.depth, 4) // The ones with a border on @@ -415,7 +414,7 @@ fun LazyListScope.commentNodeItem( } fun LazyListScope.missingCommentNodeItem( - node: CommentNodeData, + node: MissingCommentNode, increaseLazyListIndexTracker: () -> Unit, addToParentIndexes: () -> Unit, isFlat: Boolean, @@ -450,7 +449,7 @@ fun LazyListScope.missingCommentNodeItem( blurNSFW: Boolean, showScores: Boolean, ) { - val commentId = node.data.commentId + val commentId = node.missingCommentView.commentId val offset = calculateCommentOffset(node.depth, 4) // The ones with a border on val offset2 = if (node.depth == 0) { diff --git a/app/src/main/java/com/jerboa/ui/components/comment/CommentNodes.kt b/app/src/main/java/com/jerboa/ui/components/comment/CommentNodes.kt index 3f62f62df..c252557d8 100644 --- a/app/src/main/java/com/jerboa/ui/components/comment/CommentNodes.kt +++ b/app/src/main/java/com/jerboa/ui/components/comment/CommentNodes.kt @@ -8,9 +8,9 @@ import androidx.compose.foundation.lazy.LazyListState import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp -import arrow.core.Either +import com.jerboa.CommentNode import com.jerboa.CommentNodeData -import com.jerboa.FullCommentNode +import com.jerboa.MissingCommentNode import com.jerboa.datatypes.types.CommentView import com.jerboa.datatypes.types.Community import com.jerboa.datatypes.types.Person @@ -19,7 +19,7 @@ import kotlinx.collections.immutable.ImmutableList @Composable fun CommentNodes( - nodes: ImmutableList>, + nodes: ImmutableList, increaseLazyListIndexTracker: () -> Unit, addToParentIndexes: () -> Unit, isFlat: Boolean, @@ -99,7 +99,7 @@ fun CommentNodes( } fun LazyListScope.commentNodeItems( - nodes: ImmutableList>, + nodes: ImmutableList, increaseLazyListIndexTracker: () -> Unit, addToParentIndexes: () -> Unit, isFlat: Boolean, @@ -135,9 +135,9 @@ fun LazyListScope.commentNodeItems( showScores: Boolean, ) { nodes.forEach { node -> - when (val data = node.data) { - is Either.Right -> commentNodeItem( - node = CommentNodeData(data.value, node.depth, node.children, node.parent), + when (node) { + is CommentNode -> commentNodeItem( + node = node, increaseLazyListIndexTracker = increaseLazyListIndexTracker, addToParentIndexes = addToParentIndexes, isFlat = isFlat, @@ -173,8 +173,8 @@ fun LazyListScope.commentNodeItems( showScores = showScores, ) - is Either.Left -> missingCommentNodeItem( - node = CommentNodeData(data.value, node.depth, node.children, node.parent), + is MissingCommentNode -> missingCommentNodeItem( + node = node, increaseLazyListIndexTracker = increaseLazyListIndexTracker, addToParentIndexes = addToParentIndexes, isFlat = isFlat, diff --git a/app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt b/app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt index 0a467171f..2d2649fd6 100644 --- a/app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt +++ b/app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt @@ -513,11 +513,7 @@ fun PostActivity( if (postViewModel.isCommentView()) { val firstCommentNodeData = commentTree.firstOrNull() - val firstCommentPath = when (val t = firstCommentNodeData?.data) { - is Either.Left -> t.value.path - is Either.Right -> t.value.comment.path - else -> null - } + val firstCommentPath = firstCommentNodeData?.getPath() val hasParent = firstCommentPath != null && getDepthFromComment(firstCommentPath) > 0 diff --git a/app/src/test/java/com/jerboa/UtilsKtTest.kt b/app/src/test/java/com/jerboa/UtilsKtTest.kt index 7c3babfe0..ec8aa71d6 100644 --- a/app/src/test/java/com/jerboa/UtilsKtTest.kt +++ b/app/src/test/java/com/jerboa/UtilsKtTest.kt @@ -3,6 +3,7 @@ package com.jerboa import android.content.Context import androidx.compose.ui.unit.dp import com.jerboa.api.API +import com.jerboa.datatypes.sampleCommentView import com.jerboa.ui.theme.SMALL_PADDING import junitparams.JUnitParamsRunner import junitparams.Parameters @@ -244,4 +245,68 @@ class UtilsKtTest { assertEquals("https://example.com", "https://example.com".toHttps()) assertEquals("example.com", "example.com".toHttps()) } + + @Test + fun testBuildCommentsTree() { + val tree1 = buildCommentsTree(listOf(sampleCommentView), null) + assertEquals(1, tree1.size) + assertTrue(tree1[0] is CommentNode) + + val sampleCV2 = sampleCommentView.copy(comment = sampleCommentView.comment.copy(path = "0.1.2", id = 2)) + + val tree2 = buildCommentsTree(listOf(sampleCommentView, sampleCV2), null) + assertEquals(1, tree2.size) + val root2 = tree2[0] as CommentNode + assertEquals(1, root2.children.size) + assertEquals(0, root2.depth) + assertTrue(root2.children[0] is CommentNode) + assertEquals(root2, root2.children[0].parent) + assertEquals(1, root2.children[0].depth) + + // Should not generate a missing comment as parent, because we said that root is sampleCV2 + val tree3 = buildCommentsTree(listOf(sampleCV2), sampleCV2.comment.id) + assertEquals(1, tree3.size) + assertTrue(tree3[0] is CommentNode) + val root3 = tree3[0] as CommentNode + assertEquals(sampleCV2, root3.commentView) + assertEquals(0, root3.depth) + assertEquals(0, root3.children.size) + + // Should generate a missing comment as parent + val tree4 = buildCommentsTree(listOf(sampleCV2), null) + assertEquals(1, tree4.size) + assertTrue(tree4[0] is MissingCommentNode) + val root4 = tree4[0] as MissingCommentNode + assertEquals(0, root4.depth) + assertEquals(null, root4.parent) + assertEquals(1, root4.children.size) + assertEquals(1, root4.missingCommentView.commentId) + assertTrue(root4.children[0] is CommentNode) + val child4 = root4.children[0] as CommentNode + assertEquals(sampleCV2, child4.commentView) + assertEquals(1, child4.depth) + assertEquals(root4, child4.parent) + + val sampleCV5 = sampleCommentView.copy(comment = sampleCommentView.comment.copy(path = "0.1.2.3", id = 3)) + + // Confirm recursive missing parent behaviour + val tree5 = buildCommentsTree(listOf(sampleCV5), null) + assertEquals(1, tree5.size) + assertTrue(tree5[0] is MissingCommentNode) + assertEquals(1, tree5[0].children.size) + assertTrue(tree5[0].children[0] is MissingCommentNode) + assertEquals(1, tree5[0].children[0].children.size) + assertTrue(tree5[0].children[0].children[0] is CommentNode) + assertEquals(3, tree5[0].children[0].children[0].getId()) + + // Confirm that it can generate a missing comment between two comments + val tree6 = buildCommentsTree(listOf(sampleCommentView, sampleCV5), null) + assertEquals(1, tree6.size) + assertTrue(tree6[0] is CommentNode) + assertEquals(1, tree6[0].children.size) + assertTrue(tree6[0].children[0] is MissingCommentNode) // The missing comment between sampleCommentView and sampleCV5 + assertEquals(1, tree6[0].children[0].children.size) + assertTrue(tree6[0].children[0].children[0] is CommentNode) + assertEquals(3, tree6[0].children[0].children[0].getId()) + } } From 5051335fb2ef485ed3d5a0ecf214c220e61c9321 Mon Sep 17 00:00:00 2001 From: "maarten.vercruysse" Date: Sun, 1 Oct 2023 04:19:02 +0200 Subject: [PATCH 3/5] Add more tests --- app/src/main/java/com/jerboa/Utils.kt | 12 +++++++++--- app/src/test/java/com/jerboa/UtilsKtTest.kt | 7 +++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/jerboa/Utils.kt b/app/src/main/java/com/jerboa/Utils.kt index d2c49e635..b31d3c4c5 100644 --- a/app/src/main/java/com/jerboa/Utils.kt +++ b/app/src/main/java/com/jerboa/Utils.kt @@ -284,7 +284,8 @@ fun commentsToFlatNodes( * This function takes a list of comments and builds a tree from it * * In commentView it should be giving a id of the root comment - * Else it would generate a + * Else it would generate a chain of missingCommentNodes to the first comment + * Because the commentView doesn't start with the actual root comment */ fun buildCommentsTree( comments: List, @@ -307,7 +308,7 @@ fun buildCommentsTree( map[cv.comment.id] = node } - val tree = ArrayDeque(comments.size) + val tree = mutableListOf() comments.forEach { cv -> val child = map[cv.comment.id] @@ -953,7 +954,12 @@ fun getCommentParentId(commentPath: String): Int? { } } -fun getParentPath(path: String) = path.split(".").dropLast(1).joinToString(".") +/** + * Returns the path of the parent + * + * Ex: 0.1.2.3 -> 0.1.2 + */ +fun getParentPath(path: String) = path.substringBeforeLast(".") fun getDepthFromComment(commentPath: String): Int { return commentPath.split(".").size.minus(2) diff --git a/app/src/test/java/com/jerboa/UtilsKtTest.kt b/app/src/test/java/com/jerboa/UtilsKtTest.kt index ec8aa71d6..4e615c266 100644 --- a/app/src/test/java/com/jerboa/UtilsKtTest.kt +++ b/app/src/test/java/com/jerboa/UtilsKtTest.kt @@ -309,4 +309,11 @@ class UtilsKtTest { assertTrue(tree6[0].children[0].children[0] is CommentNode) assertEquals(3, tree6[0].children[0].children[0].getId()) } + + @Test + fun testGetParentPath(){ + assertEquals("0", getParentPath("0.1")) + assertEquals("0.1", getParentPath("0.1.2")) + assertEquals("0.1.2", getParentPath("0.1.2.3")) + } } From c7b38becf0bc0166689bc8812751a974213ade63 Mon Sep 17 00:00:00 2001 From: "maarten.vercruysse" Date: Sun, 1 Oct 2023 04:26:45 +0200 Subject: [PATCH 4/5] Fix formatting --- app/src/main/java/com/jerboa/Utils.kt | 1 - app/src/test/java/com/jerboa/UtilsKtTest.kt | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/src/main/java/com/jerboa/Utils.kt b/app/src/main/java/com/jerboa/Utils.kt index b31d3c4c5..35f6ef904 100644 --- a/app/src/main/java/com/jerboa/Utils.kt +++ b/app/src/main/java/com/jerboa/Utils.kt @@ -80,7 +80,6 @@ import java.net.MalformedURLException import java.net.URL import java.text.DecimalFormat import java.util.* -import kotlin.collections.ArrayDeque import kotlin.math.abs import kotlin.math.pow diff --git a/app/src/test/java/com/jerboa/UtilsKtTest.kt b/app/src/test/java/com/jerboa/UtilsKtTest.kt index 4e615c266..e3bde5e85 100644 --- a/app/src/test/java/com/jerboa/UtilsKtTest.kt +++ b/app/src/test/java/com/jerboa/UtilsKtTest.kt @@ -311,7 +311,7 @@ class UtilsKtTest { } @Test - fun testGetParentPath(){ + fun testGetParentPath() { assertEquals("0", getParentPath("0.1")) assertEquals("0.1", getParentPath("0.1.2")) assertEquals("0.1.2", getParentPath("0.1.2.3")) From 2c2a0916949916c76ad25907cbf4319db107c1b7 Mon Sep 17 00:00:00 2001 From: "maarten.vercruysse" Date: Sun, 1 Oct 2023 18:28:49 +0200 Subject: [PATCH 5/5] woodpecker