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

[Story] Reply to a poll #1976

Closed
4 of 5 tasks
Tracked by #2020
julioromano opened this issue Aug 8, 2023 · 11 comments
Closed
4 of 5 tasks
Tracked by #2020

[Story] Reply to a poll #1976

julioromano opened this issue Aug 8, 2023 · 11 comments
Assignees
@alfogrillo
Labels
App: ElementX Android App: ElementX iOS T-User Story Team: Element X Feature X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA

Comments

@julioromano
Copy link

julioromano commented Aug 8, 2023 •
edited
Loading

Description

  • As a user.
  • I want to reply to a poll.
  • So that I can comment on it without necessarily voting.

Acceptance criteria

  • Long-pressing a poll in the timeline must bring up the "message actions" bottom sheet.
  • Timeline menu (aka "bottom sheet"):
    • Must show the poll preview in the composer.
    • Must not show the "forward" action
    • Must not show the "edit" action
    • Must not show the "copy" action
    • Screen readers must properly read aloud the poll preview
    • Works with dark and light theme
    • Other existing options in the bottom sheet are verified to work
  • On replying:
    • The reply in the timeline includes the poll preview.
  • Poll preview:
    • Contains (as usual) the sender of the replied message
    • Contains the poll's question and the poll icon (design).
  • Backward compatibility
    • Replies to polls are rendered as such on legacy Element clients

Size estimate

S

Dependencies

Preconditions

No response

Out of scope

  • Showing the poll preview in the room list.
  • Showing the edit or end poll actions.

Open questions

No response

Subtasks

Android
Preview Give feedback
  1. Reply to a poll vector-im/element-x-android#1848
    julioromano
  2. Show poll question in timeline replies vector-im/element-x-android#1857
    julioromano

iOS
Preview Give feedback
No tasks being tracked yet.

Other
Preview Give feedback
  2. Fix fallback text field name for extensible events in RoomMessageEventContentWithoutRelation::make_reply_to_raw() ruma/ruma#1698
  3. Update ruma-events to 0.27.9 matrix-org/matrix-rust-sdk#2850
@julioromano julioromano mentioned this issue Aug 8, 2023
Closed
@julioromano
Copy link
Author

This can go to phase 2 if we're time constrained in phase 1

@julioromano julioromano mentioned this issue Aug 29, 2023
Closed
4 tasks
@alfogrillo
Copy link

Investigated a bit why replies aren't working for polls:
currently rust expects the reply to be for a m.room.message which have a msgtype and a body.
However the m.poll.start events don't have these fields so the parser fails.

There are at least two possible ways two fix:

  1. Change the logic inside the matrix-rust-sdk to address explicitly replies to other kind of messages other than m.room.message

  2. Make m.poll.start compliant to m.room.message. This means adding inside the content these two fields:

"body": "Poll start event description",
"msgtype": "m.text"

This is digested by the matrix-rust-sdk but it requires similar changes on the old clients to ensure interoperability.
However I think this approach is discouraged by the Poll MSC which says:

"Note that the extensible event fallbacks did not fall back to m.room.message in this MSC: this is deliberate to ensure polls are treated as first-class citizens. Client authors not willing/able to support polls are encouraged to instead support Extensible Events for better fallbacks."

fyi: @julioromano @langleyd

@julioromano
Copy link
Author

Re: ☝️

ruma/ruma#1646 plus some other rust sdk work should allow us to send replies for polls too.

@alfogrillo
Copy link

Replies to poll seem to work on EXI (and maybe also on EXA fyi: @julioromano)
However there is no specific UI in the composer when replying a to a poll (it's like replying to a text message).
@amshakal or @callumu do we have a design for it?

@callumu
Copy link

callumu commented Nov 8, 2023

Replies to poll seem to work on EXI (and maybe also on EXA fyi: @julioromano) However there is no specific UI in the composer when replying a to a poll (it's like replying to a text message). @amshakal or @callumu do we have a design for it?

I added something quickly here using the new poll icon.

Let me know if there are any issues / concerns

@alfogrillo
Copy link

Replies to poll seem to work on EXI (and maybe also on EXA fyi: @julioromano) However there is no specific UI in the composer when replying a to a poll (it's like replying to a text message). @amshakal or @callumu do we have a design for it?

I added something quickly here using the new poll icon.

Let me know if there are any issues / concerns

I guess we should use the same new icon everywhere in the timeline?
We also have an icon for the "ended poll" (with a little checkmark).
Is it gonna change as well?

@callumu
Copy link

callumu commented Nov 8, 2023 •
edited
Loading

Replies to poll seem to work on EXI (and maybe also on EXA fyi: @julioromano) However there is no specific UI in the composer when replying a to a poll (it's like replying to a text message). @amshakal or @callumu do we have a design for it?

I added something quickly here using the new poll icon.
Let me know if there are any issues / concerns

I guess we should use the same new icon everywhere in the timeline? We also have an icon for the "ended poll" (with a little checkmark). Is it gonna change as well?

Yes but won't they be automatically updated when the icons in Compound change? Here are the new icons:

Polls

Poll ended

@alfogrillo
Copy link

Replies to poll seem to work on EXI (and maybe also on EXA fyi: @julioromano) However there is no specific UI in the composer when replying a to a poll (it's like replying to a text message). @amshakal or @callumu do we have a design for it?

I added something quickly here using the new poll icon.
Let me know if there are any issues / concerns

I guess we should use the same new icon everywhere in the timeline? We also have an icon for the "ended poll" (with a little checkmark). Is it gonna change as well?

Yes but won't they be automatically updated when the icons in Compound change? Here are the new icons:

Polls

Poll ended

Unfortunately there is manual work to be done:

We need to

  • Update the library manually to use the new icons
  • Update any snapshot / ui tests to match new icons

@alfogrillo
Copy link

@callumu apparently the new icons have not been release yet?
The latest update I can found is here

@callumu
Copy link

callumu commented Nov 9, 2023

@callumu apparently the new icons have not been release yet? The latest update I can found is here

Okay. Is it easier to wait for them to be updated before adding them?

@julioromano
Copy link
Author

Replies to poll seem to work on EXI (and maybe also on EXA fyi: @julioromano) However there is no specific UI in the composer when replying a to a poll (it's like replying to a text message). @amshakal or @callumu do we have a design for it?

Replies on EXA are disabled due to #1976 (comment) as it was decided back then.

Since the rust work linked above has been done we can enable them as soon as we start working on this story.

@alfogrillo alfogrillo self-assigned this Nov 14, 2023
@julioromano julioromano mentioned this issue Nov 21, 2023
Merged
14 tasks
julioromano pushed a commit to element-hq/element-x-android that referenced this issue Nov 21, 2023
Reply to a poll
c787462 
User story: element-hq/element-meta#1976
@julioromano julioromano self-assigned this Nov 21, 2023
julioromano pushed a commit to element-hq/element-x-android that referenced this issue Nov 22, 2023
Reply to a poll (#1848)
0f0bd34 
<!-- Please read [CONTRIBUTING.md](https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md) before submitting your pull request -->
 
## Type of change

- [x] Feature
- [ ] Bugfix
- [ ] Technical
- [ ] Other :

## Content

Polls can now be replied to.

## Motivation and context

User story: element-hq/element-meta#1976

## Screenshots / GIFs

<!--
We have screenshot tests in the project, so attaching screenshots to a PR is not mandatory, as far as there
is a Composable Preview covering the changes. In this case, the change will appear in the file diff.
Note that all the UI composables should be covered by a Composable Preview.

Providing a video of the change is still very useful for the reviewer and for the history of the project.

You can use a table like this to show screenshots comparison.
Uncomment this markdown table below and edit the last line `|||`:
|copy screenshot of before here|copy screenshot of after here|

|Before|After|
|-|-|
|||
 -->

## Tests

<!-- Explain how you tested your development -->

- Step 1
- Step 2
- Step ...

## Tested devices

- [ ] Physical
- [ ] Emulator
- OS version(s):

## Checklist

<!-- Depending on the Pull Request content, it can be acceptable if some of the following checkboxes stay unchecked. -->

- [ ] Changes have been tested on an Android device or Android emulator with API 23
- [ ] UI change has been tested on both light and dark themes
- [ ] Accessibility has been taken into account. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
- [ ] Pull request is based on the develop branch
- [ ] Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#changelog
- [ ] Pull request includes screenshots or videos if containing UI changes
- [ ] Pull request includes a [sign off](https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#sign-off)
- [ ] You've made a self review of your PR
@langleyd langleyd added the X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA label Nov 28, 2023
@julioromano julioromano removed their assignment Dec 1, 2023
@manuroe manuroe closed this as completed Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alfogrillo alfogrillo

Labels
App: ElementX Android App: ElementX iOS T-User Story Team: Element X Feature X-Needs-Signoff Stories and Epics which are ready for review by product, design and QA
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@julioromano @langleyd @manuroe @alfogrillo @callumu