[Feature] Render m.sticker events #2122

surakin · 2023-12-28T16:46:52Z

Render m.sticker events

Closes #1949

Type of change

Feature
Bugfix
Technical
Other :

Content

Added rendering of sticker events (both the event and the reply preview)

Motivation and context

This solves #1949.
I mostly use matrix to talk via whatsapp bridges and this fixes having to switch to whatsapp just to see what sticker someone just sent.

Screenshots / GIFs

Tests

I sent myself stickers using Cinny web and Whatsapp and check them in both the emulator & my own phone

Tested devices

Physical
Emulator
OS version(s):
Android 13 & 14

Checklist

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/element-hq/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/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#changelog
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR

Signed-off-by: Marco Antonio Alvarez [email protected]

Closes element-hq#1949

jmartinesp · 2023-12-29T10:34:48Z

Thanks for the contribution!

The code seems to work quite well, although there are some minor issues with the way the sticker is displayed and the timestamp: in this when clause you can see how items with media (image, video, location, etc.) are displayed in an Overlay mode, the same thing should be done for stickers.
The issue with API 23 is being handled at the moment, don't worry about that check.
Also, you'll need to create a commit with a sign-off for this to be approved and merged.

Once you mark this as ready for review I'll give it some more thorough review.

Signed-off-by: Marco Antonio Alvarez <[email protected]>

…d into render_stickers

Signed-off-by: Marco Antonio Alvarez <[email protected]>

…o render_stickers

surakin · 2023-12-29T15:32:08Z

The code seems to work quite well, although there are some minor issues with the way the sticker is displayed and the timestamp: in this when clause you can see how items with media (image, video, location, etc.) are displayed in an Overlay mode, the same thing should be done for stickers.

I'm wondering if showing the timestamp below the sticker would be better, because stickers are quite small and you can't open them, so you'll always get some of it hidden by the timestamp overlay

Like this:

bmarty · 2023-12-29T15:44:35Z

Thanks for your contribution @surakin !

you can't open them

Maybe a onClick support to view the sticker in the MediaViewer? Element Android allows this (but not Element Web).

surakin · 2023-12-29T15:55:37Z

Thanks for your contribution @surakin !

you can't open them

Maybe a onClick support to view the sticker in the MediaViewer? Element Android allows this (but not Element Web).

Ah yes, that would do, trying that.

Signed-off-by: Marco Antonio Alvarez <[email protected]>

jmartinesp · 2024-01-02T09:58:05Z

@surakin sorry for the bumpy process, but I think it's the actual 1st contribution to be merged in the repo and we're seeing some issues with our CI processes and forks 😓 .

Since you made some changes to the previews we now need to update the screenshots, but that's proven to be problematic if it's done locally on your PC, so we use the CI for that. However, I'm not 100% sure on how to proceed when forks are involved.

Now that #2141 has been merged, could you either merge or rebase the latest changes in develop and run the Record screenshots for your branch in your repo? It's a bit weird because I can't see the GH action workflow displayed there, but maybe it's a permission issue.

Otherwise, maybe installing the Github CLI and using this command would generate the new screenshots in the uploaded branch:

gh workflow run recordScreenshots.yml --ref (git rev-parse --abbrev-ref HEAD)

jmartinesp · 2024-01-02T10:21:13Z

It worked, thanks! There are a lot of extra screenshots, but that's normal, given some issues on how Paparazzi names screenshots.

surakin · 2024-01-02T10:22:53Z

I think it's the actual 1st contribution to be merged in the repo

Am I the first? Yay me! 🥳

jmartinesp · 2024-01-02T11:02:29Z

@surakin it seems like the CI got stuck after the screenshots were generated and I can't approve it... Could you make some change, like a no-op commit or rebasing the existing content so it's triggered again?

surakin · 2024-01-02T11:08:02Z

did that help?

jmartinesp · 2024-01-02T11:09:41Z

did that help?

Yes, it's running now. Thank you again!

jmartinesp · 2024-01-02T11:33:45Z

It seems like there are some failing tests in :libraries:push:impl related to the stickers, could you fix those?

Signed-off-by: Marco Antonio Alvarez <[email protected]>

surakin · 2024-01-02T11:55:00Z

Ups, missed that one. Fixed.

codecov · 2024-01-02T12:17:35Z

Codecov Report

Attention: 48 lines in your changes are missing coverage. Please review.

Comparison is base (2134004) 67.05% compared to head (fdf29af) 67.02%.

Files	Patch %	Lines
...ctories/event/TimelineItemContentMessageFactory.kt	0.00%	12 Missing and 1 partial ⚠️
...meline/components/event/TimelineItemStickerView.kt	56.25%	2 Missing and 5 partials ⚠️
...ctories/event/TimelineItemContentStickerFactory.kt	63.15%	1 Missing and 6 partials ⚠️
...ndroid/features/messages/impl/MessagesPresenter.kt	0.00%	4 Missing and 1 partial ⚠️
.../messages/impl/timeline/model/InReplyToMetadata.kt	0.00%	4 Missing and 1 partial ⚠️
...e/components/event/TimelineItemEventContentView.kt	0.00%	2 Missing and 1 partial ⚠️
...s/messages/impl/timeline/model/InReplyToDetails.kt	0.00%	2 Missing and 1 partial ⚠️
...eatures/messages/impl/actionlist/ActionListView.kt	0.00%	1 Missing and 1 partial ⚠️
...timeline/model/event/TimelineItemStickerContent.kt	86.66%	1 Missing and 1 partial ⚠️
...tils/messagesummary/MessageSummaryFormatterImpl.kt	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2122      +/-   ##
===========================================
- Coverage    67.05%   67.02%   -0.04%     
===========================================
  Files         1371     1374       +3     
  Lines        34007    34116     +109     
  Branches      7487     7523      +36     
===========================================
+ Hits         22805    22867      +62     
- Misses        7587     7615      +28     
- Partials      3615     3634      +19

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

Signed-off-by: Marco Antonio Alvarez <[email protected]>

jmartinesp

Everything looks good now, thanks! The Maestro checks won't pass because of GH limitations of CI in forks, so I'll just skip them.

surakin · 2024-01-02T16:22:52Z

What about codecov? Should I worry about that? (first time I see it)

jmartinesp · 2024-01-02T16:23:56Z

No, it's ok. The coverage dropped a bit because a lot of code was added and only some of it could be tested, but it's no big deal. Thanks again!

surakin · 2024-01-02T16:24:54Z

Awesome! Happy to help 🥳

Render m.sticker events

96a1d5a

Closes element-hq#1949

surakin requested a review from a team as a code owner December 28, 2023 16:46

surakin requested review from jmartinesp and removed request for a team December 28, 2023 16:46

surakin marked this pull request as draft December 28, 2023 16:49

bmarty added the Z-Community-PR Issue is solved by a community member's PR label Dec 28, 2023

merge develop branch

d085e90

surakin mentioned this pull request Dec 28, 2023

App crashes on emulator with API level 23 #2124

Closed

Merge branch 'develop' into render_stickers

ae51ed7

surakin added 2 commits December 29, 2023 12:29

set sticker timestamp position to overlay

a977a6d

Signed-off-by: Marco Antonio Alvarez <[email protected]>

Merge branch 'render_stickers' of github.com:surakin/element-x-androi…

90543a7

…d into render_stickers

surakin marked this pull request as ready for review December 29, 2023 11:32

jmartinesp added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Dec 29, 2023

add TimelineItemContentStickerFactory missing parameters

13a4cc1

Signed-off-by: Marco Antonio Alvarez <[email protected]>

surakin changed the title ~~[Feature] Render m.sticker events [Draft]~~ [Feature] Render m.sticker events Dec 29, 2023

Merge branch 'develop' of github.com:element-hq/element-x-android int…

df71ca6

…o render_stickers

surakin added 2 commits December 29, 2023 16:57

open stickers in media viewer

847d5e2

Signed-off-by: Marco Antonio Alvarez <[email protected]>

Merge branch 'develop' into render_stickers

6f1a3d3

jmartinesp removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Dec 30, 2023

surakin and others added 2 commits January 2, 2024 10:59

Merge branch 'develop' into render_stickers

df392d4

Update screenshots

ff8d8d8

empty commit

da16b81

fix unit test

3761113

Signed-off-by: Marco Antonio Alvarez <[email protected]>

jmartinesp added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Jan 2, 2024

try to improve the coverage score

d1e4258

Signed-off-by: Marco Antonio Alvarez <[email protected]>

jmartinesp removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Jan 2, 2024

fix sticker test

fdf29af

Signed-off-by: Marco Antonio Alvarez <[email protected]>

jmartinesp approved these changes Jan 2, 2024

View reviewed changes

jmartinesp merged commit 87c8bc1 into element-hq:develop Jan 2, 2024
11 of 13 checks passed

surakin deleted the render_stickers branch January 2, 2024 16:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Render m.sticker events #2122

[Feature] Render m.sticker events #2122

surakin commented Dec 28, 2023 •

edited

Loading

jmartinesp commented Dec 29, 2023

surakin commented Dec 29, 2023 •

edited

Loading

bmarty commented Dec 29, 2023

surakin commented Dec 29, 2023

jmartinesp commented Jan 2, 2024

jmartinesp commented Jan 2, 2024

surakin commented Jan 2, 2024 •

edited

Loading

jmartinesp commented Jan 2, 2024

surakin commented Jan 2, 2024

jmartinesp commented Jan 2, 2024

jmartinesp commented Jan 2, 2024

surakin commented Jan 2, 2024

codecov bot commented Jan 2, 2024 •

edited

Loading

jmartinesp left a comment

surakin commented Jan 2, 2024

jmartinesp commented Jan 2, 2024

surakin commented Jan 2, 2024

[Feature] Render m.sticker events #2122

[Feature] Render m.sticker events #2122

Conversation

surakin commented Dec 28, 2023 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

jmartinesp commented Dec 29, 2023

surakin commented Dec 29, 2023 • edited Loading

bmarty commented Dec 29, 2023

surakin commented Dec 29, 2023

jmartinesp commented Jan 2, 2024

jmartinesp commented Jan 2, 2024

surakin commented Jan 2, 2024 • edited Loading

jmartinesp commented Jan 2, 2024

surakin commented Jan 2, 2024

jmartinesp commented Jan 2, 2024

jmartinesp commented Jan 2, 2024

surakin commented Jan 2, 2024

codecov bot commented Jan 2, 2024 • edited Loading

Codecov Report

jmartinesp left a comment

Choose a reason for hiding this comment

surakin commented Jan 2, 2024

jmartinesp commented Jan 2, 2024

surakin commented Jan 2, 2024

surakin commented Dec 28, 2023 •

edited

Loading

surakin commented Dec 29, 2023 •

edited

Loading

surakin commented Jan 2, 2024 •

edited

Loading

codecov bot commented Jan 2, 2024 •

edited

Loading