Tidy and simplify using PR suggestions
This commit is contained in:
Chris Smith
parent
0eb7cd82fc
commit
766ad50d0e
6 changed files with 95 additions and 52 deletions
|
|
@ -19,45 +19,28 @@ package io.element.android.features.location.impl.show
|
|||
import android.content.Context
|
||||
import android.content.Intent
|
||||
import android.net.Uri
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.DisposableEffect
|
||||
import androidx.compose.ui.platform.LocalContext
|
||||
import androidx.annotation.VisibleForTesting
|
||||
import com.squareup.anvil.annotations.ContributesBinding
|
||||
import io.element.android.features.location.api.Location
|
||||
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
|
||||
import io.element.android.libraries.di.AppScope
|
||||
import kotlinx.coroutines.withContext
|
||||
import io.element.android.libraries.di.ApplicationContext
|
||||
import timber.log.Timber
|
||||
import javax.inject.Inject
|
||||
|
||||
@ContributesBinding(AppScope::class)
|
||||
class AndroidLocationActions @Inject constructor(
|
||||
private val coroutineDispatchers: CoroutineDispatchers
|
||||
@ApplicationContext private val appContext: Context
|
||||
) : LocationActions {
|
||||
|
||||
private var activityContext: Context? = null
|
||||
|
||||
@Composable
|
||||
override fun Configure() {
|
||||
val context = LocalContext.current
|
||||
return DisposableEffect(Unit) {
|
||||
activityContext = context
|
||||
onDispose {
|
||||
activityContext = null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
override suspend fun share(location: Location, label: String?) {
|
||||
override fun share(location: Location, label: String?) {
|
||||
runCatching {
|
||||
// Ref: https://developer.android.com/guide/components/intents-common#ViewMap
|
||||
val suffix = if (label != null) "(${Uri.encode(label)})" else ""
|
||||
val uri = Uri.parse("geo:0,0?q=${location.lat},${location.lon}$suffix")
|
||||
val uri = Uri.parse(buildUrl(location, label))
|
||||
val showMapsIntent = Intent(Intent.ACTION_VIEW).setData(uri)
|
||||
val chooserIntent = Intent.createChooser(showMapsIntent, null)
|
||||
withContext(coroutineDispatchers.main) {
|
||||
activityContext!!.startActivity(chooserIntent)
|
||||
}
|
||||
chooserIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
|
||||
appContext.startActivity(chooserIntent)
|
||||
}.onSuccess {
|
||||
Timber.v("Open location succeed")
|
||||
}.onFailure {
|
||||
|
|
@ -65,3 +48,18 @@ class AndroidLocationActions @Inject constructor(
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
internal fun buildUrl(
|
||||
location: Location,
|
||||
label: String?,
|
||||
urlEncoder: (String) -> String = Uri::encode
|
||||
): String {
|
||||
// Ref: https://developer.android.com/guide/components/intents-common#ViewMap
|
||||
val base = "geo:0,0?q=%.6f,%.6f".format(location.lat, location.lon)
|
||||
return if (label == null) {
|
||||
base
|
||||
} else {
|
||||
"%s (%s)".format(base, urlEncoder(label))
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,14 +16,8 @@
|
|||
|
||||
package io.element.android.features.location.impl.show
|
||||
|
||||
import androidx.compose.runtime.Composable
|
||||
import io.element.android.features.location.api.Location
|
||||
|
||||
interface LocationActions {
|
||||
|
||||
@Composable
|
||||
fun Configure()
|
||||
|
||||
suspend fun share(location: Location, label: String?)
|
||||
|
||||
fun share(location: Location, label: String?)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,8 +23,6 @@ import dagger.assisted.AssistedFactory
|
|||
import dagger.assisted.AssistedInject
|
||||
import io.element.android.features.location.api.Location
|
||||
import io.element.android.libraries.architecture.Presenter
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.launch
|
||||
|
||||
class ShowLocationPresenter @AssistedInject constructor(
|
||||
private val actions: LocationActions,
|
||||
|
|
@ -39,20 +37,13 @@ class ShowLocationPresenter @AssistedInject constructor(
|
|||
|
||||
@Composable
|
||||
override fun present(): ShowLocationState {
|
||||
val coroutineScope = rememberCoroutineScope()
|
||||
actions.Configure()
|
||||
|
||||
return ShowLocationState(
|
||||
location = location,
|
||||
description = description
|
||||
) {
|
||||
when (it) {
|
||||
ShowLocationEvents.Share -> coroutineScope.share(location, description)
|
||||
ShowLocationEvents.Share -> actions.share(location, description)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun CoroutineScope.share(location: Location, label: String?) = launch {
|
||||
actions.share(location, label)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,70 @@
|
|||
/*
|
||||
* Copyright (c) 2023 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 io.element.android.features.location.impl.show
|
||||
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.features.location.api.Location
|
||||
import org.junit.Test
|
||||
import java.net.URLEncoder
|
||||
|
||||
internal class AndroidLocationActionsTest {
|
||||
|
||||
// We use an Android-native encoder in the actual app, switch to an equivalent JVM one for the tests
|
||||
private fun urlEncoder(input: String) = URLEncoder.encode(input, "US-ASCII")
|
||||
|
||||
@Test
|
||||
fun `buildUrl - truncates excessive decimals to 6dp`() {
|
||||
val location = Location(
|
||||
lat = 1.234567890123,
|
||||
lon = 123.456789012345,
|
||||
accuracy = 0f
|
||||
)
|
||||
|
||||
val actual = buildUrl(location, null, ::urlEncoder)
|
||||
val expected = "geo:0,0?q=1.234568,123.456789"
|
||||
|
||||
assertThat(actual).isEqualTo(expected)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `buildUrl - appends label if set`() {
|
||||
val location = Location(
|
||||
lat = 1.000001,
|
||||
lon = 2.000001,
|
||||
accuracy = 0f
|
||||
)
|
||||
|
||||
val actual = buildUrl(location, "point", ::urlEncoder)
|
||||
val expected = "geo:0,0?q=1.000001,2.000001 (point)"
|
||||
|
||||
assertThat(actual).isEqualTo(expected)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `buildUrl - URL encodes label`() {
|
||||
val location = Location(
|
||||
lat = 1.000001,
|
||||
lon = 2.000001,
|
||||
accuracy = 0f
|
||||
)
|
||||
|
||||
val actual = buildUrl(location, "(weird/stuff here)", ::urlEncoder)
|
||||
val expected = "geo:0,0?q=1.000001,2.000001 (%28weird%2Fstuff+here%29)"
|
||||
|
||||
assertThat(actual).isEqualTo(expected)
|
||||
}
|
||||
}
|
||||
|
|
@ -16,26 +16,17 @@
|
|||
|
||||
package io.element.android.features.location.impl.show
|
||||
|
||||
import androidx.compose.runtime.Composable
|
||||
import io.element.android.features.location.api.Location
|
||||
|
||||
class FakeLocationActions : LocationActions {
|
||||
|
||||
var configured = false
|
||||
private set
|
||||
|
||||
var sharedLocation: Location? = null
|
||||
private set
|
||||
|
||||
var sharedLabel: String? = null
|
||||
private set
|
||||
|
||||
@Composable
|
||||
override fun Configure() {
|
||||
configured = true
|
||||
}
|
||||
|
||||
override suspend fun share(location: Location, label: String?) {
|
||||
override fun share(location: Location, label: String?) {
|
||||
sharedLocation = location
|
||||
sharedLabel = label
|
||||
}
|
||||
|
|
|
|||
|
|
@ -60,7 +60,6 @@ class ShowLocationPresenterTest {
|
|||
val initialState = awaitItem()
|
||||
initialState.eventSink(ShowLocationEvents.Share)
|
||||
|
||||
Truth.assertThat(actions.configured).isTrue()
|
||||
Truth.assertThat(actions.sharedLocation).isEqualTo(location)
|
||||
Truth.assertThat(actions.sharedLabel).isEqualTo(A_DESCRIPTION)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue