8000 Add RHS informations card to show users votes by jbattistispiria · Pull Request #431 · matterpoll/matterpoll · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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.

Sign up for GitHub

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

Jump to bottom

Add RHS informations card to show users votes #431

Merged
merged 9 commits into from
Jul 16, 2022
Merged

Add RHS informations card to show users votes #431

merged 9 commits into from
Jul 16, 2022

Conversation

jbattistispiria
Copy link
Contributor
@jbattistispiria jbattistispiria commented May 31, 2022 •
edited
Loading

image

  • Add a new option when creating a poll «Show Voters».
  • Add error when anonymous option is use with show voters option.
  • Add unit tests

Change made after comments:

  • Add extra information when using the --progress setting

Show extra information on who voted for what during poll in the RHS feed.
The extra information is send in markdown by using a "card" props from the Mattermost API.
Mattermost post add the info icon to the post and create the RHS.

Fix #96

@kaakaa kaakaa self-requested a review June 15, 2022 13:40
@kaakaa kaakaa added Type/Enhancement New feature or request 3. To Review Docs/Needed Requires documentation Tech/Server Plugin Server labels Jun 15, 2022
@kaakaa kaakaa added this to the v1.6.0 milestone Jun 15, 2022
@codecov
Copy link
codecov bot commented Jun 15, 2022 •
edited
Loading

Codecov Report

Merging #431 (0bbbc56) into master (f7a2499) will increase coverage by 0.14%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #431      +/-   ##
==========================================
+ Coverage   93.63%   93.78%   +0.14%     
==========================================
  Files          14       14              
  Lines        1289     1320      +31     
==========================================
+ Hits         1207     1238      +31     
  Misses         55       55              
  Partials       27       27
Impacted Files Coverage Δ
server/plugin/api.go 91.25% <100.00%> (+0.15%) ⬆️
server/plugin/command.go 98.90% <100.00%> (+0.01%) ⬆️
server/poll/transform.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7a2499...0bbbc56. Read the comment docs.

kaakaa
kaakaa requested changes Jun 16, 2022
View reviewed changes
Copy link
Contributor
@kaakaa kaakaa left a comment

Choose a reason for hiding this comment

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

Great 👍 This feature works fine for me, and your changes almost looks great! I left a few review comments and would appreciate your response.

And Let me make one suggestion. How about including the functionality of --show-voters in the existing --progress setting? Separating those two poll settings would make --progress just display the number of votes on buttons, which does not seem useful. And I don't want to increase the number of poll setting as much as possible. How does this sounds for your use case?

kaakaa
kaakaa reviewed Jun 16, 2022
View reviewed changes
Copy link
Contributor
@kaakaa kaakaa left a comment

Choose a reason for hiding this comment

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

I forgot to send comments.

@jbattistispiria
Copy link
Contributor Author

About including the functionality of --show-voters in the existing --progress setting, i thought the same thing as you at first and i did the code that way, but i wasn't sure if people using --progress would want to have the extra information sidebar.
So i keep the two settings separated.
I'm open to make the change, let me know if that's go.

@kaakaa
Copy link
Contributor
kaakaa commented Jun 17, 2022

Thanks for sharing your thought, and I understand it. But I still be inclined to include this functionality in --progress. My thought is the following.

  • The card is not visible unless a user click on it, so it is not obtrusive
  • A poll with --progress makes the voting user name public as a result, so I don't think it will be a problem if the voting user name is made public during a poll is open
  • If a poll is created with --progress and --anonymous, with existing implementation, the voting user name will be hidden and anonymity is protected
    • In this case, a card doesn't have any useful information, but it would not be bad

If I seem to have missed something, let me know it.

@kaakaa kaakaa mentioned this pull request Jun 17, 2022
Merged
@jbattistispiria
Copy link
Contributor Author

Great. I'll completely agree with you and i will move show-voters with progress.

@jbattistispiria
Move --show-voters with --progress settings
bb1e003
@jbattistispiria
Update to use p.ConvertUserIDToDisplayName
7b0e5a3 
This converter add more informative on mouse over a name
@jbattistispiria
Replace localization function in transform.go
d0a0a2a 
Use bundle.LocalizeWithConfig instead of MustLocalize,
because MustLocalize causes panic if translation is missing.
@jbattistispiria
Copy link
Contributor Author
jbattistispiria commented Jun 20, 2022 •
edited
Loading

I made the change to include the functionality of --show-voters in the existing --progress setting.

kaakaa
kaakaa reviewed Jun 21, 2022
View reviewed changes
Copy link
Contributor
@kaakaa kaakaa left a comment

Choose a reason for hiding this comment

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

Thanks 👍 There are 2 additional points that I would like to fix, so please approve them if they looks good.

kaakaa
kaakaa approved these changes Jun 22, 2022
View reviewed changes
Copy link
Contributor
@kaakaa kaakaa left a comment

Choose a reason for hiding this comment

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

Perfect 👍

@kaakaa kaakaa added 4. To Release Do Not Merge Should not be merged until this label is removed and removed 3. To Review labels Jun 22, 2022
@kaakaa
Copy link
Contributor
kaakaa commented Jun 22, 2022

Since Matterpoll is on a way to release v1.5.0, I'd like to wait to merge this PR until the release process moves forward. If the release process seems to take a long time, I'll cut a new branch for release-1.5 and merge this. I'm careful not to generate additional work for this PR.

@kaakaa kaakaa mentioned this pull request Jul 9, 2022
Closed
@kaakaa
Copy link
Contributor
kaakaa commented Jul 16, 2022

Since there is no activity for a month in mattermost/mattermost-marketplace#298, move forward this PR. Thanks again for your contribution @jbattistispiria!

@kaakaa kaakaa merged commit a5fdb7d into matterpoll:master Jul 16, 2022
@kaakaa kaakaa removed the Do Not Merge Should not be merged until this label is removed label Jul 16, 2022
@hanzei
Copy link
Member
hanzei commented Sep 18, 2022

@jbattistispiria This is great, thanks for the contribution. 🎉

Quick q: Does this feature work on mobile?

@kaakaa
Copy link
Contributor
kaakaa commented Sep 21, 2022

No. AFAIK, card props is not supported on Mobile, so RHS information cannot be seen on mobile.

@metsuke0 metsuke0 mentioned this pull request Oct 25, 2022
Open
@q-wertz q-wertz mentioned this pull request Mar 9, 2023
Closed
This was referenced Apr 14, 2023
Closed
Closed
kaakaa added a commit that referenced this pull request Apr 14, 2023
@kaakaa
Update readme for #431
3ee804b
kaakaa added a commit that referenced this pull request May 2, 2023
@kaakaa
Updates for 1.6 (#479)
5bbd0f7 
* docs: update how to install goi18n

* Update readme for #431

* bump up version

* clean up readme
@physiker05
Copy link

Hi all! Although a bit outdated... I'd like to know how I get to see this card that shows me the users participating in an open poll? I started a poll using the option --progress, but I don't get to see the stats. Your help is very much appreciated!

@kaakaa kaakaa mentioned this pull request Apr 19, 2024
Open
@kaakaa
Copy link
Contributor
kaakaa commented Apr 23, 2024

@physiker05 I'm sorry that I didn't catch your comment.
First let me check, what version of matterpoll are you using? If you are using Matterpoll v1.6.0 or later, you should be able to see the icon to open the card by adding the --progress option.

open-card

@kaakaa kaakaa added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Apr 23, 2024
@physiker05
Copy link

@kaakaa Thanks for getting back to me! Well, you probably identified the main issue... I'm running on v1.4.0 😁.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaakaa kaakaa kaakaa approved these changes

Assignees

@jbattistispiria jbattistispiria

Labels
4. To Release Docs/Done Required documentation has been written Tech/Server Plugin Server Type/Enhancement New feature or request
Milestone
v1.6.0
Development

Successfully merging this pull request may close these issues.

Show how voted during poll via Poll Setting
4 participants
@jbattistispiria @kaakaa @hanzei @physiker05
0