[Location sharing] - Show message on start of a live #5722

mnaturel · 2022-04-08T09:51:20Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Handling beacon state event in the timeline to indicate a live location share has been started. This state of the message will be displayed until a location is received. Adding support for preview of live location sharing message as well.

Motivation and context

Closes #5710

Screenshots / GIFs

Default layout mode

Bubble layout mode

UX

Tests

Ensure the location sharing is enabled in Settings -> Preferences
Go to a room
Press the add button on the new message view
Select the location icon
Select live location sharing option
Select a duration for the live
Validate
Check a message is displayed in the timeline to indicate the live has started

Tested devices

Physical
Emulator
OS version(s): Android 11

Checklist

Changes has been tested on an Android device or Android emulator with API 21
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/vector-im/element-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-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
If you have modified the screen flow, or added new screens to the application, you have updated the test UiAllScreensSanityTest.allScreensTest()

github-actions · 2022-04-08T10:25:56Z

Unit Test Results

114 files ±0 114 suites ±0 1m 31s ⏱️ +10s
201 tests ±0 201 ✔️ ±0 0 💤 ±0 0 ❌ ±0
674 runs ±0 674 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 071340c. ± Comparison against base commit 047a45d.

♻️ This comment has been updated with latest results.

mnaturel · 2022-04-08T13:45:55Z

...java/org/matrix/android/sdk/api/session/room/model/livelocation/LiveLocationBeaconContent.kt

 import org.matrix.android.sdk.api.session.room.model.message.LocationAsset
 import org.matrix.android.sdk.api.session.room.model.message.LocationAssetType
+import org.matrix.android.sdk.api.session.room.model.message.MessageContent
+import org.matrix.android.sdk.api.session.room.model.message.MessageType
+import org.matrix.android.sdk.api.session.room.model.relation.RelationDefaultContent

 @JsonClass(generateAdapter = true)
 data class LiveLocationBeaconContent(


The only way I found to be able to show the content as a message in the timeline is to inherit from MessageContent even though it does not correspond to a message event but a state event. Is there a better way to implement it?

it looks like we use this pattern for other timeline types, eg stickers, polls, verification. Would lean towards this being ok, although will delegate to @ganfra

mnaturel · 2022-04-08T15:54:46Z

...a/im/vector/app/features/home/room/detail/timeline/factory/LiveLocationMessageItemFactory.kt

+                    val startOfLive = DateProvider.toLocalDateTime(startTimestamp)
+                    val timeout = liveLocationContent.getBestBeaconInfo()?.timeout ?: 0L
+                    val endOfLive = startOfLive.plus(timeout, ChronoUnit.MILLIS)
+                    now.isAfter(endOfLive)


The comparison seems to not work correctly when phone timezone is changed. Maybe need to express all dates in UTC to ensure correctness.

Fixed, we are using the timestamp of last received and attached location event to check the validity of the beacon.

gaelledel

Yep all good only thing is the band at the bottom of the thumbnail should be set to Background colour at 85% opacity. Unsure if this is what you have there

mnaturel · 2022-04-12T07:25:07Z

Yep all good only thing is the band at the bottom of the thumbnail should be set to Background colour at 85% opacity. Unsure if this is what you have there

Thanks for the review. I checked and the band at the bottom has indeed an alpha value to 0.85. I guess we don't see it clearly due to the light color of the map.

ouchadam · 2022-04-12T12:08:35Z

changelog.d/5710.feature

@@ -0,0 +1 @@
+Live Location Sharing - Show message on start of a live


should this be of a live location?

I supposed the "Live Location Sharing" prefix was enough to infer the type of live but I can add it if you really think it is not clear enough.

ouchadam · 2022-04-12T12:10:23Z

...dk/internal/session/room/aggregation/livelocation/DefaultLiveLocationAggregationProcessor.kt

-            return
+            beaconInfoContent.hasTimedOut = true
+        } else {
+            // Update last location info of the beacon state event


this comment doesn't add any extra information, is it needed?

You are right, we can remove it since what we do here is clear.

ouchadam · 2022-04-12T12:28:28Z

...java/org/matrix/android/sdk/api/session/room/model/livelocation/LiveLocationBeaconContent.kt

+        /**
+         * Client side tracking of whether the beacon has timed out.
+         */
+        var hasTimedOut: Boolean = false


I'm a little weary of polluting the JSON model with mutable client state, is it possible to have this somewhere else , or does this model change mean the least amount of changes?

Yes I don't really like it either. I have seen we use Entity classes for that case in the project. I will try to update it with a corresponding Entity class.

I will try to refact this in another following PR when handling the location message update.

ouchadam · 2022-04-12T12:39:41Z

...ndroid/src/main/java/org/matrix/android/sdk/api/session/room/summary/RoomSummaryConstants.kt

@@ -33,5 +33,5 @@ object RoomSummaryConstants {
            EventType.ENCRYPTED,
            EventType.STICKER,
            EventType.REACTION
-    ) + EventType.POLL_START
+    ) + EventType.POLL_START + EventType.STATE_ROOM_BEACON_INFO


looks like these event types could be inside the listOf()

In fact these event types are not of type String values but of type List<String> in order to support stable/unstable fields. That's why they are added to the `listOf().

ahhhhhhh thanks for explaining!

ouchadam · 2022-04-12T12:40:20Z

...src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt

@@ -121,30 +122,31 @@ import org.matrix.android.sdk.internal.database.lightweight.LightweightSettingsS
 import javax.inject.Inject

 class MessageItemFactory @Inject constructor(


it's very tricky to know what changed in this file due to the formatting 😢

Sorry, I will try to revert to only the changes I have done. I guess this class will be reformatted in the future with new coding styles.

if you're able to point out the changes that would be enough for me (rather than having to revert)

Too late 😄 Now you can see clearly the changes.

ouchadam · 2022-04-12T13:57:34Z

...a/im/vector/app/features/home/room/detail/timeline/factory/LiveLocationMessageItemFactory.kt

+            attributes: AbsMessageItem.Attributes,
+    ): MessageLiveLocationStartItem {
+        val width = timelineMediaSizeProvider.getMaxSize().first
+        val height = dimensionConverter.dpToPx(200)


I've seen this 200 in a few places, wondering if it could be a constant somewhere~

ouchadam

no blockers from my side, LGTM! 💯 might be worth checking in with @ganfra to confirm about the MessageContent usage

…check

mnaturel changed the title ~~[Location sharing] - Show message on start of the live~~ [Location sharing] - Show message on start of a live Apr 8, 2022

mnaturel commented Apr 8, 2022

View reviewed changes

mnaturel requested a review from gaelledel April 8, 2022 15:04

mnaturel commented Apr 8, 2022

View reviewed changes

mnaturel force-pushed the feature/mna/PSF-883-start-live-message branch from 9e9ce13 to e995c20 Compare April 11, 2022 14:38

gaelledel approved these changes Apr 12, 2022

View reviewed changes

mnaturel requested review from a team and ouchadam and removed request for a team April 12, 2022 07:25

mnaturel marked this pull request as ready for review April 12, 2022 07:25

ouchadam reviewed Apr 12, 2022

View reviewed changes

Maxime NATUREL added 13 commits April 12, 2022 15:51

Adding changelog entry

2a201b3

Creating layout for the new message item

a1fb85d

Adding new message item

e93e51d

Show start event of live without conditions

aabfc81

Creating factory for live location items

3da3589

Handling bubble mode view

26f89fa

Align corner radius of banner on map view

e2d56e2

Removing TODO

6e61085

Making state beacon events displayable in timeline

78a9bde

Adding support for notifications and message preview

d78ed32

Keep track of time out using last location event

e501929

Fixing overlay in bubble mode

c3fe782

Remove the noisy reformatting of MessageItemFactory

4670072

mnaturel force-pushed the feature/mna/PSF-883-start-live-message branch from 1673f51 to 4670072 Compare April 12, 2022 13:55

ouchadam reviewed Apr 12, 2022

View reviewed changes

ouchadam approved these changes Apr 12, 2022

View reviewed changes

Maxime NATUREL added 2 commits April 12, 2022 16:10

Create a const value for the height of message location item

4c07405

Remove non necessary comment + adding a TODO for missing aggregation …

071340c

…check

ouchadam approved these changes Apr 13, 2022

View reviewed changes

mnaturel merged commit c832c1b into develop Apr 13, 2022

mnaturel deleted the feature/mna/PSF-883-start-live-message branch April 13, 2022 09:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Location sharing] - Show message on start of a live #5722

[Location sharing] - Show message on start of a live #5722

mnaturel commented Apr 8, 2022 •

edited

Loading

github-actions bot commented Apr 8, 2022 •

edited

Loading

mnaturel Apr 8, 2022 •

edited

Loading

ouchadam Apr 12, 2022

mnaturel Apr 8, 2022 •

edited

Loading

mnaturel Apr 11, 2022

gaelledel left a comment

mnaturel commented Apr 12, 2022

ouchadam Apr 12, 2022

mnaturel Apr 12, 2022

ouchadam Apr 12, 2022

mnaturel Apr 12, 2022

ouchadam Apr 12, 2022 •

edited

Loading

mnaturel Apr 12, 2022 •

edited

Loading

ouchadam Apr 12, 2022

mnaturel Apr 13, 2022

ouchadam Apr 12, 2022

mnaturel Apr 12, 2022

ouchadam Apr 12, 2022

ouchadam Apr 12, 2022

mnaturel Apr 12, 2022

ouchadam Apr 12, 2022

mnaturel Apr 12, 2022

ouchadam Apr 12, 2022

ouchadam left a comment

		@@ -0,0 +1 @@
		Live Location Sharing - Show message on start of a live

		@@ -121,30 +122,31 @@ import org.matrix.android.sdk.internal.database.lightweight.LightweightSettingsS
		import javax.inject.Inject

		class MessageItemFactory @Inject constructor(

[Location sharing] - Show message on start of a live #5722

[Location sharing] - Show message on start of a live #5722

Conversation

mnaturel commented Apr 8, 2022 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Default layout mode

Bubble layout mode

UX

Tests

Tested devices

Checklist

github-actions bot commented Apr 8, 2022 • edited Loading

Unit Test Results

mnaturel Apr 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnaturel Apr 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaelledel left a comment

Choose a reason for hiding this comment

mnaturel commented Apr 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam Apr 12, 2022 • edited Loading

Choose a reason for hiding this comment

mnaturel Apr 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam left a comment

Choose a reason for hiding this comment

mnaturel commented Apr 8, 2022 •

edited

Loading

github-actions bot commented Apr 8, 2022 •

edited

Loading

mnaturel Apr 8, 2022 •

edited

Loading

mnaturel Apr 8, 2022 •

edited

Loading

ouchadam Apr 12, 2022 •

edited

Loading

mnaturel Apr 12, 2022 •

edited

Loading