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

Allow showing downvotes separately or hiding scores #185

Merged
merged 9 commits into from
Jul 20, 2023

Conversation

rsammelson
Copy link
Contributor

@rsammelson rsammelson commented Jul 2, 2023

Fixes #170 and fixes #135. When the setting is in the default mode, it should work the same as before.

upvote

downvote

no votes

selector

@rsammelson rsammelson changed the title Allowing showing downvotes separately Allow showing downvotes separately Jul 2, 2023
@rsammelson rsammelson marked this pull request as draft July 3, 2023 15:42
@rsammelson rsammelson marked this pull request as ready for review July 4, 2023 01:44
@rsammelson rsammelson marked this pull request as draft July 4, 2023 02:34
@rsammelson rsammelson mentioned this pull request Jul 4, 2023
@rsammelson rsammelson marked this pull request as ready for review July 4, 2023 02:57
@rsammelson rsammelson changed the title Allow showing downvotes separately Allow showing downvotes separately or hiding scores Jul 4, 2023
@rsammelson rsammelson mentioned this pull request Jul 4, 2023
@rsammelson rsammelson marked this pull request as draft July 4, 2023 17:56
@rsammelson
Copy link
Contributor Author

I accidentally ended up with this branched off of my changes from #209, so I'm going to wait until that is merged to get this one ready to merge.

@rsammelson rsammelson force-pushed the show-downvotes branch 5 times, most recently from 50aaf17 to a9927c2 Compare July 9, 2023 02:40
This was referenced Jul 10, 2023
@rsammelson
Copy link
Contributor Author

This is now waiting on #470 instead.

@rsammelson rsammelson marked this pull request as ready for review July 17, 2023 02:35
@rsammelson
Copy link
Contributor Author

@aeharding can you take a look at this?

src/helpers/vote.ts Outdated Show resolved Hide resolved
@aeharding
Copy link
Owner

@rsammelson I made some adjustment here, if you want to cherrypick or I can PR to yours.

aea3ff8

@rsammelson rsammelson force-pushed the show-downvotes branch 2 times, most recently from e26f230 to 96d83b6 Compare July 20, 2023 00:52
@rsammelson
Copy link
Contributor Author

I just saw your comment, I must have started working on it at the same time as you. I brought over some more changes from your commit in the most recent push.

@aeharding
Copy link
Owner

Oh oops!

Can we keep "cumulative"? I have that in e8861fb and I would add it but it looks like I don't have permission to push to your branch.

@rsammelson
Copy link
Contributor Author

rsammelson commented Jul 20, 2023

Yeah I can change it, I called it "net score" though which seems more precise

@aeharding
Copy link
Owner

"Total" would also work. I want to leave off "Score" though because it's not consistent with the other options to add for that one only

Do you know if there's some config you can adjust so I may push to this branch?

@rsammelson
Copy link
Contributor Author

Yeah it's a setting on my end but it says "access to secrets":

Which sounded scary so I didn't check it.

@rsammelson
Copy link
Contributor Author

I adjusted it now

@aeharding
Copy link
Owner

Please check that option. I don't know why github words it that way.

@rsammelson
Copy link
Contributor Author

Yeah on reading the help page the secrets can only possibly give modify access to other branches of the same repo, so it's not actually a security issue in this case

@rsammelson rsammelson force-pushed the show-downvotes branch 3 times, most recently from 79726b2 to b197dd4 Compare July 20, 2023 03:47
@rsammelson
Copy link
Contributor Author

When I rebased on to main a while ago I forgot to apply the cleanup I did to the new settings.

@aeharding aeharding merged commit 55e9fe8 into aeharding:main Jul 20, 2023
@aeharding
Copy link
Owner

Thanks. This looks good. I wonder if the downvotes should be hidden if none in separate mode. In the comments it gets a little busy with a bunch of zeroes that are not necessary. that's something for another pr though

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.

[Feature request] Show downvote counter Allow hiding scores
2 participants