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

Collapse comments by tapping directly on the comment body #398

Merged
merged 2 commits into from
Jun 5, 2023

Conversation

a1studmuffin
Copy link
Contributor

Users can now collapse comments by tapping anywhere on the comment body or header, and expand comments again by tapping on the comment header. This replaces the old behaviour (tap and hold on comment headers), which wasn't as intuitive for new users.

Tested on comment markdown text as well as "View source" comment text. Tested clickable links in the comment body and usernames in the comment header are still clickable.

…dy or header, and expand comments by tapping on the comment header. This replaces the old behaviour (tap and hold on comment headers), which wasn't as intuitive for new users.

Tested on comment markdown text as well as "View source" text.

Tested clickable links and usernames are still clickable.
@a1studmuffin a1studmuffin requested a review from dessalines as a code owner June 4, 2023 07:27
@@ -86,7 +86,7 @@ fun CommentNodeHeader(
score: Int,
myVote: Int?,
isModerator: Boolean,
onLongClick: () -> Unit = {}
onClick: () -> Unit = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow the convention on these components, which is to make sure there's no default. IE

onClick: () -> Unit

@@ -118,7 +118,8 @@ fun CommentNodeHeaderPreview() {
@Composable
fun CommentBody(
comment: Comment,
viewSource: Boolean
viewSource: Boolean,
onClick: () -> Unit = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

@@ -56,7 +56,7 @@ fun CommentMentionNodeHeader(
onPersonClick: (personId: Int) -> Unit,
score: Int,
myVote: Int?,
onLongClick: () -> Unit = {}
onClick: () -> Unit = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

@@ -56,7 +56,7 @@ fun CommentReplyNodeHeader(
onPersonClick: (personId: Int) -> Unit,
score: Int,
myVote: Int?,
onLongClick: () -> Unit = {}
onClick: () -> Unit = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

@@ -215,8 +215,8 @@ fun CommentOrPostNodeHeader(
bottom = MEDIUM_PADDING
)
.combinedClickable(
onLongClick = onLongClick,
onClick = {}
onLongClick = {},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you create a github issue for long-click voting? I think that'd be really nice to have.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -674,11 +674,13 @@ fun PreviewLines(
@Composable
fun MyMarkdownText(
markdown: String,
color: Color = MaterialTheme.colorScheme.onSurface
color: Color = MaterialTheme.colorScheme.onSurface,
onClick: () -> Unit = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

@dessalines
Copy link
Member

Once you get around to this, don't worry about the merge conflicts, I'll handle those.

@a1studmuffin
Copy link
Contributor Author

Had to touch a few more files for this due to not providing a default implementation for onClick parameters, let me know if the merge is a pain and I can rebase on head.

@dessalines
Copy link
Member

I got it, it'll be merged shortly after the CI passes. Thanks!

@dessalines dessalines merged commit 7eb3168 into LemmyNet:main Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants