diff --git a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/AndroidLocationActions.kt b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/AndroidLocationActions.kt index b347e2a8a3..fb23f72557 100644 --- a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/AndroidLocationActions.kt +++ b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/AndroidLocationActions.kt @@ -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)) + } +} diff --git a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/LocationActions.kt b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/LocationActions.kt index c9e982f15d..7e38bd65fa 100644 --- a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/LocationActions.kt +++ b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/LocationActions.kt @@ -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?) } diff --git a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenter.kt b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenter.kt index 6bcd9d45db..42d57ecf7d 100644 --- a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenter.kt +++ b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenter.kt @@ -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) - } } diff --git a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/AndroidLocationActionsTest.kt b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/AndroidLocationActionsTest.kt new file mode 100644 index 0000000000..14dd983f34 --- /dev/null +++ b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/AndroidLocationActionsTest.kt @@ -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) + } +} diff --git a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/FakeLocationActions.kt b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/FakeLocationActions.kt index a1806746e8..411863f725 100644 --- a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/FakeLocationActions.kt +++ b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/FakeLocationActions.kt @@ -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 } diff --git a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenterTest.kt b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenterTest.kt index b0448afcfe..5ff323f463 100644 --- a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenterTest.kt +++ b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenterTest.kt @@ -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) }