Timeline UI | MessageShield Support #3240

BillCarsonFr · 2024-07-24T13:36:47Z

Fixes https://github.com/element-hq/crypto-internal/issues/345
Depends on matrix-org/matrix-rust-sdk#3679
Design: https://www.figma.com/design/2aqj9A5WRvGY3tcdTrBek4/Unencrypted-message-warning-in-Timeline?node-id=910-77038&t=pF5GGduo3EsrFG2Q-0

Content

Show E2E message shields.

Motivation and context

Screenshots / GIFs

Tests

Step 1
Step 2
Step ...

Tested devices

Physical
Emulator
OS version(s):

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 title will be used in the release note, it clearly define what will change for the user
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

github-actions · 2024-07-24T13:43:06Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/hkC7Xz

sonarcloud · 2024-07-25T08:30:59Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

frebib · 2024-07-25T10:35:44Z

I tried this out for nothing more than sheer curiosity, and there were a couple things I noticed that were a bit weird (aside from the fact that literally every encrypted message was marked as unauthentic)

The bubble on messages received from other users (on the left hand side) are the same colour as the background (at least in dark mode) so it looks pretty odd and like there is a lot of whitespace
The bubble sits above the timestamp, which balloons the timeline a LOT vertically, because there's an empty line at the bottom of every message where there wasn't one before

bmarty · 2024-08-14T07:58:43Z

...kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt

+private fun TimelineItem.Event.shieldPosition(): MessageShieldPosition {
+    val shield = this.messageShield ?: return MessageShieldPosition.None
+
+    // sdk returns raw human readable strings, add i18n support


@jorge Could the SDK return an enum instead?
Same for the color, it should be a enum (if the first enum is not enough) with a notion of criticity. "color" is a concept for the application (UI) level.

I think you should discuss this with @pixlwave (and not poor @jorge 😅 ).

Ah yeah I fixed this… You can now switch on the ShieldState::code which is an enum 😁

On EXI we map ShieldState to this EncryptionAuthenticity representation for our sanity.

@bmarty For completeness here is the view we use for the timestamp with send/encryption warnings: TimelineItemSendInfoLabel

And the PRs element-hq/element-x-ios#3051, element-hq/element-x-ios#3100 and element-hq/element-x-ios#3116

with a notion of criticity. "color" is a concept for the application (UI) level.

FWIW, both myself and @stefanceriu raised this here and here but the crypto team don't want to change it 🤷‍♂️

bmarty · 2024-08-14T15:18:47Z

...in/kotlin/io/element/android/features/messages/impl/timeline/components/MessageShieldView.kt

+        is MessageShield.UnverifiedIdentity -> CompoundIcons.Admin()
+        is MessageShield.UnknownDevice,
+        is MessageShield.UnsignedDevice -> CompoundIcons.HelpSolid()
+        is MessageShield.SentInClear -> CompoundIcons.KeyOff()


TODO: need to check this mapping.

This should be the final mapping: https://github.com/element-hq/element-x-ios/blob/cf35056c787fbae4a219e2a442738b6a84164a95/ElementX/Sources/Services/Timeline/TimelineItemContent/EncryptionAuthenticity.swift#L59-L65

(I realise now that I linked you to a bunch of PRs that iteratively changed it over time).

Thanks. I have updated the code.
But there is no shield at all, which is against the SDK recommendation here and here.

Now I have this (from the Preview):

@BillCarsonFr can you confirm please?

Also handle click on the timeline and information displayed on long click.

bmarty · 2024-08-14T16:06:51Z

Updated, maybe need to add a feature flag as per iOS app (?)

pixlwave · 2024-08-14T16:11:08Z

Maybe need to add a feature flag as per iOS app

Tbh the only reason I've kept the iOS flag around is so we could enable this on both apps at the same time. So entirely up do you, if you don't want to add one I'll remove it from iOS when this PR is merged.

codecov · 2024-08-14T16:12:57Z

Codecov Report

Attention: Patch coverage is 95.52239% with 6 lines in your changes missing coverage. Please review.

Project coverage is 82.61%. Comparing base (ca81e46) to head (cbc59e0).
Report is 35 commits behind head on develop.

Files	Patch %	Lines
...ies/designsystem/components/dialogs/AlertDialog.kt	80.00%	2 Missing and 2 partials ⚠️
...atures/messages/impl/timeline/TimelinePresenter.kt	75.00%	0 Missing and 1 partial ⚠️
.../impl/timeline/components/ATimelineItemEventRow.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3240      +/-   ##
===========================================
+ Coverage    82.56%   82.61%   +0.05%     
===========================================
  Files         1662     1666       +4     
  Lines        38992    39121     +129     
  Branches      4724     4746      +22     
===========================================
+ Hits         32193    32320     +127     
+ Misses        5133     5132       -1     
- Partials      1666     1669       +3

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

iOS impl: https://github.com/element-hq/element-x-ios/blob/develop/ElementX/Sources/Services/Timeline/TimelineItemContent/EncryptionAuthenticity.swift#L59-L65

…eShieldDialog`

Need to add content description on the shield icon.

sonarcloud · 2024-08-16T09:54:14Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

bmarty

Implementation should be equivalent to iOS, let's merge and iterate if necessary.

BillCarsonFr added the PR-Feature For a new feature label Jul 25, 2024

BillCarsonFr added 3 commits July 25, 2024 10:01

Timeline UI | MessageShield Support

524f20b

Message Shields - i18n

0748bc8

MessageShields | Fix test compilation

b3b90a3

BillCarsonFr force-pushed the feature/valere/message_shields branch from 1f0c532 to b3b90a3 Compare July 25, 2024 08:01

review: Konsist fix

44728cb

BillCarsonFr marked this pull request as ready for review July 25, 2024 08:22

BillCarsonFr requested a review from a team as a code owner July 25, 2024 08:22

BillCarsonFr requested review from jmartinesp and removed request for a team July 25, 2024 08:22

bmarty marked this pull request as draft August 12, 2024 10:32

bmarty reviewed Aug 14, 2024

View reviewed changes

bmarty mentioned this pull request Aug 14, 2024

Show Encryption Authenticity warnings on messages in the timeline. element-hq/element-x-ios#3051

Merged

Merge branch 'develop' into feature/valere/message_shields

5d10b1f

bmarty self-assigned this Aug 14, 2024

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 14, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 14, 2024

bmarty reviewed Aug 14, 2024

View reviewed changes

Iterate on shield mapping and rendering

faf1e7d

Also handle click on the timeline and information displayed on long click.

bmarty force-pushed the feature/valere/message_shields branch from 711b484 to faf1e7d Compare August 14, 2024 15:40

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 14, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 14, 2024

Update screenshots

0661cd2

bmarty marked this pull request as ready for review August 14, 2024 16:06

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 16, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 16, 2024

bmarty added 2 commits August 16, 2024 09:56

Make extension isCritical a val instead of a fun.

3676bca

Iterate on mapping MessageShield -> icon

4efa95b

iOS impl: https://github.com/element-hq/element-x-ios/blob/develop/ElementX/Sources/Services/Timeline/TimelineItemContent/EncryptionAuthenticity.swift#L59-L65

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 16, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 16, 2024

ElementBot and others added 4 commits August 16, 2024 08:07

Update screenshots

ced4f0c

Add test on TimelineEvents.ShowShieldDialog and `TimelineEvents.Hid…

3f90b02

…eShieldDialog`

Add UI test on clicking on message shield.

2116108

Need to add content description on the shield icon.

Add preview for message shield dialog.

459f60d

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 16, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Aug 16, 2024

Update screenshots

cbc59e0

bmarty added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Aug 16, 2024

github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Aug 16, 2024

bmarty approved these changes Aug 16, 2024

View reviewed changes

bmarty enabled auto-merge August 16, 2024 12:04

bmarty merged commit 21f2c5a into develop Aug 16, 2024
29 checks passed

bmarty deleted the feature/valere/message_shields branch August 16, 2024 12:25

pixlwave mentioned this pull request Aug 19, 2024

Show encryption authenticity warnings on messages in the timeline. element-hq/element-x-ios#3180

Merged

This was referenced Aug 19, 2024

[Task] Display message shield explanation #3262

Closed

[Task] Decorate untrusted messages with a shield #3261

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Timeline UI | MessageShield Support #3240

Timeline UI | MessageShield Support #3240

BillCarsonFr commented Jul 24, 2024 •

edited

Loading

github-actions bot commented Jul 24, 2024 •

edited

Loading

sonarcloud bot commented Jul 25, 2024

frebib commented Jul 25, 2024

bmarty Aug 14, 2024 •

edited

Loading

jmartinesp Aug 14, 2024

pixlwave Aug 14, 2024

pixlwave Aug 14, 2024

pixlwave Aug 14, 2024 •

edited

Loading

bmarty Aug 14, 2024

pixlwave Aug 14, 2024 •

edited

Loading

bmarty Aug 16, 2024

bmarty commented Aug 14, 2024

pixlwave commented Aug 14, 2024

codecov bot commented Aug 14, 2024 •

edited

Loading

sonarcloud bot commented Aug 16, 2024

bmarty left a comment •

edited

Loading

Timeline UI | MessageShield Support #3240

Timeline UI | MessageShield Support #3240

Conversation

BillCarsonFr commented Jul 24, 2024 • edited Loading

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented Jul 24, 2024 • edited Loading

sonarcloud bot commented Jul 25, 2024

Quality Gate passed

frebib commented Jul 25, 2024

bmarty Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

jmartinesp Aug 14, 2024

Choose a reason for hiding this comment

pixlwave Aug 14, 2024

Choose a reason for hiding this comment

pixlwave Aug 14, 2024

Choose a reason for hiding this comment

pixlwave Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

bmarty Aug 14, 2024

Choose a reason for hiding this comment

pixlwave Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

bmarty Aug 16, 2024

Choose a reason for hiding this comment

bmarty commented Aug 14, 2024

pixlwave commented Aug 14, 2024

codecov bot commented Aug 14, 2024 • edited Loading

Codecov Report

sonarcloud bot commented Aug 16, 2024

Quality Gate passed

bmarty left a comment • edited Loading

Choose a reason for hiding this comment

BillCarsonFr commented Jul 24, 2024 •

edited

Loading

github-actions bot commented Jul 24, 2024 •

edited

Loading

bmarty Aug 14, 2024 •

edited

Loading

pixlwave Aug 14, 2024 •

edited

Loading

pixlwave Aug 14, 2024 •

edited

Loading

codecov bot commented Aug 14, 2024 •

edited

Loading

bmarty left a comment •

edited

Loading