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

feat: show total vote count in the footer. #104

Merged
merged 2 commits into from
Jan 22, 2025
Merged

Conversation

gianniguida
Copy link
Collaborator

@gianniguida gianniguida commented Jan 21, 2025

Fixes #0000

Changes proposed in this pull request:

  • Add total vote count in the footer of a global poll, once the user has voted or the poll has ended.
  • Don't show max votes once the user has voted or the poll has ended, that information is no longer relevant.

Reviewers should focus on:

Screenshot
image

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).

Required changes:

@gianniguida gianniguida requested a review from imorland January 21, 2025 14:44
@gianniguida gianniguida requested a review from a team as a code owner January 21, 2025 14:44
@DavideIadeluca DavideIadeluca added the enhancement New feature or request label Jan 22, 2025
Copy link
Member

@DavideIadeluca DavideIadeluca left a comment

Choose a reason for hiding this comment

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

While generally, I think this is a nice little addition, I would recommend putting this in a third-party extension as this component is fairly extensible.

The reason being that some communities might don't wanna show the total vote count by default or already show the total vote count before voting etc.

Not really for nor against this PR. Opinions @imorland ?

@gianniguida
Copy link
Collaborator Author

Or I can add a setting to toggle it on and off...

@DavideIadeluca
Copy link
Member

Yeah that would certainly be a possibility, but that also means more long term maintenance and complexity added here. To be honest the permissions/settings handling in polls is already a mess and unintuitive to use so I would hold back with that.

Copy link
Member

@imorland imorland left a comment

Choose a reason for hiding this comment

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

Nice work @gianniguida - I don't see a problem with this at all 👍

@imorland imorland merged commit 0a23d6e into master Jan 22, 2025
38 checks passed
@imorland imorland deleted the gg/add-total-count branch January 22, 2025 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants