Address PR review comments

This commit is contained in:
ganfra
2026-07-09 17:22:15 +02:00
parent f25ca5b963
commit 5f4bb30b18
26 changed files with 112 additions and 134 deletions
@@ -8,8 +8,8 @@
package io.element.android.features.login.impl.changeserver
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialog
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
data class ChangeServerState(
val changeServerAction: AsyncData<Unit>,
@@ -10,8 +10,8 @@ package io.element.android.features.login.impl.changeserver
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.features.login.impl.error.ChangeServerError
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialog
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerState> {
override val values: Sequence<ChangeServerState>
@@ -19,7 +19,6 @@ import androidx.compose.ui.tooling.preview.PreviewParameter
import io.element.android.features.login.impl.R
import io.element.android.features.login.impl.dialogs.SlidingSyncNotSupportedDialog
import io.element.android.features.login.impl.error.ChangeServerError
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.libraries.androidutils.system.openGooglePlay
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.designsystem.components.ProgressDialog
@@ -28,6 +27,7 @@ import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog
import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.designsystem.theme.LocalBuildMeta
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.libraries.ui.strings.CommonStrings
@Composable
@@ -1,44 +0,0 @@
/*
* Copyright (c) 2026 Element Creations Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.features.login.impl.localnetwork
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog
import io.element.android.libraries.ui.strings.CommonStrings
@Composable
fun LocalNetworkPermissionDialogView(
dialog: LocalNetworkPermissionDialog,
onSubmit: () -> Unit,
onDismiss: () -> Unit,
modifier: Modifier = Modifier,
) {
when (dialog) {
LocalNetworkPermissionDialog.None -> Unit
LocalNetworkPermissionDialog.Rationale -> ConfirmationDialog(
title = stringResource(CommonStrings.screen_local_network_opt_in_title),
content = stringResource(CommonStrings.screen_local_network_opt_in_subtitle),
submitText = stringResource(CommonStrings.dialog_allow_access),
cancelText = stringResource(CommonStrings.action_not_now),
onSubmitClick = onSubmit,
onDismiss = onDismiss,
modifier = modifier,
)
LocalNetworkPermissionDialog.Settings -> ConfirmationDialog(
title = stringResource(CommonStrings.screen_local_network_opt_in_title),
content = stringResource(CommonStrings.screen_local_network_opt_in_subtitle),
submitText = stringResource(CommonStrings.action_open_settings),
cancelText = stringResource(CommonStrings.action_not_now),
onSubmitClick = onSubmit,
onDismiss = onDismiss,
modifier = modifier,
)
}
}
@@ -17,9 +17,10 @@ import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.runtime.setValue
import dev.zacsweers.metro.Inject
import io.element.android.libraries.permissions.api.LocalNetworkPermissionAdvisor
import io.element.android.libraries.permissions.api.PermissionsEvent
import io.element.android.libraries.permissions.api.PermissionsPresenter
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionAdvisor
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
import kotlinx.coroutines.launch
@Inject
@@ -39,13 +40,13 @@ class LocalNetworkPermissionGate(
val permissionsState = permissionsPresenter.present()
var pendingSubmit by remember { mutableStateOf<T?>(null) }
val urlOf by rememberUpdatedState(urlOf)
val onProceed by rememberUpdatedState(onProceed)
val latestUrlOf by rememberUpdatedState(urlOf)
val latestOnProceed by rememberUpdatedState(onProceed)
LaunchedEffect(permissionsState.permissionGranted, pendingSubmit) {
val pending = pendingSubmit
if (pending != null && permissionsState.permissionGranted) {
coroutineScope.launch { onProceed(pending) }
coroutineScope.launch { latestOnProceed(pending) }
pendingSubmit = null
}
}
@@ -61,10 +62,10 @@ class LocalNetworkPermissionGate(
fun submit(value: T) {
coroutineScope.launch {
if (advisor.shouldRequestPermissionFor(urlOf(value))) {
if (advisor.shouldRequestPermissionFor(latestUrlOf(value))) {
pendingSubmit = value
} else {
onProceed(value)
latestOnProceed(value)
}
}
}
@@ -7,21 +7,11 @@
package io.element.android.features.login.impl.localnetwork
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
data class LocalNetworkPermissionGateState<T>(
val dialog: LocalNetworkPermissionDialog,
val submit: (T) -> Unit,
val requestPermission: () -> Unit,
val abort: () -> Unit,
)
/**
* Which rationale dialog (if any) should be rendered on top of the caller's UI.
*
* [Rationale] is shown before the runtime prompt so the user knows why the permission is needed.
* [Settings] is shown when the OS says a rationale can no longer be shown (permanently denied).
*/
enum class LocalNetworkPermissionDialog {
None,
Rationale,
Settings,
}
@@ -15,7 +15,11 @@ import androidx.compose.runtime.remember
import dev.zacsweers.metro.Inject
import io.element.android.features.login.impl.error.ChangeServerError
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionGate
import io.element.android.features.login.impl.screens.chooseaccountprovider.ChooseAccountProviderPresenter
import io.element.android.features.login.impl.screens.classic.loginwithclassic.LoginWithClassicPresenter
import io.element.android.features.login.impl.screens.confirmaccountprovider.ConfirmAccountProviderPresenter
import io.element.android.features.login.impl.screens.createaccount.AccountCreationNotSupported
import io.element.android.features.login.impl.screens.onboarding.OnBoardingPresenter
import io.element.android.features.login.impl.web.WebClientUrlForAuthenticationRetriever
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.architecture.Presenter
@@ -25,6 +29,12 @@ import io.element.android.libraries.matrix.api.auth.OAuthPrompt
import io.element.android.libraries.oauth.api.OAuthAction
import io.element.android.libraries.oauth.api.OAuthActionFlow
/**
* Presenter responsible for managing the login flow, including handling OAuth actions and
* submitting login requests.
* It's a helper to avoid code duplication. It is used by [OnBoardingPresenter], [ConfirmAccountProviderPresenter],
* [ChooseAccountProviderPresenter] and [LoginWithClassicPresenter].
*/
@Inject
class LoginModePresenter(
private val oAuthActionFlow: OAuthActionFlow,
@@ -7,9 +7,9 @@
package io.element.android.features.login.impl.login
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialog
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.matrix.api.auth.OAuthDetails
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
data class LoginModeState(
val loginMode: AsyncData<LoginMode>,
@@ -7,8 +7,8 @@
package io.element.android.features.login.impl.login
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialog
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
fun aLoginModeState(
loginMode: AsyncData<LoginMode> = AsyncData.Uninitialized,
@@ -14,7 +14,7 @@ import io.element.android.libraries.core.coroutine.parallelMap
import io.element.android.libraries.core.uri.ensureProtocol
import io.element.android.libraries.core.uri.isValidUrl
import io.element.android.libraries.matrix.api.auth.HomeServerLoginCompatibilityChecker
import io.element.android.libraries.permissions.api.LocalNetworkPermissionAdvisor
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionAdvisor
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flow
@@ -44,21 +44,14 @@ class HomeserverResolver(
// Skip the compatibility probe if we'd need ACCESS_LOCAL_NETWORK first —
// otherwise the probe hangs on the TCP timeout for ~30s. Emit the URL as a
// candidate directly; the actual sign-in flow triggers the permission prompt.
if (localNetworkPermissionAdvisor.shouldRequestPermissionFor(url)) {
currentList.add(HomeserverData(homeserverUrl = url))
withContext(flowContext) {
emit(currentList.toList())
}
return@parallelMap
}
val isValid = homeServerLoginCompatibilityChecker.check(url)
.onFailure { Timber.w(it, "Failed to check compatibility with homeserver $url") }
.getOrNull()
?: return@parallelMap
val shouldRequestPermissionOrIsValid = localNetworkPermissionAdvisor.shouldRequestPermissionFor(url) ||
homeServerLoginCompatibilityChecker.check(url)
.onFailure { Timber.w(it, "Failed to check compatibility with homeserver $url") }
.getOrNull()
?: return@parallelMap
// Emit the list as soon as possible
if (isValid) {
if (shouldRequestPermissionOrIsValid) {
currentList.add(HomeserverData(homeserverUrl = url))
withContext(flowContext) {
emit(currentList.toList())
@@ -33,7 +33,6 @@ import androidx.compose.ui.unit.dp
import io.element.android.compound.tokens.generated.CompoundIcons
import io.element.android.features.login.impl.R
import io.element.android.features.login.impl.accountprovider.AccountProviderView
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.features.login.impl.login.LoginModeEvent
import io.element.android.features.login.impl.login.LoginModeView
import io.element.android.libraries.architecture.AsyncData
@@ -46,6 +45,7 @@ import io.element.android.libraries.designsystem.theme.components.Button
import io.element.android.libraries.designsystem.theme.components.Scaffold
import io.element.android.libraries.designsystem.theme.components.TopAppBar
import io.element.android.libraries.matrix.api.auth.OAuthDetails
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.libraries.ui.strings.CommonStrings
@Composable
@@ -35,7 +35,6 @@ import androidx.compose.ui.tooling.preview.PreviewParameter
import androidx.compose.ui.unit.dp
import io.element.android.compound.theme.ElementTheme
import io.element.android.features.login.impl.R
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.features.login.impl.login.LoginModeEvent
import io.element.android.features.login.impl.login.LoginModeView
import io.element.android.libraries.architecture.AsyncData
@@ -52,6 +51,7 @@ import io.element.android.libraries.designsystem.theme.components.Button
import io.element.android.libraries.designsystem.theme.components.OutlinedButton
import io.element.android.libraries.designsystem.theme.components.Text
import io.element.android.libraries.matrix.api.auth.OAuthDetails
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.libraries.testtags.TestTags
import io.element.android.libraries.testtags.testTag
import io.element.android.libraries.ui.strings.CommonStrings
@@ -20,7 +20,6 @@ import androidx.compose.ui.tooling.preview.PreviewParameter
import androidx.compose.ui.unit.dp
import io.element.android.compound.tokens.generated.CompoundIcons
import io.element.android.features.login.impl.R
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.features.login.impl.login.LoginModeEvent
import io.element.android.features.login.impl.login.LoginModeView
import io.element.android.libraries.architecture.AsyncData
@@ -33,6 +32,7 @@ import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.designsystem.theme.components.Button
import io.element.android.libraries.designsystem.theme.components.TextButton
import io.element.android.libraries.matrix.api.auth.OAuthDetails
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.libraries.testtags.TestTags
import io.element.android.libraries.testtags.testTag
import io.element.android.libraries.ui.strings.CommonStrings
@@ -33,7 +33,6 @@ import androidx.compose.ui.unit.dp
import io.element.android.compound.theme.ElementTheme
import io.element.android.compound.tokens.generated.CompoundIcons
import io.element.android.features.login.impl.R
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.features.login.impl.login.LoginModeEvent
import io.element.android.features.login.impl.login.LoginModeView
import io.element.android.libraries.architecture.AsyncData
@@ -52,6 +51,7 @@ import io.element.android.libraries.designsystem.theme.components.IconSource
import io.element.android.libraries.designsystem.theme.components.Text
import io.element.android.libraries.designsystem.theme.components.TextButton
import io.element.android.libraries.matrix.api.auth.OAuthDetails
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialogView
import io.element.android.libraries.testtags.TestTags
import io.element.android.libraries.testtags.testTag
import io.element.android.libraries.ui.strings.CommonStrings
@@ -15,7 +15,6 @@ import io.element.android.features.login.impl.accesscontrol.DefaultAccountProvid
import io.element.android.features.login.impl.accountprovider.AccountProvider
import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource
import io.element.android.features.login.impl.error.ChangeServerError
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionDialog
import io.element.android.features.login.impl.localnetwork.LocalNetworkPermissionGate
import io.element.android.features.wellknown.test.FakeWellknownRetriever
import io.element.android.features.wellknown.test.anElementWellKnown
@@ -25,6 +24,7 @@ import io.element.android.libraries.matrix.test.AN_EXCEPTION
import io.element.android.libraries.matrix.test.A_HOMESERVER_URL
import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService
import io.element.android.libraries.matrix.test.auth.aMatrixHomeServerDetails
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
import io.element.android.libraries.permissions.test.FakeLocalNetworkPermissionAdvisor
import io.element.android.libraries.permissions.test.FakePermissionsPresenter
import io.element.android.libraries.permissions.test.FakePermissionsPresenterFactory
@@ -34,6 +34,7 @@ import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationSer
import io.element.android.libraries.matrix.test.core.aBuildMeta
import io.element.android.libraries.oauth.api.OAuthActionFlow
import io.element.android.libraries.oauth.test.customtab.FakeOAuthActionFlow
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionAdvisor
import io.element.android.libraries.sessionstorage.api.SessionStore
import io.element.android.libraries.sessionstorage.test.InMemorySessionStore
import io.element.android.libraries.sessionstorage.test.aSessionData
@@ -316,7 +317,7 @@ fun createLoginModePresenter(
oAuthActionFlow: OAuthActionFlow = FakeOAuthActionFlow(),
authenticationService: MatrixAuthenticationService = FakeMatrixAuthenticationService(),
webClientUrlForAuthenticationRetriever: WebClientUrlForAuthenticationRetriever = FakeWebClientUrlForAuthenticationRetriever(),
localNetworkPermissionAdvisor: io.element.android.libraries.permissions.api.LocalNetworkPermissionAdvisor =
localNetworkPermissionAdvisor: LocalNetworkPermissionAdvisor =
io.element.android.libraries.permissions.test.FakeLocalNetworkPermissionAdvisor(),
permissionsPresenterFactory: io.element.android.libraries.permissions.api.PermissionsPresenter.Factory =
io.element.android.libraries.permissions.test.FakePermissionsPresenterFactory(),