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.

Sign up for GitHub

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

Adding additional vote display modes #1378

Merged
merged 9 commits into from
Feb 23, 2024
Merged

Adding additional vote display modes #1378

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
db14038
Adding additional vote display modes
dessalines Feb 14, 2024
b3b9e9a
Adding a threshold.
dessalines Feb 14, 2024
fe4c0ee
Merge remote-tracking branch 'origin/main' into vote_percentages
dessalines Feb 14, 2024
2fceefb
Using label for comment count.
dessalines Feb 14, 2024
2c48cec
Merge branch 'main' into vote_percentages
dessalines Feb 19, 2024
5c4e896
Merge remote-tracking branch 'origin/main' into vote_percentages
dessalines Feb 19, 2024
0d690c3
Refactor to pass in instantscores.
dessalines Feb 21, 2024
e39a355
Fixing format percent.
dessalines Feb 21, 2024
387a674
Merge branch 'main' into vote_percentages
dessalines Feb 23, 2024
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
5 changes: 5 additions & 0 deletions app/src/main/java/com/jerboa/Constants.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,9 @@ package com.jerboa

const val DEBOUNCE_DELAY = 1000L
const val MAX_POST_TITLE_LENGTH = 200

/**
* Hides the downvote or percentage, if below this threshold
*/
const val SHOW_UPVOTE_PCT_THRESHOLD = .9F
const val VIEW_VOTES_LIMIT = 40L
9 changes: 9 additions & 0 deletions app/src/main/java/com/jerboa/datatypes/Others.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,15 @@ data class PostFeatureData(
val featured: Boolean,
)

// TODO this should be got rid of after https://github.com/LemmyNet/lemmy/issues/4449
enum class VoteDisplayMode {
Full,
ScoreAndUpvotePercentage,
UpvotePercentage,
Score,
HideAll,
}

/**
* Says which type of users can view which bottom app bar tabs.
*/
Expand Down
39 changes: 38 additions & 1 deletion app/src/main/java/com/jerboa/feat/Voting.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.jerboa.feat

import com.jerboa.datatypes.VoteDisplayMode

enum class VoteType(val value: Int) {
Upvote(1),
Downvote(-1),
Expand All @@ -19,7 +21,8 @@ data class InstantScores(
val newVote = newVote(this.myVote, voteAction)
// get original (up/down)votes, add (up/down)vote if (up/down)voted
val upvotes = this.upvotes - (if (this.myVote == 1) 1 else 0) + (if (newVote == 1) 1 else 0)
val downvotes = this.downvotes - (if (this.myVote == -1) 1 else 0) + (if (newVote == -1) 1 else 0)
val downvotes =
this.downvotes - (if (this.myVote == -1) 1 else 0) + (if (newVote == -1) 1 else 0)

return InstantScores(
myVote = newVote,
Expand All @@ -28,10 +31,44 @@ data class InstantScores(
score = upvotes - downvotes,
)
}

fun scoreOrPctStr(voteDisplayMode: VoteDisplayMode): String? {
return scoreOrPctStr(
score = score,
upvotes = upvotes,
downvotes = downvotes,
voteDisplayMode = voteDisplayMode,
)
}
}

// Set myVote to given action unless it was already set to that action, in which case we reset to 0
fun newVote(
oldVote: Int,
voteAction: VoteType,
): Int = if (voteAction.value == oldVote) 0 else voteAction.value

fun upvotePercent(
upvotes: Long,
downvotes: Long,
): Float {
return (upvotes.toFloat() / (upvotes + downvotes))
}

fun formatPercent(pct: Float): String {
val formatted = "%.0f".format(pct * 100F)
return "$formatted%"
dessalines marked this conversation as resolved.
Show resolved Hide resolved
}

private fun scoreOrPctStr(
score: Long,
upvotes: Long,
downvotes: Long,
voteDisplayMode: VoteDisplayMode,
): String? {
return when (voteDisplayMode) {
VoteDisplayMode.UpvotePercentage -> formatPercent(upvotePercent(upvotes, downvotes))
VoteDisplayMode.HideAll -> null
else -> score.toString()
}
}
27 changes: 22 additions & 5 deletions app/src/main/java/com/jerboa/model/SiteViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import com.jerboa.api.API
import com.jerboa.api.ApiState
import com.jerboa.api.DEFAULT_INSTANCE
import com.jerboa.api.toApiState
import com.jerboa.datatypes.VoteDisplayMode
import com.jerboa.db.entity.AnonAccount
import com.jerboa.db.entity.isAnon
import com.jerboa.db.repository.AccountRepository
Expand Down Expand Up @@ -130,7 +131,8 @@ class SiteViewModel(private val accountRepository: AccountRepository) : ViewMode
viewModelScope.launch {
viewModelScope.launch {
unreadAppCountRes = ApiState.Loading
unreadAppCountRes = API.getInstance().getUnreadRegistrationApplicationCount().toApiState()
unreadAppCountRes =
API.getInstance().getUnreadRegistrationApplicationCount().toApiState()
}
}
}
Expand Down Expand Up @@ -199,7 +201,10 @@ class SiteViewModel(private val accountRepository: AccountRepository) : ViewMode

fun showAvatar(): Boolean {
return when (val res = siteRes) {
is ApiState.Success -> res.data.my_user?.local_user_view?.local_user?.show_avatars ?: true
is ApiState.Success ->
res.data.my_user?.local_user_view?.local_user?.show_avatars
?: true

else -> true
}
}
Expand All @@ -211,10 +216,22 @@ class SiteViewModel(private val accountRepository: AccountRepository) : ViewMode
}
}

fun showScores(): Boolean {
// TODO this should probably be persisted rather than waited for
// For the current default, just use FullScores
fun voteDisplayMode(): VoteDisplayMode {
val defaultMode = VoteDisplayMode.Full
Copy link
Member Author

@dessalines dessalines Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Here's what makes this all temporary. When this actually comes back from the siteRes, then we can remove it.

I thought at first that we need to persist this setting, but I guess not.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we don't persist this? Don't we keep resetting it each time the user opens the app again?

Copy link
Member Author

Choose a reason for hiding this comment

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

The question is : Which local user settings should we be saving to our local database?

I think we only really need to save things required before the first fetch, so default_sort_type . But as for other things that come back with getSite ? I'm not sure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should discuss this, but not only the context of this setting but like sort options on each screen too.

For backwards compatibility it seems that we will need to save the above setting too unless it not persistenting for older versions is intended.

Also I don't think it is guaranteed that getSite has returned before feed loads? So initially it won't be correct occasionally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Also I don't think it is guaranteed that getSite has returned before feed loads? So initially it won't be correct occasionally?

Yep, it won't be correct until after GetSite has loaded. In practice I haven't seen a case where the any feed loads faster than GetSite tho.

In that SiteViewModel, the current things that could be persisted are admins, followList, voteDisplayMode, enableDownvotes, and showAvatars.

If we decide to persist them, we should do that as a later PR, all at once tho. I'm not 100% convinced its worth it... but we do already have everything set up (persisting them would work exactly like default_sort_type)

Copy link
Collaborator

@MV-GH MV-GH Feb 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yep, it won't be correct until after GetSite has loaded. In practice I haven't seen a case where the any feed loads faster than GetSite tho.

That must be very situational. I just tested 5 times on my phone and all 5 times it did the feed request and response before the site response. You can check this by checking in the log, the order for the responses

(request)

image

(response)

image

image

admins, followList, voteDisplayMode, enableDownvotes, and showAvatars.

I care less about those are more about things we don't persist at all, like the sort types on each screen (communityview, postview, ...)

Maybe we should devise a much more flexible system that doesn't require so much overhead (as code) for persisting more fields.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sort types on each screen should just use the default_sort_type , which is already saved in the DB.

We could add a string / JSON field for user settings, but I really don't like putting untyped things in the DB, as that makes migrations and many other things a nightmare.

Copy link
Collaborator

@MV-GH MV-GH Feb 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We can't reuse default sort because it is in a different context+ different enum (sometimes)

For example comment sort options
or sort options in saved/own profile would be different context where you wouldn't want to reuse the same sort as for the feed

I'm also not a fan of Json but I was more - thinking of additional table that has 1 to many relationship and holds acc id, fieldname, field state. Or something so that we wouldn't need migrations to add more fields to persist. I don't want to bloat Account to much. End goal would be that we just add single column to a data class and the infra would take care of everything else.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add default_comment_sort for users and sites eventually: LemmyNet/lemmy#4128

I'm not sure what would be best, but at least for now lets just add more columns to the account table as necessary.

return when (val res = siteRes) {
is ApiState.Success -> res.data.my_user?.local_user_view?.local_user?.show_scores ?: true
else -> true
is ApiState.Success ->
res.data.my_user?.let { mui ->
if (mui.local_user_view.local_user.show_scores) {
defaultMode
} else {
VoteDisplayMode.HideAll
}
} ?: run {
defaultMode
}
else -> defaultMode
}
}

Expand Down
33 changes: 18 additions & 15 deletions app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import com.jerboa.border
import com.jerboa.buildCommentsTree
import com.jerboa.calculateCommentOffset
import com.jerboa.datatypes.BanFromCommunityData
import com.jerboa.datatypes.VoteDisplayMode
import com.jerboa.datatypes.getContent
import com.jerboa.datatypes.sampleCommentView
import com.jerboa.datatypes.sampleCommunity
Expand Down Expand Up @@ -99,17 +100,22 @@ fun CommentNodeHeader(
commentView: CommentView,
onPersonClick: (personId: PersonId) -> Unit,
score: Long,
upvotes: Long,
downvotes: Long,
myVote: Int,
collapsedCommentsCount: Long,
isExpanded: Boolean,
onClick: () -> Unit,
onLongClick: () -> Unit,
showAvatar: Boolean,
showScores: Boolean,
voteDisplayMode: VoteDisplayMode,
) {
CommentOrPostNodeHeader(
creator = commentView.creator,
score = score,
upvotes = upvotes,
downvotes = downvotes,
voteDisplayMode = voteDisplayMode,
myVote = myVote,
published = commentView.comment.published,
updated = commentView.comment.updated,
Expand All @@ -122,7 +128,6 @@ fun CommentNodeHeader(
onClick = onClick,
onLongCLick = onLongClick,
showAvatar = showAvatar,
showScores = showScores,
isDistinguished = commentView.comment.distinguished,
)
}
Expand All @@ -133,14 +138,16 @@ fun CommentNodeHeaderPreview() {
CommentNodeHeader(
commentView = sampleCommentView,
score = 23,
upvotes = 21,
downvotes = 2,
voteDisplayMode = VoteDisplayMode.Full,
myVote = 26,
onPersonClick = {},
onClick = {},
onLongClick = {},
collapsedCommentsCount = 5,
isExpanded = false,
showAvatar = true,
showScores = true,
)
}

Expand Down Expand Up @@ -220,7 +227,7 @@ fun LazyListScope.commentNodeItem(
enableDownVotes: Boolean,
showAvatar: Boolean,
blurNSFW: BlurNSFW,
showScores: Boolean,
voteDisplayMode: VoteDisplayMode,
swipeToActionPreset: SwipeToActionPreset,
) {
val commentView = node.commentView
Expand Down Expand Up @@ -333,6 +340,9 @@ fun LazyListScope.commentNodeItem(
onPersonClick = onPersonClick,
score = instantScores.score,
myVote = instantScores.myVote,
upvotes = instantScores.upvotes,
downvotes = instantScores.downvotes,
voteDisplayMode = voteDisplayMode,
onClick = {
onHeaderClick(commentView)
},
Expand All @@ -342,7 +352,6 @@ fun LazyListScope.commentNodeItem(
collapsedCommentsCount = commentView.counts.child_count,
isExpanded = isExpanded(commentId),
showAvatar = showAvatar,
showScores = showScores,
)
AnimatedVisibility(
visible = isExpanded(commentId) || showCollapsedCommentContent,
Expand Down Expand Up @@ -409,7 +418,6 @@ fun LazyListScope.commentNodeItem(
},
account = account,
enableDownVotes = enableDownVotes,
showScores = showScores,
viewSource = viewSource,
)
}
Expand Down Expand Up @@ -484,7 +492,7 @@ fun LazyListScope.commentNodeItem(
enableDownVotes = enableDownVotes,
showAvatar = showAvatar,
blurNSFW = blurNSFW,
showScores = showScores,
voteDisplayMode = voteDisplayMode,
admins = admins,
moderators = moderators,
swipeToActionPreset = swipeToActionPreset,
Expand Down Expand Up @@ -531,7 +539,7 @@ fun LazyListScope.missingCommentNodeItem(
enableDownVotes: Boolean,
showAvatar: Boolean,
blurNSFW: BlurNSFW,
showScores: Boolean,
voteDisplayMode: VoteDisplayMode,
swipeToActionPreset: SwipeToActionPreset,
) {
val commentId = node.missingCommentView.commentId
Expand Down Expand Up @@ -638,7 +646,7 @@ fun LazyListScope.missingCommentNodeItem(
enableDownVotes = enableDownVotes,
showAvatar = showAvatar,
blurNSFW = blurNSFW,
showScores = showScores,
voteDisplayMode = voteDisplayMode,
swipeToActionPreset = swipeToActionPreset,
)
}
Expand Down Expand Up @@ -765,7 +773,6 @@ fun CommentFooterLine(
onClick: () -> Unit,
onLongClick: () -> Unit,
account: Account,
showScores: Boolean,
viewSource: Boolean,
) {
var showMoreOptions by remember { mutableStateOf(false) }
Expand Down Expand Up @@ -838,18 +845,14 @@ fun CommentFooterLine(
) {
VoteGeneric(
myVote = instantScores.myVote,
votes = instantScores.upvotes,
type = VoteType.Upvote,
onVoteClick = onUpvoteClick,
showNumber = (instantScores.downvotes != 0L) && showScores,
account = account,
)
if (enableDownVotes) {
VoteGeneric(
myVote = instantScores.myVote,
votes = instantScores.downvotes,
type = VoteType.Downvote,
showNumber = showScores,
onVoteClick = onDownvoteClick,
account = account,
)
Expand Down Expand Up @@ -944,7 +947,7 @@ fun CommentNodesPreview() {
showAvatar = true,
blurNSFW = BlurNSFW.NSFW,
account = AnonAccount,
showScores = true,
voteDisplayMode = VoteDisplayMode.Full,
swipeToActionPreset = SwipeToActionPreset.DEFAULT,
)
}
Expand Down
11 changes: 6 additions & 5 deletions app/src/main/java/com/jerboa/ui/components/comment/CommentNodes.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.jerboa.CommentNode
import com.jerboa.CommentNodeData
import com.jerboa.MissingCommentNode
import com.jerboa.datatypes.BanFromCommunityData
import com.jerboa.datatypes.VoteDisplayMode
import com.jerboa.db.entity.Account
import com.jerboa.feat.BlurNSFW
import com.jerboa.feat.SwipeToActionPreset
Expand Down Expand Up @@ -66,7 +67,7 @@ fun CommentNodes(
enableDownVotes: Boolean,
showAvatar: Boolean,
blurNSFW: BlurNSFW,
showScores: Boolean,
voteDisplayMode: VoteDisplayMode,
swipeToActionPreset: SwipeToActionPreset,
) {
LazyColumn(state = listState) {
Expand Down Expand Up @@ -110,7 +111,7 @@ fun CommentNodes(
enableDownVotes = enableDownVotes,
showAvatar = showAvatar,
blurNSFW = blurNSFW,
showScores = showScores,
voteDisplayMode = voteDisplayMode,
swipeToActionPreset = swipeToActionPreset,
)
item {
Expand Down Expand Up @@ -159,7 +160,7 @@ fun LazyListScope.commentNodeItems(
enableDownVotes: Boolean,
showAvatar: Boolean,
blurNSFW: BlurNSFW,
showScores: Boolean,
voteDisplayMode: VoteDisplayMode,
swipeToActionPreset: SwipeToActionPreset,
) {
nodes.forEach { node ->
Expand Down Expand Up @@ -205,7 +206,7 @@ fun LazyListScope.commentNodeItems(
enableDownVotes = enableDownVotes,
showAvatar = showAvatar,
blurNSFW = blurNSFW,
showScores = showScores,
voteDisplayMode = voteDisplayMode,
swipeToActionPreset = swipeToActionPreset,
)

Expand Down Expand Up @@ -250,7 +251,7 @@ fun LazyListScope.commentNodeItems(
enableDownVotes = enableDownVotes,
showAvatar = showAvatar,
blurNSFW = blurNSFW,
showScores = showScores,
voteDisplayMode = voteDisplayMode,
swipeToActionPreset = swipeToActionPreset,
)
}
Expand Down
Loading