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 voting for multiple options without change perm #78

Merged
merged 4 commits into from
Jul 21, 2023

Conversation

dsevillamartin
Copy link
Member

@dsevillamartin dsevillamartin commented Jul 21, 2023

Fixes #76

Changes proposed in this pull request:

  • Add 'Vote' button that appears only if vote allows multiple votes & user cannot change vote
  • Make help text & vote button sticky to bottom of poll options (to help users not miss it!)
  • Add poll option to prevent users from changing votes in poll
  • Fix poll creation not saving "hide votes" perm from 926bd7c (Hide votes in poll until poll expires #2)

Reviewers should focus on:

  • Not 100% sure if the sticky works on mobile? The mobile responsive emulator on firefox seemed to ignore it. I don't think it's that big of an issue.
  • A potential problem is that navigating to another Flarum page will not alert the user that they never voted. I'm wary of using onbeforeremove for this since that could alert potentially in other situations. I believe this could maybe be done altering router? That may be too complicated.

Screenshot
image
image
image

Confirmed

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

@Wlork
Copy link

Wlork commented Jul 21, 2023

Hello @dsevillamartin :)

Looks like a great job, as usual !

However, I have several questions:

  • I'm puzzled about fixing the "Vote" button. I understand the idea, but I like to see all options directly before voting.

The height will be adjustable ?
And how it'll work with images ?

  • The button will only appear when there is a multiple choice ? If only one choice is set (with non vote change) is activated, the button will appear too ?

Thank you in advance :)

@dsevillamartin
Copy link
Member Author

@Wlork I think a video will answer most of these questions. For the last, the button only appears if it's multiple choice & the user cannot change their vote. This functionality isn't necessary if the user can only select one vote or if they can change votes at any point.

Only.Poll.-.Flarum.New.mp4

@Wlork
Copy link

Wlork commented Jul 21, 2023

@dsevillamartin Thank for video :)

I think a video will answer most of these questions.

Yes, I confirm, I prefer to have all options directly rather than scrolling.... Perhaps, if possible, an option in Admin to show a part of options or all of options ?

This functionality isn't necessary if the user can only select one vote or if they can change votes at any point.

OK 👍 If there is only one choice, there will be the alert at the bottom "You cannot change your vote after voting" ? I can't see on your screenshot ?

@dsevillamartin
Copy link
Member Author

@Wlork Not sure what you mean by have all the options directly? You have to scroll without this PR, and you still do with it. You're just given the info on the poll in advance without needing to reach the bottom first.

OK 👍 If there is only one choice, there will be the alert at the bottom "You cannot change your vote after voting" ? I can't see on your screenshot ?

If users can only vote for one choice, it will still say that they cannot change their vote.

@Wlork
Copy link

Wlork commented Jul 21, 2023

@dsevillamartin

Not sure what you mean by have all the options directly? You have to scroll without this PR, and you still do with it. You're just given the info on the poll in advance without needing to reach the bottom first.

Yes. It may sound crazy, but I prefer scroll down the page and see all options than scroll down a poll 😄
So, if we could choose, that would be great..... If it's not possible, that's fine :)

If users can only vote for one choice, it will still say that they cannot change their vote.

👍

@dsevillamartin
Copy link
Member Author

Yes. It may sound crazy, but I prefer scroll down the page and see all options than scroll down a poll 😄

I'm not sure what you mean by this. I don't understand where you want the poll options to appear?
I don't have plans on modifying this UI any further, but I'd like to visualize what you're describing.

@dsevillamartin
Copy link
Member Author

@Wlork Oh, if you think the poll is now in its own scrollable container - it's not. You still scroll the page to view more options, it's just that the bottom is now affixed to the bottom of the screen until you pass it, so it can show important poll info.

@Wlork
Copy link

Wlork commented Jul 21, 2023

@dsevillamartin

Oh, right !

It wasn't obvious on video, so thanks for the explanation. I'm reassured now :)

@dsevillamartin dsevillamartin merged commit b326cff into master Jul 21, 2023
@dsevillamartin dsevillamartin deleted the ds/allow-multiple-votes-without-change-perm branch July 21, 2023 21:06
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.

"Change vote" permission clashes with multiple vote polls
2 participants