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

[Poll] Render ended poll and improve UI in the timeline #1113

Merged
merged 17 commits into from Aug 25, 2023

Conversation

Florian14
Copy link
Contributor

@Florian14 Florian14 commented Aug 22, 2023

Successor of #1031 and continue element-hq/element-meta#1830

Content

  • Improve the UI to match the final design
  • Render ended poll on the start event

Note:

  • Poll events are still not produced with the current matrix sdk, screenshots have been taken with a custom sdk.
  • Timestamp overlapping the poll tile will be fixed in a separate PR.
  • I will write some tests to ensure a certain poll event produces the correct UI models.

Screenshots

poll_timeline_demo.webm

@github-actions
Copy link
Contributor

github-actions bot commented Aug 22, 2023

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/WrX9mr

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Patch coverage: 64.13% and project coverage change: +0.11% 🎉

Comparison is base (cbeab31) 57.06% compared to head (29cd8f2) 57.17%.
Report is 3 commits behind head on develop.

❗ Current head 29cd8f2 differs from pull request most recent head 7fc44e6. Consider uploading reports for the commit 7fc44e6 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1113      +/-   ##
===========================================
+ Coverage    57.06%   57.17%   +0.11%     
===========================================
  Files         1022     1021       -1     
  Lines        26266    26175      -91     
  Branches      5307     5322      +15     
===========================================
- Hits         14988    14965      -23     
+ Misses        8969     8890      -79     
- Partials      2309     2320      +11     
Files Changed Coverage Δ
...line/factories/event/TimelineItemContentFactory.kt 56.52% <ø> (ø)
.../factories/event/TimelineItemContentPollFactory.kt 8.33% <0.00%> (-4.72%) ⬇️
...ures/messages/impl/timeline/groups/Groupability.kt 57.69% <ø> (ø)
...tformatter/impl/DefaultRoomLastMessageFormatter.kt 95.16% <0.00%> (ø)
...entformatter/impl/DefaultTimelineEventFormatter.kt 0.00% <ø> (ø)
...ies/matrix/api/timeline/item/event/EventContent.kt 86.04% <ø> (ø)
.../timeline/item/event/TimelineEventContentMapper.kt 0.00% <ø> (ø)
...aries/designsystem/theme/components/RadioButton.kt 56.00% <25.00%> (-0.67%) ⬇️
.../timeline/components/event/TimelineItemPollView.kt 43.75% <50.00%> (+3.75%) ⬆️
...ment/android/libraries/matrix/api/poll/PollKind.kt 66.66% <50.00%> (-33.34%) ⬇️
... and 7 more

... and 35 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Florian14 Florian14 marked this pull request as ready for review August 22, 2023 13:57
@Florian14 Florian14 requested a review from a team as a code owner August 22, 2023 13:57
@Florian14 Florian14 requested review from jmartinesp and removed request for a team August 22, 2023 13:57
Copy link
Contributor

@jmartinesp jmartinesp left a comment

Choose a reason for hiding this comment

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

LGTM, I just have a comment about the previews modified.

RadioButton(selected = false, onClick = {})
RadioButton(selected = false, enabled = false, onClick = {})
}
Row(horizontalArrangement = Arrangement.spacedBy(6.dp)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, these were used so the designers could take a quick look in Showkase and verify the buttons look good in all possible states by looking at a single preview. Could we go back to displaying the 4 states at the same time?

If we want the interaction to work, we could use selected = checked in the first 2 components and selected = !checked in the next two?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I have an issue with the interactive mode on the RadioButton preview but the interaction works fine in Showkase

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I just triggered the screenshots action)

@sonarcloud
Copy link

sonarcloud bot commented Aug 25, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Florian14 Florian14 merged commit 360c24d into develop Aug 25, 2023
13 checks passed
@Florian14 Florian14 deleted the feature/fre/improve_poll_event_timeline_rendering branch August 25, 2023 08:14
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.

None yet

3 participants