Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored location sharing timeline items to use the mapserver configured by the wellknown file #7852

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7852.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Refactored location sharing timeline items to use the mapserver configured by the wellknown file
2 changes: 0 additions & 2 deletions library/ui-strings/src/main/res/values/donottranslate.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,4 @@
<!-- onboarding english only word play -->
<string name="cut_the_slack_from_teams" translatable="false">Cut the slack from teams.</string>

<!-- WIP -->
<string name="location_map_view_copyright" translatable="false">© MapTiler © OpenStreetMap contributors</string>
</resources>
1 change: 1 addition & 0 deletions vector-config/src/main/java/im/vector/app/config/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ object Config {

const val ENABLE_LOCATION_SHARING = true
const val LOCATION_MAP_TILER_KEY = "fU3vlMsMn4Jb6dnEIFsx"
const val ENABLE_LOCATION_SHARING_MAPSERVER_FALLBACK = true

/**
* The maximum length of voice messages in milliseconds.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ object ConfigurationModule {
@Provides
fun providesLocationSharingConfig() = LocationSharingConfig(
mapTilerKey = Config.LOCATION_MAP_TILER_KEY,
isMapTilerFallbackEnabled = Config.ENABLE_LOCATION_SHARING_MAPSERVER_FALLBACK
)

@Provides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import android.widget.TextView
import androidx.core.view.isVisible
import com.airbnb.epoxy.EpoxyAttribute
import com.airbnb.epoxy.EpoxyModelClass
import com.bumptech.glide.request.RequestOptions
import com.mapbox.mapboxsdk.maps.MapView
import im.vector.app.R
import im.vector.app.core.epoxy.ClickListener
import im.vector.app.core.epoxy.VectorEpoxyHolder
Expand All @@ -36,6 +36,8 @@ import im.vector.app.features.home.AvatarRenderer
import im.vector.app.features.home.room.detail.timeline.action.LocationUiData
import im.vector.app.features.home.room.detail.timeline.item.BindingOptions
import im.vector.app.features.home.room.detail.timeline.tools.findPillsAndProcess
import im.vector.app.features.location.INITIAL_MAP_ZOOM_IN_TIMELINE
import im.vector.app.features.location.zoomToLocation
import im.vector.app.features.media.ImageContentRenderer
import im.vector.lib.core.utils.epoxy.charsequence.EpoxyCharSequence
import org.matrix.android.sdk.api.util.MatrixItem
Expand Down Expand Up @@ -98,10 +100,13 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
holder.body.isVisible = locationUiData == null
holder.mapViewContainer.isVisible = locationUiData != null
locationUiData?.let { safeLocationUiData ->
GlideApp.with(holder.staticMapImageView)
.load(safeLocationUiData.locationUrl)
.apply(RequestOptions.centerCropTransform())
.into(holder.staticMapImageView)
holder.staticMapView.getMapAsync { mapbox ->
mapbox.setStyle(safeLocationUiData.locationUrl)
safeLocationUiData.locationData?.let {
mapbox.zoomToLocation(it, false, INITIAL_MAP_ZOOM_IN_TIMELINE)
}
mapbox.uiSettings.setAllGesturesEnabled(false)
}

safeLocationUiData.locationPinProvider.create(safeLocationUiData.locationOwnerId) { pinDrawable ->
GlideApp.with(holder.staticMapPinImageView)
Expand All @@ -124,7 +129,7 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
val timestamp by bind<TextView>(R.id.bottom_sheet_message_preview_timestamp)
val imagePreview by bind<ImageView>(R.id.bottom_sheet_message_preview_image)
val mapViewContainer by bind<FrameLayout>(R.id.mapViewContainer)
val staticMapImageView by bind<ImageView>(R.id.staticMapImageView)
val staticMapView by bind<MapView>(R.id.staticMapView)
val staticMapPinImageView by bind<ImageView>(R.id.staticMapPinImageView)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import im.vector.app.core.platform.EmptyViewEvents
import im.vector.app.core.platform.VectorViewModel
import im.vector.app.core.platform.VectorViewModelAction
import im.vector.app.features.VectorFeatures
import im.vector.app.features.location.UrlMapProvider
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import should be removed.

import im.vector.app.features.settings.VectorPreferences

class AttachmentTypeSelectorViewModel @AssistedInject constructor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ import im.vector.app.features.analytics.extensions.toAnalyticsType
import im.vector.app.features.analytics.plan.Signup
import im.vector.app.features.analytics.store.AnalyticsStore
import im.vector.app.features.home.room.list.home.release.ReleaseNotesPreferencesStore
import im.vector.app.features.location.UrlMapProvider
import im.vector.app.features.login.ReAuthHelper
import im.vector.app.features.onboarding.AuthenticationDescription
import im.vector.app.features.raw.wellknown.ElementWellKnown
import im.vector.app.features.raw.wellknown.WellknownService
import im.vector.app.features.raw.wellknown.getElementWellknown
import im.vector.app.features.raw.wellknown.isSecureBackupRequired
import im.vector.app.features.raw.wellknown.withElementWellKnown
Expand All @@ -54,6 +56,7 @@ import kotlinx.coroutines.flow.onCompletion
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.takeWhile
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.MatrixPatterns.getServerName
import org.matrix.android.sdk.api.auth.UIABaseAuth
import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor
import org.matrix.android.sdk.api.auth.UserPasswordAuth
Expand Down Expand Up @@ -82,7 +85,7 @@ import kotlin.coroutines.resumeWithException
class HomeActivityViewModel @AssistedInject constructor(
@Assisted private val initialState: HomeActivityViewState,
private val activeSessionHolder: ActiveSessionHolder,
private val rawService: RawService,
private val wellknownService: WellknownService,
private val reAuthHelper: ReAuthHelper,
private val analyticsStore: AnalyticsStore,
private val lightweightSettingsStorage: LightweightSettingsStorage,
Expand Down Expand Up @@ -231,7 +234,7 @@ class HomeActivityViewModel @AssistedInject constructor(
.onEach { info ->
val isVerified = info.getOrNull()?.isTrusted() ?: false
if (!isVerified && onceTrusted) {
rawService.withElementWellKnown(viewModelScope, safeActiveSession.sessionParams) {
wellknownService.getElementWellknown(safeActiveSession.sessionParams.userId.getServerName())?.let {
sessionHasBeenUnverified(it)
}
}
Expand Down Expand Up @@ -382,7 +385,8 @@ class HomeActivityViewModel @AssistedInject constructor(
Timber.w("## No session to init cross signing or bootstrap")
}

val elementWellKnown = rawService.getElementWellknown(session.sessionParams)
val elementWellKnown = wellknownService.getElementWellknown(session.sessionParams.userId.getServerName())

val isSecureBackupRequired = elementWellKnown?.isSecureBackupRequired() ?: false

// In case of account creation, it is already done before
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
package im.vector.app.features.home.room.detail.timeline.action

import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvider
import im.vector.app.features.location.LocationData

/**
* Data used to display Location data in the message bottom sheet.
*/
data class LocationUiData(
val locationData: LocationData?,
val locationUrl: String,
val locationOwnerId: String?,
val locationPinProvider: LocationPinProvider,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import im.vector.app.features.home.room.detail.timeline.item.E2EDecoration
import im.vector.app.features.home.room.detail.timeline.tools.createLinkMovementMethod
import im.vector.app.features.home.room.detail.timeline.tools.linkify
import im.vector.app.features.html.SpanUtils
import im.vector.app.features.location.INITIAL_MAP_ZOOM_IN_TIMELINE
import im.vector.app.features.location.UrlMapProvider
import im.vector.app.features.location.toLocationData
import im.vector.app.features.media.ImageContentRenderer
Expand Down Expand Up @@ -228,13 +227,12 @@ class MessageActionsEpoxyController @Inject constructor(

val locationContent = state.timelineEvent()?.root?.getClearContent().toModel<MessageLocationContent>(catchError = true)
?: return null
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

return LocationUiData(
locationUrl = locationUrl,
locationData = locationContent.toLocationData(),
locationUrl = urlMapProvider.getMapStyleUrl(),
locationOwnerId = locationOwnerId,
locationPinProvider = locationPinProvider,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import im.vector.app.features.home.room.detail.timeline.item.MessageLiveLocation
import im.vector.app.features.home.room.detail.timeline.item.MessageLiveLocationItem_
import im.vector.app.features.home.room.detail.timeline.item.MessageLiveLocationStartItem
import im.vector.app.features.home.room.detail.timeline.item.MessageLiveLocationStartItem_
import im.vector.app.features.location.INITIAL_MAP_ZOOM_IN_TIMELINE
import im.vector.app.features.location.UrlMapProvider
import im.vector.app.features.location.toLocationData
import org.matrix.android.sdk.api.session.Session
Expand Down Expand Up @@ -105,13 +104,12 @@ class LiveLocationShareMessageItemFactory @Inject constructor(
val width = timelineMediaSizeProvider.getMaxSize().first
val height = dimensionConverter.dpToPx(MessageItemFactory.MESSAGE_LOCATION_ITEM_HEIGHT_IN_DP)

val locationUrl = runningState.lastGeoUri.toLocationData()?.let {
urlMapProvider.buildStaticMapUrl(it, INITIAL_MAP_ZOOM_IN_TIMELINE, width, height)
}
val lastLocationData = runningState.lastGeoUri.toLocationData()

return MessageLiveLocationItem_()
.attributes(attributes)
.locationUrl(locationUrl)
.locationData(lastLocationData)
.mapStyleUrl(urlMapProvider.getMapStyleUrl())
.mapWidth(width)
.mapHeight(height)
.locationUserId(attributes.informationData.senderId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ import im.vector.app.features.html.EventHtmlRenderer
import im.vector.app.features.html.PillsPostProcessor
import im.vector.app.features.html.SpanUtils
import im.vector.app.features.html.VectorHtmlCompressor
import im.vector.app.features.location.INITIAL_MAP_ZOOM_IN_TIMELINE
import im.vector.app.features.location.UrlMapProvider
import im.vector.app.features.location.toLocationData
import im.vector.app.features.media.ImageContentRenderer
Expand Down Expand Up @@ -222,15 +221,12 @@ class MessageItemFactory @Inject constructor(
val width = timelineMediaSizeProvider.getMaxSize().first
val height = dimensionConverter.dpToPx(MESSAGE_LOCATION_ITEM_HEIGHT_IN_DP)

val locationUrl = locationContent.toLocationData()?.let {
urlMapProvider.buildStaticMapUrl(it, INITIAL_MAP_ZOOM_IN_TIMELINE, width, height)
}

val locationUserId = if (locationContent.isSelfLocation()) informationData.senderId else null

return MessageLocationItem_()
.attributes(attributes)
.locationUrl(locationUrl)
.locationData(locationContent.toLocationData())
.mapStyleUrl(urlMapProvider.getMapStyleUrl())
.mapWidth(width)
.mapHeight(height)
.locationUserId(locationUserId)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2021 New Vector Ltd
* Copyright (c) 2022 BWI GmbH
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are there these changes of copyrights? I am not entirely sure is is allowed to be changed to something else than New Vector Ltd.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like @artkoenig has added here the copyright notice to show that copyright goes to BWI GmbH.
It is fine to have the joint copyright here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an expert in this area but I know there has been irritation about this topic recently. So just to double check that we're doing the correct thing here: Has this particular question been confirmed with legal?

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,35 +17,34 @@

package im.vector.app.features.home.room.detail.timeline.item

import android.graphics.drawable.Drawable
import android.widget.ImageView
import android.widget.TextView
import androidx.annotation.IdRes
import androidx.annotation.LayoutRes
import androidx.core.view.isVisible
import androidx.core.view.updateLayoutParams
import com.airbnb.epoxy.EpoxyAttribute
import com.bumptech.glide.load.DataSource
import com.bumptech.glide.load.engine.GlideException
import com.bumptech.glide.load.resource.bitmap.RoundedCorners
import com.bumptech.glide.request.RequestListener
import com.bumptech.glide.request.RequestOptions
import com.bumptech.glide.request.target.Target
import com.mapbox.mapboxsdk.maps.MapView
import im.vector.app.R
import im.vector.app.core.glide.GlideApp
import im.vector.app.core.utils.DimensionConverter
import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvider
import im.vector.app.features.home.room.detail.timeline.style.TimelineMessageLayout
import im.vector.app.features.home.room.detail.timeline.style.granularRoundedCorners
import im.vector.app.features.location.INITIAL_MAP_ZOOM_IN_TIMELINE
import im.vector.app.features.location.LocationData
import im.vector.app.features.location.MapLoadingErrorView
import im.vector.app.features.location.MapLoadingErrorViewState
import im.vector.app.features.location.zoomToLocation

abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
@LayoutRes layoutId: Int = R.layout.item_timeline_event_base
) : AbsMessageItem<H>(layoutId) {

@EpoxyAttribute
var locationUrl: String? = null
var locationData: LocationData? = null

@EpoxyAttribute
var mapStyleUrl: String? = null

@EpoxyAttribute
var locationUserId: String? = null
Expand All @@ -64,62 +64,65 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
bindMap(holder)
}

override fun onViewAttachedToWindow(holder: H) {
super.onViewAttachedToWindow(holder)
holder.staticMapView.onStart()
}

override fun onViewDetachedFromWindow(holder: H) {
super.onViewDetachedFromWindow(holder)
holder.staticMapView.onStop()
}

private fun bindMap(holder: Holder) {
val location = locationUrl ?: return
val messageLayout = attributes.informationData.messageLayout
val imageCornerTransformation = if (messageLayout is TimelineMessageLayout.Bubble) {
messageLayout.cornersRadius.granularRoundedCorners()
} else {
val dimensionConverter = DimensionConverter(holder.view.resources)
RoundedCorners(dimensionConverter.dpToPx(8))
}
holder.staticMapImageView.updateLayoutParams {
width = mapWidth
height = mapHeight

holder.staticMapView.apply {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general practice in the project, we try to avoid using apply as much as possible as it leads sometimes to strange bugs since the scope of this is changed inside.

updateLayoutParams {
width = mapWidth
height = mapHeight
}
addOnDidFailLoadingMapListener {
holder.staticMapLoadingErrorView.isVisible = true
val mapErrorViewState = MapLoadingErrorViewState(imageCornerTransformation)
holder.staticMapLoadingErrorView.render(mapErrorViewState)
}

addOnDidFinishLoadingMapListener {
locationPinProvider?.create(locationUserId) { pinDrawable ->
holder.staticMapPinImageView.setImageDrawable(pinDrawable)
}
holder.staticMapLoadingErrorView.isVisible = false
}

clipToOutline = true
getMapAsync { mapbox ->
mapbox.setStyle(mapStyleUrl)
locationData?.let {
mapbox.zoomToLocation(it, false, INITIAL_MAP_ZOOM_IN_TIMELINE)
}
mapbox.uiSettings.setAllGesturesEnabled(false)
mapbox.addOnMapClickListener {
attributes.itemClickListener?.invoke(holder.staticMapView)
true
}
mapbox.addOnMapLongClickListener {
attributes.itemLongClickListener?.onLongClick(holder.staticMapView)
true
}
}
}
GlideApp.with(holder.staticMapImageView)
.load(location)
.apply(RequestOptions.centerCropTransform())
.placeholder(holder.staticMapImageView.drawable)
.listener(object : RequestListener<Drawable> {
override fun onLoadFailed(
e: GlideException?,
model: Any?,
target: Target<Drawable>?,
isFirstResource: Boolean
): Boolean {
holder.staticMapPinImageView.setImageDrawable(null)
holder.staticMapLoadingErrorView.isVisible = true
val mapErrorViewState = MapLoadingErrorViewState(imageCornerTransformation)
holder.staticMapLoadingErrorView.render(mapErrorViewState)
holder.staticMapCopyrightTextView.isVisible = false
return false
}

override fun onResourceReady(
resource: Drawable?,
model: Any?,
target: Target<Drawable>?,
dataSource: DataSource?,
isFirstResource: Boolean
): Boolean {
locationPinProvider?.create(locationUserId) { pinDrawable ->
// we are not using Glide since it does not display it correctly when there is no user photo
holder.staticMapPinImageView.setImageDrawable(pinDrawable)
}
holder.staticMapLoadingErrorView.isVisible = false
holder.staticMapCopyrightTextView.isVisible = true
return false
}
})
.transform(imageCornerTransformation)
.into(holder.staticMapImageView)
}

abstract class Holder(@IdRes stubId: Int) : AbsMessageItem.Holder(stubId) {
val staticMapImageView by bind<ImageView>(R.id.staticMapImageView)
val staticMapView by bind<MapView>(R.id.staticMapView)
val staticMapPinImageView by bind<ImageView>(R.id.staticMapPinImageView)
val staticMapLoadingErrorView by bind<MapLoadingErrorView>(R.id.staticMapLoadingError)
val staticMapCopyrightTextView by bind<TextView>(R.id.staticMapCopyrightTextView)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ package im.vector.app.features.location

data class LocationSharingConfig(
val mapTilerKey: String,
val isMapTilerFallbackEnabled: Boolean
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a trailing comma here?

)
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class LocationSharingFragment :

lifecycleScope.launchWhenCreated {
views.mapView.initialize(
url = urlMapProvider.getMapUrl(),
url = urlMapProvider.getMapStyleUrl(),
locationTargetChangeListener = this@LocationSharingFragment
)
}
Expand Down
Loading