Live Location Sharing - User List Bottom Sheet [PSF-890] #6170

onurays · 2022-05-27T14:53:48Z

Type of change

Feature
Bugfix
Technical
Other :

Content

This PR implements a bottom sheet on maximized live location sharing screen. The bottom sheet contains the user list who currently share their live location. If the current user is sharing a live location the list item also has a button to allow stopping location sharing. Clicking a user should zoom to user's location.

Motivation and context

Closes #6012

Screenshots / GIFs

Screenrecorder-2022-05-27-17-22-57-182.mp4

Screenrecorder-2022-05-27-17-30-05-425.mp4

Tests

Select location attachment icon from timeline composer
Select "Share live location" in location screen
Ideally share live location from another user in the same room too
Live location item should be visible in timeline, tap on the item, should navigate to maximized map screen
You should see all users' avatars on map and the bottom sheet with the user list
You should see a stop sharing button next to your user's item in bottom sheet
Selecting stop sharing button should remove your user on map and in bottom sheet list
Selecting a user in bottom sheet should zoom to the selected user's location on map
Bottom sheet should be extendable and scrollable

Tested devices

Physical
Emulator
OS version(s):

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-05-27T15:15:55Z

Unit Test Results

142 files ±0 142 suites ±0 2m 14s ⏱️ +5s
229 tests ±0 229 ✔️ ±0 0 💤 ±0 0 ❌ ±0
760 runs ±0 760 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 353f290. ± Comparison against base commit 828e13f.

♻️ This comment has been updated with latest results.

ouchadam · 2022-05-30T09:15:34Z

to double check, should we be showing the seconds in the updated X ago?

ouchadam · 2022-05-30T09:22:03Z

.../src/main/java/im/vector/app/features/location/live/map/LiveLocationBottomSheetController.kt

+        private val avatarRenderer: AvatarRenderer,
+        private val vectorDateFormatter: VectorDateFormatter,
+        private val stringProvider: StringProvider,
+        private val clock: Clock,


💯 for injecting the clock!

ariskotsomitopoulos

PR seems fine, just a minor comment

ariskotsomitopoulos · 2022-05-30T09:16:54Z

vector/src/main/java/im/vector/app/core/utils/TextUtils.kt

+        return formatDurationWithUnits(duration, stringProvider::getString)
+    }
+
+    private fun formatDurationWithUnits(duration: Duration, getString: ((Int) -> String)): String {


I would add a doc/comment for this function to be clear what it does

Nice idea, done.

mnaturel · 2022-05-30T09:41:44Z

vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewFragment.kt

@@ -50,7 +53,8 @@ import javax.inject.Inject
 */
 class LocationLiveMapViewFragment @Inject constructor(
        private var urlMapProvider: UrlMapProvider,
-) : VectorBaseFragment<FragmentSimpleContainerBinding>() {
+        private var bottomSheetController: LiveLocationBottomSheetController,
+) : VectorBaseFragment<FragmentLocationLiveMapViewBinding>() {


I think you can remove the fragment_simple_container.xml.

Right, removed.

onurays · 2022-05-30T10:22:53Z

to double check, should we be showing the seconds in the updated X ago?

We shouldn't. I will hide seconds if it has minutes or hours part. I am not sure if we prefer something like "Updated recently".

mnaturel · 2022-05-30T11:06:19Z

vector/src/main/res/layout/fragment_location_live_map_view.xml

+    android:background="@drawable/bg_live_location_users_bottom_sheet">
+
+    <FrameLayout
+        android:id="@+id/fragmentContainer"


I don't know if we should prefix the view ids by for example liveLocationMap to avoid having duplicated ids inside the app?

It is better with prefix. Done.

mnaturel · 2022-05-30T11:08:13Z

vector/src/main/res/values/strings.xml

@@ -3037,6 +3037,8 @@
    <string name="live_location_sharing_notification_description">Location sharing is in progress</string>
    <string name="labs_enable_live_location">Enable Live Location Sharing</string>
    <string name="labs_enable_live_location_summary">Temporary implementation: locations persist in room history</string>
+    <string name="live_location_bottom_sheet_stop_sharing">Stop sharing</string>


Should we have a dedicated string for the bottom sheet or could we reuse the "Stop" string location_share_live_stop used in other views to stop a live location share?

According to design we need a new string.

mnaturel · 2022-05-30T11:11:49Z

vector/sampledata/live_location_users.json

+      "lastUpdatedAt": "Updated 12min ago"
+    },
+    {
+      "displayName": "You",


Is the second item used? I only see the first one used in text for preview of the item layout.

Yes, it is seen in RecyclerView's preview.

mnaturel · 2022-05-30T11:23:53Z

vector/src/main/res/layout/item_live_location_users_bottom_sheet.xml

+<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:app="http://schemas.android.com/apk/res-auto"
+    xmlns:tools="http://schemas.android.com/tools"
+    android:layout_width="match_parent"


Maybe we could add android:background="?selectableItemBackground" to have ripple effect when clicking on a user item?

Nice catch, done.

mnaturel · 2022-05-30T11:28:46Z

vector/src/main/res/layout/fragment_location_live_map_view.xml

+        android:layout_height="match_parent"
+        android:background="@drawable/bg_live_location_users_bottom_sheet"
+        app:behavior_peekHeight="200dp"
+        app:behavior_hideable="false"


Do we want the bottom sheet always visible? I expected to be able to have the map view in full screen (i.e. just seeing the top of the bottom sheet), I don't know what is expected here.

According to design we need to. But I also want to discuss this with the team.

mnaturel · 2022-05-30T11:36:16Z

vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewFragment.kt

@@ -50,7 +53,8 @@ import javax.inject.Inject
 */
 class LocationLiveMapViewFragment @Inject constructor(


When creating the SymbolManager for the map in setupMap() method, we can add option to allow override of symbols in order to see every users even if they are very near from each other:

symbolManager = SymbolManager(mapFragment.view as MapView, mapboxMap, style).also { it.iconAllowOverlap = true }

I didn't know this option, cool! Done.

* develop: (114 commits) Docs: Fix various formatting and spelling issues in notifications.md Fixing non necessary breaking line continuing to the originally supplied url when a rtl override character is detected splitting url detection condition into separate branches Cleaner code Create extension `String?.toActiveSpaceOrOrphanRooms()` to reduce noise. Add changelog Fix test compilation Add some Kdoc Add some Kdoc Create SpaceFilter.OrphanRooms to improve the API. Not 100% of the side effect. There is probably some (fixed?) bugs here. Rename ActiveSpaceFilter to SpaceFilter Remove `ActiveSpaceFilter.None` Prefer nullability for API coherency of `RoomSummaryQueryParams` Add some Kdoc Remove duplicated lines of code (the same code is done a few lines later) Remove `RoomCategoryFilter.ALL` Prefer nullability for API coherency of `RoomSummaryQueryParams` `displayName` default value is now `QueryStringValue.NoCondition`. It was working fine since in the DB we always have a name using `RoomDisplayNameFallbackProvider`, which in our current implementation always return a non empty String. Small rework for nicer code Remove duplicated code lines Remove `roomId` from `RoomSummaryQueryParams.Builder()`. Create a new API in RoomService to observe a room summary from a roomId. ... # Conflicts: # vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewFragment.kt

mnaturel · 2022-05-30T15:09:35Z

...rc/test/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapperTest.kt

                pinDrawable = pinDrawable,
                locationData = LocationData(
                        latitude = A_LATITUDE,
                        longitude = A_LONGITUDE,
                        uncertainty = A_UNCERTAINTY
                ),
-                endOfLiveTimestampMillis = A_END_OF_LIVE_TIMESTAMP
+                endOfLiveTimestampMillis = A_END_OF_LIVE_TIMESTAMP,
+                locationTimestampMillis = A_END_OF_LIVE_TIMESTAMP,


I think here we should declare a dedicated location timestamp that should be set in the given locationDataContent variable. So that the mapping is checked.

mnaturel · 2022-05-30T15:19:23Z

...rc/test/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapperTest.kt

@@ -79,14 +84,16 @@ class UserLiveLocationViewStateMapperTest {
        val viewState = userLiveLocationViewStateMapper.map(summary)

        val expectedViewState = UserLiveLocationViewState(
-                userId = A_USER_ID,
+                matrixItem = MatrixItem.UserItem(id = A_USER_ID, displayName = "User 2", avatarUrl = ""),


I think we will also need to mock the return of getUser() method for the fakeSession

Absolutely, done. Thanks for the help.

github-actions · 2022-05-30T18:17:59Z

Matrix SDK

Integration Tests Results:

[org.matrix.android.sdk.session]
[org.matrix.android.sdk.account]
[org.matrix.android.sdk.internal]
[org.matrix.android.sdk.ordering]
[org.matrix.android.sdk.PermalinkParserTest]

onurays added 10 commits May 27, 2022 15:25

Add required resources.

453aa28

Add bottom sheet layout.

88de113

Refactor duration formatter to be able to user StringProvider.

44b2a7f

Implement bottom sheet controller.

924d7e1

Add location update timestamp live location view state mapper.

8247b1d

Implement user list bottom sheet.

daa0734

Implement stop sharing button visibility.

83e7920

Implement stop sharing function from bottom sheet.

2eaf843

Zoom to selected user from bottom sheet.

8018ba3

Changelog added.

8bc7d3e

onurays requested review from a team, yostyle and ariskotsomitopoulos and removed request for a team May 27, 2022 14:56

ouchadam self-requested a review May 30, 2022 09:08

ouchadam reviewed May 30, 2022

View reviewed changes

ariskotsomitopoulos approved these changes May 30, 2022

View reviewed changes

mnaturel reviewed May 30, 2022

View reviewed changes

Remove unused layout.

b262aad

Base automatically changed from feature/mna/PSF-1019-user-pins to develop May 30, 2022 10:32

Code documentation.

aa344b1

mnaturel reviewed May 30, 2022

View reviewed changes

onurays added 2 commits May 30, 2022 15:40

Code review fixes.

21abc3f

mnaturel approved these changes May 30, 2022

View reviewed changes

Fix unit tests.

84b3d55

mnaturel reviewed May 30, 2022

View reviewed changes

yostyle approved these changes May 30, 2022

View reviewed changes

Fix unit tests.

353f290

onurays merged commit 4ccd242 into develop May 30, 2022

onurays deleted the feature/ons/live_location_bottom_sheet branch May 30, 2022 18:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Live Location Sharing - User List Bottom Sheet [PSF-890] #6170

Live Location Sharing - User List Bottom Sheet [PSF-890] #6170

onurays commented May 27, 2022 •

edited by mnaturel

Loading

github-actions bot commented May 27, 2022 •

edited

Loading

ouchadam commented May 30, 2022

ouchadam May 30, 2022

ariskotsomitopoulos left a comment

ariskotsomitopoulos May 30, 2022

onurays May 30, 2022

mnaturel May 30, 2022

onurays May 30, 2022

onurays commented May 30, 2022

mnaturel May 30, 2022

onurays May 30, 2022

mnaturel May 30, 2022 •

edited

Loading

onurays May 30, 2022

mnaturel May 30, 2022

onurays May 30, 2022

mnaturel May 30, 2022

onurays May 30, 2022

mnaturel May 30, 2022

onurays May 30, 2022

mnaturel May 30, 2022

onurays May 30, 2022

mnaturel May 30, 2022

onurays May 30, 2022

mnaturel May 30, 2022

onurays May 30, 2022

onurays May 30, 2022

github-actions bot commented May 30, 2022

		@@ -50,7 +53,8 @@ import javax.inject.Inject
		*/
		class LocationLiveMapViewFragment @Inject constructor(

Live Location Sharing - User List Bottom Sheet [PSF-890] #6170

Live Location Sharing - User List Bottom Sheet [PSF-890] #6170

Conversation

onurays commented May 27, 2022 • edited by mnaturel Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented May 27, 2022 • edited Loading

Unit Test Results

ouchadam commented May 30, 2022

Choose a reason for hiding this comment

ariskotsomitopoulos left a 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

onurays commented May 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnaturel May 30, 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

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

github-actions bot commented May 30, 2022

Matrix SDK

Integration Tests Results:

onurays commented May 27, 2022 •

edited by mnaturel

Loading

github-actions bot commented May 27, 2022 •

edited

Loading

mnaturel May 30, 2022 •

edited

Loading