Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Allow ending polls #7305

Merged
merged 20 commits into from
Dec 8, 2021
Merged

Allow ending polls #7305

merged 20 commits into from
Dec 8, 2021

Conversation

andybalaam
Copy link
Contributor

@andybalaam andybalaam commented Dec 7, 2021 •
edited by github-actions bot
Loading

This change allows us to end a poll and see the results highlighted and with a little trophy symbol:

end-poll

This change is marked as an internal change (Task), so will not be included in the changelog.

Preview: https://61b0c70c1a94ab2550cb7837--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@andybalaam andybalaam added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Dec 7, 2021
@andybalaam andybalaam requested a review from a team as a code owner December 7, 2021 16:58
t3chguy
t3chguy suggested changes Dec 7, 2021
View reviewed changes
res/css/views/messages/_MPollBody.scss Outdated Show resolved Hide resolved
res/css/views/messages/_MPollBody.scss Outdated Show resolved Hide resolved
res/css/views/messages/_MPollBody.scss Outdated Show resolved Hide resolved
src/components/views/context_menus/MessageContextMenu.tsx Outdated Show resolved Hide resolved
src/components/views/context_menus/MessageContextMenu.tsx Outdated Show resolved Hide resolved
src/components/views/messages/MessageActionBar.tsx Outdated Show resolved Hide resolved
src/components/views/messages/MessageActionBar.tsx Outdated Show resolved Hide resolved
src/components/views/messages/MPollBody.tsx Outdated Show resolved Hide resolved
src/components/views/messages/MPollBody.tsx Outdated Show resolved Hide resolved
src/components/views/messages/MPollBody.tsx Outdated Show resolved Hide resolved
andybalaam and others added 10 commits December 8, 2021 09:22
@andybalaam @t3chguy
Add space after comma in CSS
fc9b951 
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
@andybalaam @t3chguy
Add space after comma in CSS
5605afb 
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
@andybalaam @t3chguy
Add space after comma in CSS
006b645 
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
@andybalaam @t3chguy
Fix incorrect whitespace
d69196b 
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
@andybalaam @t3chguy
Add type declaration to canEndPoll
41d71c6 
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
@andybalaam @t3chguy
Fix incorrect whitespace
ed8a531 
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
@andybalaam @t3chguy
Add return type to isOnOrBeforeEnd
a3bf7f5 
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
@andybalaam
Provide return type for pollEndTs
ee129db
@andybalaam
Refactor poll options into functional React components
f90b28d
@andybalaam
Fix whitespace of OptionsButton and add a type declaration
9e8786c
gaelledel
gaelledel approved these changes Dec 8, 2021
View reviewed changes
Copy link

@gaelledel gaelledel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good stuff!
I wonder if we could tweak the radio button colour when unchecked to Dark/Quinary? https://www.figma.com/file/tepA3ELWBE5v4O2WB8PqWn/Polls?node-id=496%3A54285

And for the votes text to be right aligned with the bar with equal number of padding that you have on the left side. If it is 8px currently, can we do 12px on each side instead?
Screenshot 2021-12-08 at 12 31 07
Screenshot 2021-12-08 at 12 31 23

Like so
Screenshot 2021-12-08 at 12 37 54

@andybalaam
Copy link
Contributor Author

I wonder if we could tweak the radio button colour when unchecked to Dark/Quinary? figma.com/file/tepA3ELWBE5v4O2WB8PqWn/Polls?node-id=496%3A54285

Should it be Quinary in the Settings dialog too? They share a colour at the moment:
image

@andybalaam
Copy link
Contributor Author

And for the votes text to be right aligned with the bar with equal number of padding that you have on the left side. If it is 8px currently, can we do 12px on each side instead?

That's weird - it looks right for me:
image

Maybe it's a cross-browser issue. I'll look into it.

@andybalaam
Copy link
Contributor Author

Maybe it's a cross-browser issue. I'll look into it.

Ah no, my bad - it goes wrong when the poll is ended. Working on it.

@andybalaam
Copy link
Contributor Author

andybalaam commented Dec 8, 2021 •
edited
Loading

OK, @gaelledel I think I've completed the changes you asked for.
image
image
image
image

andybalaam and others added 2 commits December 8, 2021 13:19
@andybalaam
Re-order imports in EndPollDialog
bfd594a
@andybalaam @t3chguy
Apply suggestions from code review
61c048b 
Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
@gaelledel
Copy link

I wonder if we could tweak the radio button colour when unchecked to Dark/Quinary? figma.com/file/tepA3ELWBE5v4O2WB8PqWn/Polls?node-id=496%3A54285

Should it be Quinary in the Settings dialog too? They share a colour at the moment: image
Is it the same colour for Light and Dark ? E.g: Quartenary/Light - Quarternary/Dark?

@andybalaam
Copy link
Contributor Author

Is it the same colour for Light and Dark ? E.g: Quartenary/Light - Quarternary/Dark?

Yes, in both themes it is quaternary. My latest change makes it quinary in both. Is that right?

@andybalaam
Copy link
Contributor Author

Yes, in both themes it is quaternary. My latest change makes it quinary in both. Is that right?

We agreed to go back to quaternary for both themes. Done in f697a17

t3chguy
t3chguy approved these changes Dec 8, 2021
View reviewed changes
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

src/components/views/messages/MPollBody.tsx Outdated Show resolved Hide resolved
@andybalaam andybalaam enabled auto-merge (squash) December 8, 2021 14:51
@andybalaam andybalaam merged commit 2b52e17 into matrix-org:develop Dec 8, 2021
@turt2live turt2live mentioned this pull request Jan 3, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@t3chguy t3chguy t3chguy approved these changes

@gaelledel gaelledel gaelledel approved these changes

Assignees
No one assigned
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andybalaam @gaelledel @t3chguy