Code review fixes.

This commit is contained in:
Onuray Sahin 2022-01-17 18:30:04 +03:00
parent f9c5b2021d
commit 5581e0b5ba
19 changed files with 123 additions and 90 deletions

View File

@ -22,7 +22,7 @@ import com.squareup.moshi.JsonClass
@JsonClass(generateAdapter = true)
data class LocationInfo(
/**
* Required. Required. RFC5870 formatted geo uri 'geo:latitude,longitude;uncertainty' like 'geo:40.05,29.24;30' representing this location.
* Required. RFC5870 formatted geo uri 'geo:latitude,longitude;uncertainty' like 'geo:40.05,29.24;30' representing this location.
*/
@Json(name = "uri") val geoUri: String? = null,

View File

@ -36,7 +36,6 @@ 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.LocationData
import im.vector.app.features.location.MapTilerMapView
import im.vector.app.features.location.VectorMapListener
import im.vector.app.features.media.ImageContentRenderer
import org.matrix.android.sdk.api.util.MatrixItem
@ -101,15 +100,15 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
holder.mapView.isVisible = locationData != null
holder.body.isVisible = locationData == null
locationData?.let { location ->
holder.mapView.initialize(object : VectorMapListener {
override fun onMapReady() {
holder.mapView.initialize {
if (holder.view.isAttachedToWindow) {
holder.mapView.zoomToLocation(location.latitude, location.longitude, 15.0)
locationPinProvider?.create(matrixItem.id) { pinDrawable ->
holder.mapView.addPinToMap(matrixItem.id, pinDrawable)
holder.mapView.updatePinLocation(matrixItem.id, location.latitude, location.longitude)
}
}
})
}
}
}

View File

@ -89,6 +89,9 @@ class DisplayableEventFormatter @Inject constructor(
MessageType.MSGTYPE_FILE -> {
simpleFormat(senderName, stringProvider.getString(R.string.sent_a_file), appendAuthor)
}
MessageType.MSGTYPE_LOCATION -> {
simpleFormat(senderName, stringProvider.getString(R.string.sent_location), appendAuthor)
}
else -> {
simpleFormat(senderName, messageContent.body, appendAuthor)
}

View File

@ -23,10 +23,10 @@ import androidx.core.content.ContextCompat
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.features.home.AvatarRenderer
import org.billcarsonfr.jsonviewer.Utils
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.util.toMatrixItem
import javax.inject.Inject
import javax.inject.Singleton
@ -34,7 +34,7 @@ import javax.inject.Singleton
@Singleton
class LocationPinProvider @Inject constructor(
private val context: Context,
private val session: Session,
private val activeSessionHolder: ActiveSessionHolder,
private val avatarRenderer: AvatarRenderer
) {
private val cache = mutableMapOf<String, Drawable>()
@ -49,7 +49,7 @@ class LocationPinProvider @Inject constructor(
return
}
session.getUser(userId)?.toMatrixItem()?.let {
activeSessionHolder.getActiveSession().getUser(userId)?.toMatrixItem()?.let {
val size = Utils.dpToPx(44, context)
avatarRenderer.render(glideRequests, it, object : CustomTarget<Drawable>(size, size) {
override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {

View File

@ -25,7 +25,6 @@ import im.vector.app.core.epoxy.onClick
import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvider
import im.vector.app.features.location.LocationData
import im.vector.app.features.location.MapTilerMapView
import im.vector.app.features.location.VectorMapListener
@EpoxyModelClass(layout = R.layout.item_timeline_event_base)
abstract class MessageLocationItem : AbsMessageItem<MessageLocationItem.Holder>() {
@ -58,16 +57,14 @@ abstract class MessageLocationItem : AbsMessageItem<MessageLocationItem.Holder>(
}
holder.mapView.apply {
initialize(object : VectorMapListener {
override fun onMapReady() {
zoomToLocation(location.latitude, location.longitude, INITIAL_ZOOM)
initialize {
zoomToLocation(location.latitude, location.longitude, INITIAL_ZOOM)
locationPinProvider?.create(locationOwnerId) { pinDrawable ->
addPinToMap(locationOwnerId, pinDrawable)
updatePinLocation(locationOwnerId, location.latitude, location.longitude)
}
locationPinProvider?.create(locationOwnerId) { pinDrawable ->
addPinToMap(locationOwnerId, pinDrawable)
updatePinLocation(locationOwnerId, location.latitude, location.longitude)
}
})
}
}
}

View File

@ -0,0 +1,21 @@
/*
* Copyright (c) 2022 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package im.vector.app.features.location
const val INITIAL_MAP_ZOOM = 15.0
const val MIN_TIME_MILLIS_TO_UPDATE_LOCATION = 1 * 60 * 1000L // every 1 minute
const val MIN_DISTANCE_METERS_TO_UPDATE_LOCATION = 10f

View File

@ -26,19 +26,6 @@ data class LocationData(
val uncertainty: Double?
) : Parcelable {
fun toGeoUri(): String {
return buildString {
append("geo:")
append(latitude)
append(",")
append(longitude)
append("?q=")
append(latitude)
append(",")
append(longitude)
}
}
companion object {
/**

View File

@ -31,7 +31,7 @@ import javax.inject.Inject
class LocationPreviewFragment @Inject constructor(
private val locationPinProvider: LocationPinProvider
) : VectorBaseFragment<FragmentLocationPreviewBinding>(), VectorMapListener {
) : VectorBaseFragment<FragmentLocationPreviewBinding>() {
private val args: LocationSharingArgs by args()
@ -42,7 +42,11 @@ class LocationPreviewFragment @Inject constructor(
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
views.mapView.initialize(this)
views.mapView.initialize {
if (isAdded) {
onMapReady()
}
}
}
override fun getMenuRes() = R.menu.menu_location_preview
@ -62,21 +66,17 @@ class LocationPreviewFragment @Inject constructor(
openLocation(requireActivity(), location.latitude, location.longitude)
}
override fun onMapReady() {
private fun onMapReady() {
val location = args.initialLocationData ?: return
val userId = args.locationOwnerId
locationPinProvider.create(userId) { pinDrawable ->
views.mapView.apply {
zoomToLocation(location.latitude, location.longitude, INITIAL_ZOOM)
zoomToLocation(location.latitude, location.longitude, INITIAL_MAP_ZOOM)
deleteAllPins()
addPinToMap(userId, pinDrawable)
updatePinLocation(userId, location.latitude, location.longitude)
}
}
}
companion object {
const val INITIAL_ZOOM = 15.0
}
}

View File

@ -21,4 +21,5 @@ import im.vector.app.core.platform.VectorViewModelAction
sealed class LocationSharingAction : VectorViewModelAction {
data class OnLocationUpdate(val locationData: LocationData) : LocationSharingAction()
object OnShareLocation : LocationSharingAction()
object OnLocationProviderIsNotAvailable : LocationSharingAction()
}

View File

@ -20,9 +20,10 @@ import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import com.airbnb.mvrx.activityViewModel
import com.airbnb.mvrx.fragmentViewModel
import com.google.android.material.dialog.MaterialAlertDialogBuilder
import im.vector.app.R
import im.vector.app.core.extensions.exhaustive
import im.vector.app.core.platform.VectorBaseFragment
import im.vector.app.databinding.FragmentLocationSharingBinding
import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvider
@ -33,13 +34,13 @@ class LocationSharingFragment @Inject constructor(
private val locationTracker: LocationTracker,
private val session: Session,
private val locationPinProvider: LocationPinProvider
) : VectorBaseFragment<FragmentLocationSharingBinding>(), LocationTracker.Callback, VectorMapListener {
) : VectorBaseFragment<FragmentLocationSharingBinding>(), LocationTracker.Callback {
init {
locationTracker.callback = this
}
private val viewModel: LocationSharingViewModel by activityViewModel()
private val viewModel: LocationSharingViewModel by fragmentViewModel()
private var lastZoomValue: Double = -1.0
@ -50,7 +51,11 @@ class LocationSharingFragment @Inject constructor(
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
views.mapView.initialize(this)
views.mapView.initialize {
if (isAdded) {
onMapReady()
}
}
views.shareLocationContainer.debouncedClicks {
viewModel.handle(LocationSharingAction.OnShareLocation)
@ -60,7 +65,7 @@ class LocationSharingFragment @Inject constructor(
when (it) {
LocationSharingViewEvents.LocationNotAvailableError -> handleLocationNotAvailableError()
LocationSharingViewEvents.Close -> activity?.finish()
}
}.exhaustive
}
}
@ -69,7 +74,7 @@ class LocationSharingFragment @Inject constructor(
locationTracker.stop()
}
override fun onMapReady() {
private fun onMapReady() {
locationPinProvider.create(session.myUserId) {
views.mapView.addPinToMap(
pinId = USER_PIN_NAME,
@ -81,7 +86,7 @@ class LocationSharingFragment @Inject constructor(
}
override fun onLocationUpdate(locationData: LocationData) {
lastZoomValue = if (lastZoomValue == -1.0) INITIAL_ZOOM else views.mapView.getCurrentZoom() ?: INITIAL_ZOOM
lastZoomValue = if (lastZoomValue == -1.0) INITIAL_MAP_ZOOM else views.mapView.getCurrentZoom() ?: INITIAL_MAP_ZOOM
views.mapView.zoomToLocation(locationData.latitude, locationData.longitude, lastZoomValue)
views.mapView.deleteAllPins()
@ -90,16 +95,21 @@ class LocationSharingFragment @Inject constructor(
viewModel.handle(LocationSharingAction.OnLocationUpdate(locationData))
}
override fun onLocationProviderIsNotAvailable() {
viewModel.handle(LocationSharingAction.OnLocationProviderIsNotAvailable)
}
private fun handleLocationNotAvailableError() {
MaterialAlertDialogBuilder(requireActivity())
.setTitle(R.string.location_not_available_dialog_title)
.setMessage(R.string.location_not_available_dialog_content)
.setPositiveButton(R.string.ok, null)
.setPositiveButton(R.string.ok) { _, _ ->
activity?.finish()
}
.show()
}
companion object {
const val INITIAL_ZOOM = 15.0
const val USER_PIN_NAME = "USER_PIN_NAME"
}
}

View File

@ -22,6 +22,7 @@ import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import im.vector.app.core.di.MavericksAssistedViewModelFactory
import im.vector.app.core.di.hiltMavericksViewModelFactory
import im.vector.app.core.extensions.exhaustive
import im.vector.app.core.platform.VectorViewModel
import org.matrix.android.sdk.api.session.Session
@ -42,9 +43,10 @@ class LocationSharingViewModel @AssistedInject constructor(
override fun handle(action: LocationSharingAction) {
when (action) {
is LocationSharingAction.OnLocationUpdate -> handleLocationUpdate(action.locationData)
LocationSharingAction.OnShareLocation -> handleShareLocation()
}
is LocationSharingAction.OnLocationUpdate -> handleLocationUpdate(action.locationData)
LocationSharingAction.OnShareLocation -> handleShareLocation()
LocationSharingAction.OnLocationProviderIsNotAvailable -> handleLocationProviderIsNotAvailable()
}.exhaustive
}
private fun handleShareLocation() = withState { state ->
@ -65,4 +67,8 @@ class LocationSharingViewModel @AssistedInject constructor(
copy(lastKnownLocation = locationData)
}
}
private fun handleLocationProviderIsNotAvailable() {
_viewEvents.post(LocationSharingViewEvents.LocationNotAvailableError)
}
}

View File

@ -27,15 +27,18 @@ import timber.log.Timber
import javax.inject.Inject
class LocationTracker @Inject constructor(
private val context: Context) : LocationListener {
private val context: Context
) : LocationListener {
interface Callback {
fun onLocationUpdate(locationData: LocationData)
fun onLocationProviderIsNotAvailable()
}
private var locationManager: LocationManager? = null
var callback: Callback? = null
@RequiresPermission(anyOf = [Manifest.permission.ACCESS_COARSE_LOCATION, Manifest.permission.ACCESS_FINE_LOCATION])
fun start() {
val locationManager = context.getSystemService<LocationManager>()
@ -47,6 +50,7 @@ class LocationTracker @Inject constructor(
isGpsEnabled -> LocationManager.GPS_PROVIDER
isNetworkEnabled -> LocationManager.NETWORK_PROVIDER
else -> {
callback?.onLocationProviderIsNotAvailable()
Timber.v("## LocationTracker. There is no location provider available")
return
}
@ -54,16 +58,17 @@ class LocationTracker @Inject constructor(
// Send last known location without waiting location updates
it.getLastKnownLocation(provider)?.let { lastKnownLocation ->
callback?.onLocationUpdate(LocationData(lastKnownLocation.latitude, lastKnownLocation.longitude, lastKnownLocation.accuracy.toDouble()))
callback?.onLocationUpdate(lastKnownLocation.toLocationData())
}
it.requestLocationUpdates(
provider,
MIN_TIME_MILLIS_TO_UPDATE,
MIN_DISTANCE_METERS_TO_UPDATE,
MIN_TIME_MILLIS_TO_UPDATE_LOCATION,
MIN_DISTANCE_METERS_TO_UPDATE_LOCATION,
this
)
} ?: run {
callback?.onLocationProviderIsNotAvailable()
Timber.v("## LocationTracker. LocationManager is not available")
}
}
@ -75,11 +80,10 @@ class LocationTracker @Inject constructor(
}
override fun onLocationChanged(location: Location) {
callback?.onLocationUpdate(LocationData(location.latitude, location.longitude, location.accuracy.toDouble()))
callback?.onLocationUpdate(location.toLocationData())
}
companion object {
const val MIN_TIME_MILLIS_TO_UPDATE = 1 * 60 * 1000L // every 1 minute
const val MIN_DISTANCE_METERS_TO_UPDATE = 10f
private fun Location.toLocationData(): LocationData {
return LocationData(latitude, longitude, accuracy.toDouble())
}
}

View File

@ -39,13 +39,13 @@ class MapTilerMapView @JvmOverloads constructor(
private var symbolManager: SymbolManager? = null
private var style: Style? = null
override fun initialize(listener: VectorMapListener) {
override fun initialize(onMapReady: () -> Unit) {
getMapAsync { map ->
map.setStyle(styleUrl) { style ->
this.symbolManager = SymbolManager(this, map, style)
this.map = map
this.style = style
listener.onMapReady()
onMapReady()
}
}
}

View File

@ -18,12 +18,8 @@ package im.vector.app.features.location
import android.graphics.drawable.Drawable
interface VectorMapListener {
fun onMapReady()
}
interface VectorMapView {
fun initialize(listener: VectorMapListener)
fun initialize(onMapReady: () -> Unit)
fun addPinToMap(pinId: String, image: Drawable)
fun updatePinLocation(pinId: String, latitude: Double, longitude: Double)

View File

@ -1000,6 +1000,6 @@ class VectorPreferences @Inject constructor(private val context: Context) {
}
fun isLocationSharingEnabled(): Boolean {
return defaultPrefs.getBoolean(SETTINGS_PREF_ENABLE_LOCATION_SHARING, true)
return defaultPrefs.getBoolean(SETTINGS_PREF_ENABLE_LOCATION_SHARING, false)
}
}

View File

@ -108,7 +108,7 @@
android:layout_width="0dp"
android:layout_height="200dp"
android:layout_marginTop="6dp"
android:importantForAccessibility="no"
android:contentDescription="@string/attachment_type_location"
android:scaleType="centerCrop"
android:visibility="gone"
app:layout_constraintEnd_toEndOf="@id/bottom_sheet_message_preview_timestamp"

View File

@ -1,26 +1,34 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
<androidx.cardview.widget.CardView xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:id="@+id/mapViewContainer"
android:layout_width="match_parent"
android:layout_height="wrap_content">
android:layout_height="wrap_content"
app:cardCornerRadius="8dp">
<im.vector.app.features.location.MapTilerMapView
android:id="@+id/mapView"
android:layout_width="0dp"
android:layout_height="200dp"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/mapViewContainer"
android:layout_width="match_parent"
android:layout_height="wrap_content">
<FrameLayout
android:id="@+id/clickableMapArea"
android:layout_width="0dp"
android:layout_height="0dp"
app:layout_constraintBottom_toBottomOf="@id/mapView"
app:layout_constraintEnd_toEndOf="@id/mapView"
app:layout_constraintStart_toStartOf="@id/mapView"
app:layout_constraintTop_toTopOf="@id/mapView" />
<im.vector.app.features.location.MapTilerMapView
android:id="@+id/mapView"
android:layout_width="0dp"
android:layout_height="200dp"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
</androidx.constraintlayout.widget.ConstraintLayout>
<FrameLayout
android:id="@+id/clickableMapArea"
android:layout_width="0dp"
android:layout_height="0dp"
app:layout_constraintBottom_toBottomOf="@id/mapView"
app:layout_constraintEnd_toEndOf="@id/mapView"
app:layout_constraintStart_toStartOf="@id/mapView"
app:layout_constraintTop_toTopOf="@id/mapView" />
</androidx.constraintlayout.widget.ConstraintLayout>
</androidx.cardview.widget.CardView>

View File

@ -2755,6 +2755,7 @@
<string name="sent_a_poll">Poll</string>
<string name="sent_a_reaction">Reacted with: %s</string>
<string name="sent_verification_conclusion">Verification Conclusion</string>
<string name="sent_location">Shared their location</string>
<string name="verification_request_waiting">Waiting…</string>
<string name="verification_request_other_cancelled">%s cancelled</string>
@ -3704,8 +3705,8 @@
<string name="location_activity_title_preview">Location</string>
<string name="a11y_location_share_icon">Share location</string>
<string name="location_share">Share location</string>
<string name="location_not_available_dialog_title">Element could not access your location</string>
<string name="location_not_available_dialog_content">Element could not access your location. Please try again later.</string>
<string name="template_location_not_available_dialog_title">${app_name} could not access your location</string>
<string name="template_location_not_available_dialog_content">${app_name} could not access your location. Please try again later.</string>
<string name="location_share_external">Open with</string>
<string name="settings_enable_location_sharing">Enable location sharing</string>

View File

@ -29,7 +29,7 @@
android:title="@string/font_size" />
<im.vector.app.core.preference.VectorSwitchPreference
android:defaultValue="true"
android:defaultValue="false"
android:key="SETTINGS_PREF_ENABLE_LOCATION_SHARING"
android:title="@string/settings_enable_location_sharing" />