Skip to content

Comments

feat(poll): improve poll card and dialog UI#16137

Open
DorraJaouad wants to merge 4 commits intomainfrom
feat/noid/enhance-poll-layout
Open

feat(poll): improve poll card and dialog UI#16137
DorraJaouad wants to merge 4 commits intomainfrom
feat/noid/enhance-poll-layout

Conversation

@DorraJaouad
Copy link
Contributor

@DorraJaouad DorraJaouad commented Oct 12, 2025

☑️ Resolves

  • It may not be obvious for users if the poll is anonymous nor if it is multiple choice (only after trying to select more)
  • It is hard to visually detect if the poll is open.

Improvement for upstream lib regarding NcChip: nextcloud-libraries/nextcloud-vue#7652

  • add variant prop values to showcase success, warningm error state like we have for other components
  • It lacks inline end space

⚠️ A problem: due to the fact that we don't have a trigger to update poll data (someone voted) when poll is anonymous, moderators are not able to see the number of voters until they reload.

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏚️ Not voted 🏡 voted 🏁 closed
image image image
image

🏁 Checklist

  • 🌏 Tested with different browsers / clients:
    • Chromium (Chrome / Edge / Opera / Brave)
    • Firefox
    • Safari
    • Talk Desktop
    • Integrations with Files sidebar and other apps
    • Not risky to browser differences / client
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

@DorraJaouad

This comment was marked as resolved.

Antreesy

This comment was marked as resolved.

@DorraJaouad

This comment was marked as resolved.

@DorraJaouad DorraJaouad force-pushed the feat/noid/enhance-poll-layout branch from 2f7924b to 966f993 Compare October 21, 2025 08:58
@DorraJaouad DorraJaouad force-pushed the feat/noid/enhance-poll-layout branch from 966f993 to 3aebb75 Compare November 20, 2025 15:05
@DorraJaouad DorraJaouad requested a review from Antreesy November 20, 2025 15:07
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Otherwise ok

},

showPollStatus() {
return this.poll && Object.keys(this.poll).length > 0 && this.poll.status !== POLL.STATUS.DRAFT
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return this.poll && Object.keys(this.poll).length > 0 && this.poll.status !== POLL.STATUS.DRAFT
return this.poll?.status && this.poll.status !== POLL.STATUS.DRAFT

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still thinking this can be done better. this.poll can not be an empty object {} to capture this case, and poll status is a mandatory field. Maybe

Suggested change
return this.poll && Object.keys(this.poll).length > 0 && this.poll.status !== POLL.STATUS.DRAFT
return this.poll && this.poll.status !== POLL.STATUS.DRAFT

@nimishavijay

This comment was marked as resolved.

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

First commit has some comments, otherwise fine.

Second commit is good, I would even backport to 33 (after fixing votedSelf issue)

overflow-wrap: anywhere;

&--draft {
display: flex;
Copy link
Contributor

Choose a reason for hiding this comment

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

Something went off here

Current As before
Image Image

Copy link
Contributor

Choose a reason for hiding this comment

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

Still needs align-items: center;

Signed-off-by: Dorra Jaouad <dorra.jaoued7@gmail.com>
Signed-off-by: Dorra Jaouad <dorra.jaoued7@gmail.com>
@DorraJaouad DorraJaouad force-pushed the feat/noid/enhance-poll-layout branch from 8eb3ce1 to 0cbf6d5 Compare February 11, 2026 08:49
this.polls[token][poll.id] = poll

if (this.polls[token][poll.id]) {
Object.assign(this.polls[token][poll.id], poll)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure it's going to work well with Pinia internals. Better would be to:

// Some comment on why it's needed e.g. for chat-relay
this.polls[token][poll.id] = Object.assign({}, this.polls[token][poll.id], poll)

context.dispatch('updateConversationLastMessage', { token, lastMessage: message })
if (fromRealtime) {
const pollsStore = usePollsStore()
if (message.poll?.id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's going to complain later at TS

Suggested change
if (message.poll?.id) {
if ('poll' in message && message.poll.id) {

token,
pollId: message.messageParameters.poll.id,
})
if (message.poll?.id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's going to complain later at TS

Suggested change
if (message.poll?.id) {
if ('poll' in message && message.poll.id) {

pollFooterText() {
if (this.poll?.status === POLL.STATUS.OPEN) {
if (this.isPollOpen) {
return this.poll?.votedSelf.length > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

consider that it's also used three times, I'd also move it to separate computed for perf and readability:

Suggested change
return this.poll?.votedSelf.length > 0
selfHasVoted() {
return this.poll?.votedSelf?.length > 0
},

},

showPollStatus() {
return this.poll && Object.keys(this.poll).length > 0 && this.poll.status !== POLL.STATUS.DRAFT
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still thinking this can be done better. this.poll can not be an empty object {} to capture this case, and poll status is a mandatory field. Maybe

Suggested change
return this.poll && Object.keys(this.poll).length > 0 && this.poll.status !== POLL.STATUS.DRAFT
return this.poll && this.poll.status !== POLL.STATUS.DRAFT

overflow-wrap: anywhere;

&--draft {
display: flex;
Copy link
Contributor

Choose a reason for hiding this comment

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

Still needs align-items: center;

return calculateVotePercentage(votes, this.poll.numVoters)
},

pollImportance() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: is it really important? Maybe we can go with more neutral pollChipVariant?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants