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

Disable or hide downvote actions when instance does not have downvotes enabled #717

Merged
merged 6 commits into from
Sep 7, 2023

Conversation

hjiangsu
Copy link
Member

@hjiangsu hjiangsu commented Sep 7, 2023

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:

  • Swipe gestures for downvotes for both posts and comments will throw an error indicating the instance has disabled downvotes
  • In the comfortable view, the downvote icon is hidden when downvotes are disabled
  • The comment button action downvote button is hidden when downvotes are disabled
  • The post page downvote action is disabled

Issue Being Fixed

Issue Number: #19

Screenshots / Recordings

Screenshots
simulator_screenshot_D8D6A0D9-DAE1-43EF-8233-E9BC41D3D106
simulator_screenshot_6D2771A9-121D-40A6-B32B-663859AC48B9
simulator_screenshot_4C823B94-3D33-4A4C-B0E4-DE4C722AD2B4
simulator_screenshot_4EC1D842-1372-4D8D-A720-B455C5E7BE0C

Checklist

  • Did you update CHANGELOG.md?
  • Did you use localized strings where applicable?
  • Did you add semanticLabels where applicable for accessibility? (N/A)

Copy link
Member

@micahmo micahmo left a 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!

@hjiangsu
Copy link
Member Author

hjiangsu commented Sep 7, 2023

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

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 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

@micahmo
Copy link
Member

micahmo commented Sep 7, 2023

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).

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.

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.

Yep, totally agreed on the decision here here. My comment was less about swipe gestures.

@hjiangsu
Copy link
Member Author

hjiangsu commented Sep 7, 2023

Alright, updated some logic to handle checking instance settings when adding/switching accounts, and also removed downvote on post page @micahmo

simulator_screenshot_09C19558-D060-4F22-AB7A-ACA0A7529143

Copy link
Member

@micahmo micahmo left a comment

Choose a reason for hiding this comment

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

LGTM!

Just a mental note that I will have to test this functionality with anonymous instances in #677.

EDIT: Well, duh, you can't vote at all if you're not logged in! Confirmed, these changes work fine with and don't conflict at all with #677.

@hjiangsu hjiangsu merged commit 01bb1c0 into develop Sep 7, 2023
@micahmo micahmo deleted the feature/disable-downvotes-on-instance branch September 8, 2023 03:05
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