From d49f6bfbb6665c0d96afaba82af2ee239efc16d9 Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoit@matrix.org>
Date: Thu, 1 Feb 2024 16:32:36 +0100
Subject: [PATCH 1/7] Equivalent code.
---
.../detail/timeline/action/MessageActionsEpoxyController.kt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt
index 307b7f4b99..7ab8862886 100644
--- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt
+++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsEpoxyController.kt
@@ -238,7 +238,7 @@ class MessageActionsEpoxyController @Inject constructor(
val locationUrl = locationContent.toLocationData()
?.let { urlMapProvider.buildStaticMapUrl(it, INITIAL_MAP_ZOOM_IN_TIMELINE, 1200, 800) }
?: return null
- val locationOwnerId = if (locationContent.isSelfLocation()) state.informationData.matrixItem.id else null
+ val locationOwnerId = if (locationContent.isSelfLocation()) state.informationData.senderId else null
return LocationUiData(
locationUrl = locationUrl,
From 343468717ce5da7a198a48448a28e0da24c6dfa4 Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoit@matrix.org>
Date: Thu, 1 Feb 2024 16:33:59 +0100
Subject: [PATCH 2/7] Add a test
---
.../java/im/vector/app/features/location/LocationDataTest.kt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/vector/src/test/java/im/vector/app/features/location/LocationDataTest.kt b/vector/src/test/java/im/vector/app/features/location/LocationDataTest.kt
index a0d5b0c3f3..04fe00a94d 100644
--- a/vector/src/test/java/im/vector/app/features/location/LocationDataTest.kt
+++ b/vector/src/test/java/im/vector/app/features/location/LocationDataTest.kt
@@ -17,6 +17,7 @@
package im.vector.app.features.location
import org.amshove.kluent.shouldBeEqualTo
+import org.amshove.kluent.shouldBeFalse
import org.amshove.kluent.shouldBeNull
import org.amshove.kluent.shouldBeTrue
import org.junit.Test
@@ -80,6 +81,9 @@ class LocationDataTest {
val contentWithSelfAssetType = MessageLocationContent(body = "", geoUri = "", unstableLocationAsset = LocationAsset(type = LocationAssetType.SELF))
contentWithSelfAssetType.isSelfLocation().shouldBeTrue()
+
+ val contentWithPinAssetType = MessageLocationContent(body = "", geoUri = "", unstableLocationAsset = LocationAsset(type = LocationAssetType.PIN))
+ contentWithPinAssetType.isSelfLocation().shouldBeFalse()
}
@Test
From ff439546c51a85ab0ba02cd265b667a15801f573 Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoit@matrix.org>
Date: Fri, 2 Feb 2024 15:34:51 +0100
Subject: [PATCH 3/7] Improve cache of drawables used for rendering location
pin. In particular, use the Glide cache, and ensure that if an error occurs
and later the avatar can be retrieved, the cache will be replaced. Also limit
cache size to 32. Also use UserItem as a key, instead of just the userId, so
that if displayName or avatarUrl change, there will be not cache hit.
---
.../timeline/helper/LocationPinProvider.kt | 38 ++++++++++++-------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt
index 7f276f2f73..fb4169568c 100644
--- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt
+++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt
@@ -19,7 +19,7 @@ package im.vector.app.features.home.room.detail.timeline.helper
import android.content.Context
import android.graphics.drawable.Drawable
import android.graphics.drawable.LayerDrawable
-import androidx.annotation.ColorInt
+import android.util.LruCache
import androidx.core.content.ContextCompat
import androidx.core.graphics.drawable.DrawableCompat
import com.bumptech.glide.request.target.CustomTarget
@@ -30,11 +30,17 @@ import im.vector.app.core.glide.GlideApp
import im.vector.app.core.utils.DimensionConverter
import im.vector.app.features.home.AvatarRenderer
import org.matrix.android.sdk.api.session.getUserOrDefault
+import org.matrix.android.sdk.api.util.MatrixItem
import org.matrix.android.sdk.api.util.toMatrixItem
import timber.log.Timber
import javax.inject.Inject
import javax.inject.Singleton
+private data class CachedDrawable(
+ val drawable: Drawable,
+ val isError: Boolean,
+)
+
@Singleton
class LocationPinProvider @Inject constructor(
private val context: Context,
@@ -43,7 +49,7 @@ class LocationPinProvider @Inject constructor(
private val avatarRenderer: AvatarRenderer,
private val matrixItemColorProvider: MatrixItemColorProvider
) {
- private val cache = mutableMapOf<String, Drawable>()
+ private val cache = LruCache<MatrixItem.UserItem, CachedDrawable>(32)
private val glideRequests by lazy {
GlideApp.with(context)
@@ -60,23 +66,16 @@ class LocationPinProvider @Inject constructor(
return
}
- if (cache.contains(userId)) {
- callback(cache[userId]!!)
- return
- }
-
activeSessionHolder
.getActiveSession()
.getUserOrDefault(userId)
.toMatrixItem()
.let { userItem ->
val size = dimensionConverter.dpToPx(44)
- val bgTintColor = matrixItemColorProvider.getColor(userItem)
avatarRenderer.render(glideRequests, userItem, object : CustomTarget<Drawable>(size, size) {
override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
Timber.d("## Location: onResourceReady")
- val pinDrawable = createPinDrawable(resource, bgTintColor)
- cache[userId] = pinDrawable
+ val pinDrawable = createPinDrawable(userItem, resource, isError = false)
callback(pinDrawable)
}
@@ -87,17 +86,29 @@ class LocationPinProvider @Inject constructor(
}
override fun onLoadFailed(errorDrawable: Drawable?) {
+ // Note: `onLoadFailed` is also called when the user has no avatarUrl
+ // and the errorDrawable is actually the placeholder.
Timber.w("## Location: onLoadFailed")
errorDrawable ?: return
- val pinDrawable = createPinDrawable(errorDrawable, bgTintColor)
- cache[userId] = pinDrawable
+ val pinDrawable = createPinDrawable(userItem, errorDrawable, isError = true)
callback(pinDrawable)
}
})
}
}
- private fun createPinDrawable(drawable: Drawable, @ColorInt bgTintColor: Int): Drawable {
+ private fun createPinDrawable(
+ userItem: MatrixItem.UserItem,
+ drawable: Drawable,
+ isError: Boolean,
+ ): Drawable {
+ val fromCache = cache.get(userItem)
+ // Return the cached drawable only if it is valid, or the new drawable is again an error
+ if (fromCache != null && (!fromCache.isError || isError)) {
+ return fromCache.drawable
+ }
+
+ val bgTintColor = matrixItemColorProvider.getColor(userItem)
val bgUserPin = ContextCompat.getDrawable(context, R.drawable.bg_map_user_pin)!!
// use mutate on drawable to avoid sharing the color when we have multiple different user pins
DrawableCompat.setTint(bgUserPin.mutate(), bgTintColor)
@@ -106,6 +117,7 @@ class LocationPinProvider @Inject constructor(
val topInset = dimensionConverter.dpToPx(4)
val bottomInset = dimensionConverter.dpToPx(8)
layerDrawable.setLayerInset(1, horizontalInset, topInset, horizontalInset, bottomInset)
+ cache.put(userItem, CachedDrawable(layerDrawable, isError))
return layerDrawable
}
}
From 8b1bd7940dcf1b464960a6e58e2875a48d686f73 Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoit@matrix.org>
Date: Fri, 2 Feb 2024 16:01:59 +0100
Subject: [PATCH 4/7] Fix avatar with initial not displayed on message preview
bottom sheet
---
.../core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt b/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt
index bb1b0fbd7b..1e4c3dbbc5 100644
--- a/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt
+++ b/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt
@@ -104,9 +104,8 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
.into(holder.staticMapImageView)
safeLocationUiData.locationPinProvider.create(safeLocationUiData.locationOwnerId) { pinDrawable ->
- GlideApp.with(holder.staticMapPinImageView)
- .load(pinDrawable)
- .into(holder.staticMapPinImageView)
+ // we are not using Glide since it does not display it correctly when there is no user photo
+ holder.staticMapPinImageView.setImageDrawable(pinDrawable)
}
}
}
From c6bb054fd7177cbbc46b8cdfa74c3eee8df27efd Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoit@matrix.org>
Date: Fri, 2 Feb 2024 17:25:00 +0100
Subject: [PATCH 5/7] Add a log when User is not kwown.
---
.../org/matrix/android/sdk/api/session/SessionExtensions.kt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/SessionExtensions.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/SessionExtensions.kt
index 96dac27618..267e832d1e 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/SessionExtensions.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/SessionExtensions.kt
@@ -19,6 +19,7 @@ package org.matrix.android.sdk.api.session
import org.matrix.android.sdk.api.session.room.Room
import org.matrix.android.sdk.api.session.room.model.RoomSummary
import org.matrix.android.sdk.api.session.user.model.User
+import timber.log.Timber
/**
* Get a room using the RoomService of a Session.
@@ -41,4 +42,5 @@ fun Session.getUser(userId: String): User? = userService().getUser(userId)
/**
* Similar to [getUser], but fallback to a User without details if the User is not known by the SDK, or if Session is null.
*/
-fun Session?.getUserOrDefault(userId: String): User = this?.userService()?.getUser(userId) ?: User(userId)
+fun Session?.getUserOrDefault(userId: String): User = this?.userService()?.getUser(userId)
+ ?: User(userId).also { Timber.w("User $userId not found in local cache, fallback to default") }
From 6ea0129beeb6e65564ab437c0572a3ea24f7ec12 Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoit@matrix.org>
Date: Fri, 2 Feb 2024 18:23:09 +0100
Subject: [PATCH 6/7] Location sharing: use Room member avatar instead of
profile avatar.
---
.../LiveLocationShareAggregatedSummary.kt | 1 +
...iveLocationShareAggregatedSummaryMapper.kt | 1 +
...ocationShareAggregatedSummaryMapperTest.kt | 3 +
.../DefaultLocationSharingServiceTest.kt | 2 +
.../BottomSheetMessagePreviewItem.kt | 3 +-
.../LiveLocationShareMessageItemFactory.kt | 2 +-
.../timeline/factory/MessageItemFactory.kt | 4 +-
.../timeline/helper/LocationPinProvider.kt | 63 ++++++++-----------
.../timeline/item/AbsMessageLocationItem.kt | 5 +-
.../timeline/item/MessageLiveLocationItem.kt | 2 +-
.../location/LocationSharingViewModel.kt | 6 +-
.../map/UserLiveLocationViewStateMapper.kt | 18 +++++-
.../preview/LocationPreviewViewModel.kt | 19 +++++-
.../preview/LocationPreviewViewState.kt | 2 +
.../GetLiveLocationShareSummaryUseCaseTest.kt | 1 +
.../GetListOfUserLiveLocationUseCaseTest.kt | 3 +
.../UserLiveLocationViewStateMapperTest.kt | 7 ++-
.../app/test/fakes/FakeLocationPinProvider.kt | 5 +-
18 files changed, 91 insertions(+), 56 deletions(-)
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/livelocation/LiveLocationShareAggregatedSummary.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/livelocation/LiveLocationShareAggregatedSummary.kt
index 5ad1a48217..8d0a95ad13 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/livelocation/LiveLocationShareAggregatedSummary.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/livelocation/LiveLocationShareAggregatedSummary.kt
@@ -22,6 +22,7 @@ import org.matrix.android.sdk.api.session.room.model.message.MessageBeaconLocati
* Aggregation info concerning a live location share.
*/
data class LiveLocationShareAggregatedSummary(
+ val roomId: String?,
val userId: String?,
/**
* Indicate whether the live is currently running.
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapper.kt
index 4a4c730a0b..61f4450e8f 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapper.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapper.kt
@@ -28,6 +28,7 @@ internal class LiveLocationShareAggregatedSummaryMapper @Inject constructor() :
override fun map(entity: LiveLocationShareAggregatedSummaryEntity): LiveLocationShareAggregatedSummary {
return LiveLocationShareAggregatedSummary(
+ roomId = entity.roomId,
userId = entity.userId,
isActive = entity.isActive,
endOfLiveTimestampMillis = entity.endOfLiveTimestampMillis,
diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapperTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapperTest.kt
index 47d5f46525..94264ffe43 100644
--- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapperTest.kt
+++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapperTest.kt
@@ -24,6 +24,7 @@ import org.matrix.android.sdk.api.session.room.model.message.LocationInfo
import org.matrix.android.sdk.api.session.room.model.message.MessageBeaconLocationDataContent
import org.matrix.android.sdk.internal.database.model.livelocation.LiveLocationShareAggregatedSummaryEntity
+private const val ANY_ROOM_ID = "a-room-id"
private const val ANY_USER_ID = "a-user-id"
private const val ANY_ACTIVE_STATE = true
private const val ANY_TIMEOUT = 123L
@@ -40,6 +41,7 @@ class LiveLocationShareAggregatedSummaryMapperTest {
val summary = mapper.map(entity)
summary shouldBeEqualTo LiveLocationShareAggregatedSummary(
+ roomId = ANY_ROOM_ID,
userId = ANY_USER_ID,
isActive = ANY_ACTIVE_STATE,
endOfLiveTimestampMillis = ANY_TIMEOUT,
@@ -48,6 +50,7 @@ class LiveLocationShareAggregatedSummaryMapperTest {
}
private fun anEntity(content: MessageBeaconLocationDataContent) = LiveLocationShareAggregatedSummaryEntity(
+ roomId = ANY_ROOM_ID,
userId = ANY_USER_ID,
isActive = ANY_ACTIVE_STATE,
endOfLiveTimestampMillis = ANY_TIMEOUT,
diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/location/DefaultLocationSharingServiceTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/location/DefaultLocationSharingServiceTest.kt
index 1f15a9bee8..5c27562c9c 100644
--- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/location/DefaultLocationSharingServiceTest.kt
+++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/location/DefaultLocationSharingServiceTest.kt
@@ -229,6 +229,7 @@ internal class DefaultLocationSharingServiceTest {
fun `livedata of live summaries is correctly computed`() {
val entity = LiveLocationShareAggregatedSummaryEntity()
val summary = LiveLocationShareAggregatedSummary(
+ roomId = A_ROOM_ID,
userId = "",
isActive = true,
endOfLiveTimestampMillis = 123,
@@ -255,6 +256,7 @@ internal class DefaultLocationSharingServiceTest {
fun `given an event id when getting livedata on corresponding live summary then it is correctly computed`() {
val entity = LiveLocationShareAggregatedSummaryEntity()
val summary = LiveLocationShareAggregatedSummary(
+ roomId = A_ROOM_ID,
userId = "",
isActive = true,
endOfLiveTimestampMillis = 123,
diff --git a/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt b/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt
index 1e4c3dbbc5..cf5e590081 100644
--- a/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt
+++ b/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt
@@ -103,7 +103,8 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
.apply(RequestOptions.centerCropTransform())
.into(holder.staticMapImageView)
- safeLocationUiData.locationPinProvider.create(safeLocationUiData.locationOwnerId) { pinDrawable ->
+ val pinMatrixItem = matrixItem.takeIf { safeLocationUiData.locationOwnerId != null }
+ safeLocationUiData.locationPinProvider.create(pinMatrixItem) { pinDrawable ->
// we are not using Glide since it does not display it correctly when there is no user photo
holder.staticMapPinImageView.setImageDrawable(pinDrawable)
}
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/LiveLocationShareMessageItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/LiveLocationShareMessageItemFactory.kt
index 493602a291..8482f0e2b6 100644
--- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/LiveLocationShareMessageItemFactory.kt
+++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/LiveLocationShareMessageItemFactory.kt
@@ -114,7 +114,7 @@ class LiveLocationShareMessageItemFactory @Inject constructor(
.locationUrl(locationUrl)
.mapWidth(width)
.mapHeight(height)
- .locationUserId(attributes.informationData.senderId)
+ .pinMatrixItem(attributes.informationData.matrixItem)
.locationPinProvider(locationPinProvider)
.highlighted(highlight)
.leftGuideline(avatarSizeProvider.leftGuideline)
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt
index dd52c05265..94189a7a53 100644
--- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt
+++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt
@@ -233,14 +233,14 @@ class MessageItemFactory @Inject constructor(
urlMapProvider.buildStaticMapUrl(it, INITIAL_MAP_ZOOM_IN_TIMELINE, width, height)
}
- val locationUserId = if (locationContent.isSelfLocation()) informationData.senderId else null
+ val pinMatrixItem = if (locationContent.isSelfLocation()) informationData.matrixItem else null
return MessageLocationItem_()
.attributes(attributes)
.locationUrl(locationUrl)
.mapWidth(width)
.mapHeight(height)
- .locationUserId(locationUserId)
+ .pinMatrixItem(pinMatrixItem)
.locationPinProvider(locationPinProvider)
.highlighted(highlight)
.leftGuideline(avatarSizeProvider.leftGuideline)
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt
index fb4169568c..2fdd7198a9 100644
--- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt
+++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt
@@ -25,13 +25,10 @@ import androidx.core.graphics.drawable.DrawableCompat
import com.bumptech.glide.request.target.CustomTarget
import com.bumptech.glide.request.transition.Transition
import im.vector.app.R
-import im.vector.app.core.di.ActiveSessionHolder
import im.vector.app.core.glide.GlideApp
import im.vector.app.core.utils.DimensionConverter
import im.vector.app.features.home.AvatarRenderer
-import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.util.MatrixItem
-import org.matrix.android.sdk.api.util.toMatrixItem
import timber.log.Timber
import javax.inject.Inject
import javax.inject.Singleton
@@ -44,12 +41,11 @@ private data class CachedDrawable(
@Singleton
class LocationPinProvider @Inject constructor(
private val context: Context,
- private val activeSessionHolder: ActiveSessionHolder,
private val dimensionConverter: DimensionConverter,
private val avatarRenderer: AvatarRenderer,
private val matrixItemColorProvider: MatrixItemColorProvider
) {
- private val cache = LruCache<MatrixItem.UserItem, CachedDrawable>(32)
+ private val cache = LruCache<MatrixItem, CachedDrawable>(32)
private val glideRequests by lazy {
GlideApp.with(context)
@@ -57,48 +53,41 @@ class LocationPinProvider @Inject constructor(
/**
* Creates a pin drawable. If userId is null then a generic pin drawable will be created.
- * @param userId userId that will be used to retrieve user avatar
+ * @param matrixUser user that will be used to retrieve user avatar
* @param callback Pin drawable will be sent through the callback
*/
- fun create(userId: String?, callback: (Drawable) -> Unit) {
- if (userId == null) {
+ fun create(matrixUser: MatrixItem?, callback: (Drawable) -> Unit) {
+ if (matrixUser == null) {
callback(ContextCompat.getDrawable(context, R.drawable.ic_location_pin)!!)
return
}
+ val size = dimensionConverter.dpToPx(44)
+ avatarRenderer.render(glideRequests, matrixUser, object : CustomTarget<Drawable>(size, size) {
+ override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
+ Timber.d("## Location: onResourceReady")
+ val pinDrawable = createPinDrawable(matrixUser, resource, isError = false)
+ callback(pinDrawable)
+ }
- activeSessionHolder
- .getActiveSession()
- .getUserOrDefault(userId)
- .toMatrixItem()
- .let { userItem ->
- val size = dimensionConverter.dpToPx(44)
- avatarRenderer.render(glideRequests, userItem, object : CustomTarget<Drawable>(size, size) {
- override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
- Timber.d("## Location: onResourceReady")
- val pinDrawable = createPinDrawable(userItem, resource, isError = false)
- callback(pinDrawable)
- }
+ override fun onLoadCleared(placeholder: Drawable?) {
+ // Is it possible? Put placeholder instead?
+ // FIXME The doc says it has to be implemented and should free resources
+ Timber.d("## Location: onLoadCleared")
+ }
- override fun onLoadCleared(placeholder: Drawable?) {
- // Is it possible? Put placeholder instead?
- // FIXME The doc says it has to be implemented and should free resources
- Timber.d("## Location: onLoadCleared")
- }
-
- override fun onLoadFailed(errorDrawable: Drawable?) {
- // Note: `onLoadFailed` is also called when the user has no avatarUrl
- // and the errorDrawable is actually the placeholder.
- Timber.w("## Location: onLoadFailed")
- errorDrawable ?: return
- val pinDrawable = createPinDrawable(userItem, errorDrawable, isError = true)
- callback(pinDrawable)
- }
- })
- }
+ override fun onLoadFailed(errorDrawable: Drawable?) {
+ // Note: `onLoadFailed` is also called when the user has no avatarUrl
+ // and the errorDrawable is actually the placeholder.
+ Timber.w("## Location: onLoadFailed")
+ errorDrawable ?: return
+ val pinDrawable = createPinDrawable(matrixUser, errorDrawable, isError = true)
+ callback(pinDrawable)
+ }
+ })
}
private fun createPinDrawable(
- userItem: MatrixItem.UserItem,
+ userItem: MatrixItem,
drawable: Drawable,
isError: Boolean,
): Drawable {
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageLocationItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageLocationItem.kt
index 4903b8c8cf..c3294d0301 100644
--- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageLocationItem.kt
+++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageLocationItem.kt
@@ -38,6 +38,7 @@ import im.vector.app.features.home.room.detail.timeline.style.TimelineMessageLay
import im.vector.app.features.home.room.detail.timeline.style.granularRoundedCorners
import im.vector.app.features.location.MapLoadingErrorView
import im.vector.app.features.location.MapLoadingErrorViewState
+import org.matrix.android.sdk.api.util.MatrixItem
abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
@LayoutRes layoutId: Int = R.layout.item_timeline_event_base
@@ -47,7 +48,7 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
var locationUrl: String? = null
@EpoxyAttribute
- var locationUserId: String? = null
+ var pinMatrixItem: MatrixItem? = null
@EpoxyAttribute
var mapWidth: Int = 0
@@ -103,7 +104,7 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
dataSource: DataSource?,
isFirstResource: Boolean
): Boolean {
- locationPinProvider?.create(locationUserId) { pinDrawable ->
+ locationPinProvider?.create(pinMatrixItem) { pinDrawable ->
// we are not using Glide since it does not display it correctly when there is no user photo
holder.staticMapPinImageView.setImageDrawable(pinDrawable)
}
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageLiveLocationItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageLiveLocationItem.kt
index 010abecc73..e1a081f450 100644
--- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageLiveLocationItem.kt
+++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageLiveLocationItem.kt
@@ -49,7 +49,7 @@ abstract class MessageLiveLocationItem : AbsMessageLocationItem<MessageLiveLocat
private fun bindLiveLocationBanner(holder: Holder) {
// TODO in a future PR add check on device id to confirm that is the one that sent the beacon
- val isEmitter = currentUserId != null && currentUserId == locationUserId
+ val isEmitter = currentUserId != null && currentUserId == pinMatrixItem?.id
val messageLayout = attributes.informationData.messageLayout
val viewState = buildViewState(holder, messageLayout, isEmitter)
holder.liveLocationRunningBanner.isVisible = true
diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt
index cdef7d3302..4841cffa1b 100644
--- a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt
@@ -106,11 +106,13 @@ class LocationSharingViewModel @AssistedInject constructor(
private fun updatePin(isUserPin: Boolean? = true) {
if (isUserPin.orFalse()) {
- locationPinProvider.create(userId = session.myUserId) {
+ val matrixItem = room.membershipService().getRoomMember(session.myUserId)?.toMatrixItem()
+ ?: session.getUserOrDefault(session.myUserId).toMatrixItem()
+ locationPinProvider.create(matrixItem) {
updatePinDrawableInState(it)
}
} else {
- locationPinProvider.create(userId = null) {
+ locationPinProvider.create(null) {
updatePinDrawableInState(it)
}
}
diff --git a/vector/src/main/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapper.kt b/vector/src/main/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapper.kt
index f4f4a462ce..7c35243ceb 100644
--- a/vector/src/main/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapper.kt
+++ b/vector/src/main/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapper.kt
@@ -20,8 +20,10 @@ import im.vector.app.core.di.ActiveSessionHolder
import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvider
import im.vector.app.features.location.toLocationData
import kotlinx.coroutines.suspendCancellableCoroutine
+import org.matrix.android.sdk.api.session.getRoom
import org.matrix.android.sdk.api.session.getUserOrDefault
import org.matrix.android.sdk.api.session.room.model.livelocation.LiveLocationShareAggregatedSummary
+import org.matrix.android.sdk.api.util.MatrixItem
import org.matrix.android.sdk.api.util.toMatrixItem
import javax.inject.Inject
@@ -43,11 +45,21 @@ class UserLiveLocationViewStateMapper @Inject constructor(
// do nothing on cancellation
}
else -> {
- locationPinProvider.create(userId) { pinDrawable ->
- val session = activeSessionHolder.getActiveSession()
+ val session = activeSessionHolder.getActiveSession()
+ val roomId = liveLocationShareAggregatedSummary.roomId
+ val matrixItem = if (roomId != null) {
+ session.getRoom(roomId)
+ ?.membershipService()
+ ?.getRoomMember(userId)
+ ?.toMatrixItem()
+ ?: MatrixItem.UserItem(userId)
+ } else {
+ session.getUserOrDefault(userId).toMatrixItem()
+ }
+ locationPinProvider.create(matrixItem) { pinDrawable ->
val locationTimestampMillis = liveLocationShareAggregatedSummary.lastLocationDataContent?.getBestTimestampMillis()
val viewState = UserLiveLocationViewState(
- matrixItem = session.getUserOrDefault(userId).toMatrixItem(),
+ matrixItem = matrixItem,
pinDrawable = pinDrawable,
locationData = locationData,
endOfLiveTimestampMillis = liveLocationShareAggregatedSummary.endOfLiveTimestampMillis,
diff --git a/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewModel.kt b/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewModel.kt
index a1544ac2af..4de005b265 100644
--- a/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewModel.kt
@@ -30,6 +30,8 @@ import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.session.Session
+import org.matrix.android.sdk.api.util.MatrixItem
+import org.matrix.android.sdk.api.util.toMatrixItem
class LocationPreviewViewModel @AssistedInject constructor(
@Assisted private val initialState: LocationPreviewViewState,
@@ -46,12 +48,23 @@ class LocationPreviewViewModel @AssistedInject constructor(
companion object : MavericksViewModelFactory<LocationPreviewViewModel, LocationPreviewViewState> by hiltMavericksViewModelFactory()
init {
- initPin(initialState.pinUserId)
+ val matrixItem = if (initialState.roomId != null && initialState.pinUserId != null) {
+ session
+ .roomService()
+ .getRoom(initialState.roomId)
+ ?.membershipService()
+ ?.getRoomMember(initialState.pinUserId)
+ ?.toMatrixItem()
+ ?: MatrixItem.UserItem(initialState.pinUserId)
+ } else {
+ null
+ }
+ initPin(matrixItem)
initLocationTracking()
}
- private fun initPin(userId: String?) {
- locationPinProvider.create(userId) { pinDrawable ->
+ private fun initPin(matrixItem: MatrixItem?) {
+ locationPinProvider.create(matrixItem) { pinDrawable ->
setState { copy(pinDrawable = pinDrawable) }
}
}
diff --git a/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewState.kt b/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewState.kt
index 23f8d4d7dc..81ea6ebdce 100644
--- a/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewState.kt
+++ b/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewState.kt
@@ -23,6 +23,7 @@ import im.vector.app.features.location.LocationSharingArgs
data class LocationPreviewViewState(
val pinLocationData: LocationData? = null,
+ val roomId: String? = null,
val pinUserId: String? = null,
val pinDrawable: Drawable? = null,
val loadingMapHasFailed: Boolean = false,
@@ -32,6 +33,7 @@ data class LocationPreviewViewState(
constructor(args: LocationSharingArgs) : this(
pinLocationData = args.initialLocationData,
+ roomId = args.roomId,
pinUserId = args.locationOwnerId,
)
}
diff --git a/vector/src/test/java/im/vector/app/features/location/live/GetLiveLocationShareSummaryUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/location/live/GetLiveLocationShareSummaryUseCaseTest.kt
index 89966b5317..e2699e3b0b 100644
--- a/vector/src/test/java/im/vector/app/features/location/live/GetLiveLocationShareSummaryUseCaseTest.kt
+++ b/vector/src/test/java/im/vector/app/features/location/live/GetLiveLocationShareSummaryUseCaseTest.kt
@@ -54,6 +54,7 @@ class GetLiveLocationShareSummaryUseCaseTest {
@Test
fun `given a room id and event id when calling use case then flow on summary is returned`() = runTest {
val summary = LiveLocationShareAggregatedSummary(
+ roomId = A_ROOM_ID,
userId = "userId",
isActive = true,
endOfLiveTimestampMillis = 123,
diff --git a/vector/src/test/java/im/vector/app/features/location/live/map/GetListOfUserLiveLocationUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/location/live/map/GetListOfUserLiveLocationUseCaseTest.kt
index 6d24858915..5bee779453 100644
--- a/vector/src/test/java/im/vector/app/features/location/live/map/GetListOfUserLiveLocationUseCaseTest.kt
+++ b/vector/src/test/java/im/vector/app/features/location/live/map/GetListOfUserLiveLocationUseCaseTest.kt
@@ -59,18 +59,21 @@ class GetListOfUserLiveLocationUseCaseTest {
@Test
fun `given a room id then the correct flow of view states list is collected`() = runTest {
val summary1 = LiveLocationShareAggregatedSummary(
+ roomId = A_ROOM_ID,
userId = "userId1",
isActive = true,
endOfLiveTimestampMillis = 123,
lastLocationDataContent = MessageBeaconLocationDataContent()
)
val summary2 = LiveLocationShareAggregatedSummary(
+ roomId = A_ROOM_ID,
userId = "userId2",
isActive = true,
endOfLiveTimestampMillis = 1234,
lastLocationDataContent = MessageBeaconLocationDataContent()
)
val summary3 = LiveLocationShareAggregatedSummary(
+ roomId = A_ROOM_ID,
userId = "userId3",
isActive = true,
endOfLiveTimestampMillis = 1234,
diff --git a/vector/src/test/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapperTest.kt b/vector/src/test/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapperTest.kt
index 46742da874..8233f8998c 100644
--- a/vector/src/test/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapperTest.kt
+++ b/vector/src/test/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapperTest.kt
@@ -72,6 +72,7 @@ class UserLiveLocationViewStateMapperTest {
@Test
fun `given a summary with invalid data then result is null`() = runTest {
val summary1 = LiveLocationShareAggregatedSummary(
+ roomId = null,
userId = null,
isActive = true,
endOfLiveTimestampMillis = null,
@@ -98,17 +99,19 @@ class UserLiveLocationViewStateMapperTest {
unstableTimestampMillis = A_LOCATION_TIMESTAMP
)
val summary = LiveLocationShareAggregatedSummary(
+ roomId = null,
userId = A_USER_ID,
isActive = A_IS_ACTIVE,
endOfLiveTimestampMillis = A_END_OF_LIVE_TIMESTAMP,
lastLocationDataContent = locationDataContent,
)
- locationPinProvider.givenCreateForUserId(A_USER_ID, pinDrawable)
+ val matrixItem = MatrixItem.UserItem(id = A_USER_ID, displayName = A_USER_DISPLAY_NAME, avatarUrl = "")
+ locationPinProvider.givenCreateForMatrixItem(matrixItem, pinDrawable)
val viewState = userLiveLocationViewStateMapper.map(summary)
val expectedViewState = UserLiveLocationViewState(
- matrixItem = MatrixItem.UserItem(id = A_USER_ID, displayName = A_USER_DISPLAY_NAME, avatarUrl = ""),
+ matrixItem = matrixItem,
pinDrawable = pinDrawable,
locationData = LocationData(
latitude = A_LATITUDE,
diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeLocationPinProvider.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeLocationPinProvider.kt
index 726093215f..c8ba878af1 100644
--- a/vector/src/test/java/im/vector/app/test/fakes/FakeLocationPinProvider.kt
+++ b/vector/src/test/java/im/vector/app/test/fakes/FakeLocationPinProvider.kt
@@ -21,12 +21,13 @@ import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvid
import io.mockk.every
import io.mockk.invoke
import io.mockk.mockk
+import org.matrix.android.sdk.api.util.MatrixItem
class FakeLocationPinProvider {
val instance = mockk<LocationPinProvider>(relaxed = true)
- fun givenCreateForUserId(userId: String, expectedDrawable: Drawable) {
- every { instance.create(userId, captureLambda()) } answers { lambda<(Drawable) -> Unit>().invoke(expectedDrawable) }
+ fun givenCreateForMatrixItem(matrixItem: MatrixItem, expectedDrawable: Drawable) {
+ every { instance.create(matrixItem, captureLambda()) } answers { lambda<(Drawable) -> Unit>().invoke(expectedDrawable) }
}
}
From e3406783499538c943afe375982553eabe024d1b Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoitm@matrix.org>
Date: Mon, 5 Feb 2024 09:43:05 +0100
Subject: [PATCH 7/7] changelog
---
changelog.d/8749.bugfix | 1 +
1 file changed, 1 insertion(+)
create mode 100644 changelog.d/8749.bugfix
diff --git a/changelog.d/8749.bugfix b/changelog.d/8749.bugfix
new file mode 100644
index 0000000000..d9166791bd
--- /dev/null
+++ b/changelog.d/8749.bugfix
@@ -0,0 +1 @@
+Fix issues about location Event avatar rendering.