-
Notifications
You must be signed in to change notification settings - Fork 68
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
Disable or hide downvote actions when instance does not have downvotes enabled #717
Conversation
…ave downvotes enabled
…/disable-downvotes-on-instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me!
The only question I'd have is whether there's an inconsistency between hiding the button in some places and disabling it in others. I don't have a strong preference, but just wanted to mention it as a point of discussion. Otherwise, LGTM!
Just noticed that I was missing logic for checking the downvote setting when we switch accounts, so I'll add that in first before merging it in
I thought a bit about this too - I think hiding the buttons themselves make sense since that isn't an action that should be performed (in hindsight, maybe I can also just hide the downvote icon/value on the post too). However, for things like swipe gestures, I decided to keep the downvote action in and just show an error because the swipe gesture setting is somewhat of a "global" setting. It might potentially cause some confusion if the user has accounts on different instances where the downvote is enabled/disabled, and the swipe gesture settings change when they switch between instances |
Sounds good, that's what I was getting at. If we're going with hiding, we should be able to hide on the post too. I think it's a flex layout so hopefully everything should still look fine.
Yep, totally agreed on the decision here here. My comment was less about swipe gestures. |
Alright, updated some logic to handle checking instance settings when adding/switching accounts, and also removed downvote on post page @micahmo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Description
This PR hides downvote buttons and disables downvote actions when an instance does not have downvotes enabled. The following changes have been added/tested:
Issue Being Fixed
Issue Number: #19
Screenshots / Recordings
Checklist
semanticLabel
s where applicable for accessibility? (N/A)