-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add new option to hide voting when not allowed #113
base: master
Are you sure you want to change the base?
Add new option to hide voting when not allowed #113
Conversation
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.
Hello and thank you for the PR!
Looks good to me, but can you please remove the dist files from this? In order to keep merging as straightforward as possible, the js will be automatically built once merged :)
Sounds good! Let me know if that works! |
Hi @hackeresq, similarly like in #114 this PR has merge conflicts. Please reset the history to the first commit you made on this branch, amend it and force push without the dist files being changed. Thanks! |
It's been a while.. I had some thoughts about this, and I feel like adding another setting would make the already opaque permission/settings handling even worse. I think for the next major release of this extension we should make the extension more opinionated by default but improve the extensibility of it. Therefore, I'm actually inclined to close this PR because I'm not willing to maintain this. Thoughts @imorland? |
Fixes #32
Changes proposed in this pull request:
This adds a new option in extension settings that allows admins to hide voting options when the post/tag/user does not have permissions to vote.
Reviewers should focus on:
This only adds a new configuration option. But perhaps the logic to hide should be reviewed:
This same logic was added to the alternate layout as well.
Screenshot
Confirmed
composer test
).Required changes:
N/A