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

QOL Remove unnecessary clickable areas from PostListingList #710

Merged
merged 2 commits into from
Jun 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -70,12 +70,17 @@ fun CommunityLink(
thumbnailSize: Int = ICON_THUMBNAIL_SIZE,
style: TextStyle = MaterialTheme.typography.bodyMedium,
onClick: (community: CommunitySafe) -> Unit,
clickable: Boolean = true,
showDefaultIcon: Boolean,
) {
Row(
verticalAlignment = Alignment.CenterVertically,
horizontalArrangement = Arrangement.spacedBy(spacing),
modifier = modifier.clickable { onClick(community) },
modifier = if (clickable) {
modifier.clickable { onClick(community) }
} else {
modifier
},
) {
community.icon?.let {
CircularIcon(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ fun PersonNamePreview() {
fun PersonProfileLink(
person: PersonSafe,
onClick: (personId: Int) -> Unit,
clickable: Boolean = true,
showTags: Boolean = false,
isPostCreator: Boolean = false,
isModerator: Boolean = false,
Expand All @@ -68,7 +69,11 @@ fun PersonProfileLink(
Row(
verticalAlignment = Alignment.CenterVertically,
horizontalArrangement = Arrangement.spacedBy(SMALL_PADDING),
modifier = Modifier.clickable { onClick(person.id) },
modifier = if (clickable) {
Modifier.clickable { onClick(person.id) }
} else {
Modifier
},
) {
if (showAvatar) {
person.avatar?.also {
Expand Down
14 changes: 4 additions & 10 deletions app/src/main/java/com/jerboa/ui/components/post/PostListing.kt
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,6 @@ fun PostListing(
onDownvoteClick(it)
},
onPostClick = onPostClick,
onCommunityClick = onCommunityClick,
onPersonClick = onPersonClick,
isModerator = isModerator,
showCommunityName = showCommunityName,
account = account,
Expand Down Expand Up @@ -972,8 +970,6 @@ fun PostListingList(
onUpvoteClick: (postView: PostView) -> Unit,
onDownvoteClick: (postView: PostView) -> Unit,
onPostClick: (postView: PostView) -> Unit,
onCommunityClick: (community: CommunitySafe) -> Unit,
onPersonClick: (personId: Int) -> Unit,
isModerator: Boolean,
showCommunityName: Boolean = true,
account: Account?,
Expand Down Expand Up @@ -1017,15 +1013,17 @@ fun PostListingList(
if (showCommunityName) {
CommunityLink(
community = postView.community,
onClick = onCommunityClick,
onClick = {},
clickable = false,
showDefaultIcon = false,
)
DotSpacer(0.dp)
}
PersonProfileLink(
person = postView.creator,
isModerator = isModerator,
onClick = onPersonClick,
onClick = {},
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is a good idea. Now users won't be able to go to a person's profile, or a community, from the post listing, at all.

@twizmwazin thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think people need or want to go to a users profile from the front page. If they want to, they can open the post and click through from there.

Take a look at the linked discussion thread, over 100 upvotes and almost 20 people commenting that this is also annoying them.

Copy link
Contributor

Choose a reason for hiding this comment

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

There was na issue opened about this, I'll link it here if I can find it. Users (including myself) accidentally fat-finger the small touch targets frequently and it is a bit of an annoyance. It is simple enough to open the post and then click the appropriate link from in there. An option wouldn't be a bad idea, which is what I think Boost does.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's the issue: #488

Copy link
Member

Choose a reason for hiding this comment

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

mmmk, I spose its fine, especially since this just affects the list version.

clickable = false,
color = MaterialTheme.colorScheme.onSurface.muted,
showAvatar = showAvatar,
)
Expand Down Expand Up @@ -1136,8 +1134,6 @@ fun PostListingListPreview() {
onUpvoteClick = {},
onDownvoteClick = {},
onPostClick = {},
onCommunityClick = {},
onPersonClick = {},
isModerator = false,
account = null,
showVotingArrowsInListView = true,
Expand All @@ -1162,8 +1158,6 @@ fun PostListingListWithThumbPreview() {
onUpvoteClick = {},
onDownvoteClick = {},
onPostClick = {},
onCommunityClick = {},
onPersonClick = {},
isModerator = false,
account = null,
showVotingArrowsInListView = true,
Expand Down