mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-03 16:10:00 +00:00
Fix account provider dropped on first Continue and address review comments
This commit is contained in:
+14
-4
@@ -20,6 +20,7 @@ import androidx.compose.runtime.setValue
|
|||||||
import dev.zacsweers.metro.Assisted
|
import dev.zacsweers.metro.Assisted
|
||||||
import dev.zacsweers.metro.AssistedFactory
|
import dev.zacsweers.metro.AssistedFactory
|
||||||
import dev.zacsweers.metro.AssistedInject
|
import dev.zacsweers.metro.AssistedInject
|
||||||
|
import io.element.android.appconfig.AuthenticationConfig
|
||||||
import io.element.android.features.login.impl.accountprovider.AccountProvider
|
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.accountprovider.AccountProviderDataSource
|
||||||
import io.element.android.features.login.impl.changeserver.ChangeServerEvents
|
import io.element.android.features.login.impl.changeserver.ChangeServerEvents
|
||||||
@@ -28,6 +29,7 @@ import io.element.android.features.login.impl.login.LoginModeEvent
|
|||||||
import io.element.android.features.login.impl.login.LoginModeState
|
import io.element.android.features.login.impl.login.LoginModeState
|
||||||
import io.element.android.libraries.architecture.AsyncData
|
import io.element.android.libraries.architecture.AsyncData
|
||||||
import io.element.android.libraries.architecture.Presenter
|
import io.element.android.libraries.architecture.Presenter
|
||||||
|
import io.element.android.libraries.core.uri.ensureProtocol
|
||||||
import io.element.android.libraries.preferences.api.store.AppPreferencesStore
|
import io.element.android.libraries.preferences.api.store.AppPreferencesStore
|
||||||
|
|
||||||
@AssistedInject
|
@AssistedInject
|
||||||
@@ -58,12 +60,18 @@ class ConfirmAccountProviderPresenter(
|
|||||||
var userInput by rememberSaveable { mutableStateOf<String?>(null) }
|
var userInput by rememberSaveable { mutableStateOf<String?>(null) }
|
||||||
val accountProviderInput = userInput ?: accountProvider.url
|
val accountProviderInput = userInput ?: accountProvider.url
|
||||||
|
|
||||||
// Offer the most recent previously-used account provider that the current input is a prefix of.
|
// Offer an account provider that the current input is a prefix of: the most recent previously-used
|
||||||
|
// one first, then matrix.org. matrix.org is always available for autocomplete even before any
|
||||||
|
// sign-in, offered both with and without the https:// scheme so it completes whether the user starts
|
||||||
|
// typing "matrix.org" or "https://matrix.org".
|
||||||
val accountProviderSuggestion = remember(accountProviderInput, homeserverHistory) {
|
val accountProviderSuggestion = remember(accountProviderInput, homeserverHistory) {
|
||||||
val input = accountProviderInput.trim()
|
val input = accountProviderInput.trim()
|
||||||
|
val candidates = homeserverHistory +
|
||||||
|
AuthenticationConfig.MATRIX_ORG_URL +
|
||||||
|
AuthenticationConfig.MATRIX_ORG_URL.removePrefix("https://")
|
||||||
input.takeIf { it.isNotEmpty() }
|
input.takeIf { it.isNotEmpty() }
|
||||||
?.let { prefix ->
|
?.let { prefix ->
|
||||||
homeserverHistory.firstOrNull { it.length > prefix.length && it.startsWith(prefix, ignoreCase = true) }
|
candidates.firstOrNull { it.length > prefix.length && it.startsWith(prefix, ignoreCase = true) }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -94,9 +102,11 @@ class ConfirmAccountProviderPresenter(
|
|||||||
// Validate (and persist) the chosen account provider before proceeding. This also enforces the
|
// Validate (and persist) the chosen account provider before proceeding. This also enforces the
|
||||||
// account-provider access control, which the login submit does not run on its own.
|
// account-provider access control, which the login submit does not run on its own.
|
||||||
is ConfirmAccountProviderEvents.Continue -> {
|
is ConfirmAccountProviderEvents.Continue -> {
|
||||||
submittedAccountProvider = event.accountProvider
|
// Default the scheme to https:// so entering a bare host (e.g. "matrix.org") works.
|
||||||
|
val accountProviderUrl = event.accountProvider.trim().ensureProtocol()
|
||||||
|
submittedAccountProvider = accountProviderUrl
|
||||||
changeServerState.eventSink(
|
changeServerState.eventSink(
|
||||||
ChangeServerEvents.ChangeServer(AccountProvider(url = event.accountProvider.trim()))
|
ChangeServerEvents.ChangeServer(AccountProvider(url = accountProviderUrl))
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
ConfirmAccountProviderEvents.ClearError -> {
|
ConfirmAccountProviderEvents.ClearError -> {
|
||||||
|
|||||||
+14
-9
@@ -19,6 +19,7 @@ import androidx.compose.runtime.LaunchedEffect
|
|||||||
import androidx.compose.runtime.getValue
|
import androidx.compose.runtime.getValue
|
||||||
import androidx.compose.runtime.mutableStateOf
|
import androidx.compose.runtime.mutableStateOf
|
||||||
import androidx.compose.runtime.remember
|
import androidx.compose.runtime.remember
|
||||||
|
import androidx.compose.runtime.rememberUpdatedState
|
||||||
import androidx.compose.runtime.setValue
|
import androidx.compose.runtime.setValue
|
||||||
import androidx.compose.ui.Modifier
|
import androidx.compose.ui.Modifier
|
||||||
import androidx.compose.ui.focus.FocusRequester
|
import androidx.compose.ui.focus.FocusRequester
|
||||||
@@ -88,12 +89,16 @@ fun ConfirmAccountProviderView(
|
|||||||
?.substring(input.length)
|
?.substring(input.length)
|
||||||
.orEmpty()
|
.orEmpty()
|
||||||
|
|
||||||
|
// The Continue button lives in a footer slot that is not recomposed while the user types, so a direct
|
||||||
|
// capture of the field text (via ::submit) goes stale and can submit an empty/prefilled value. Reading
|
||||||
|
// the value through rememberUpdatedState keeps it current regardless of which composition scope reads it.
|
||||||
|
val accountProviderToSubmit by rememberUpdatedState(input + suggestionSuffix)
|
||||||
|
|
||||||
fun submit() {
|
fun submit() {
|
||||||
// Dismiss the keyboard and release focus while the account provider is being validated.
|
// Dismiss the keyboard and release focus while the account provider is being validated.
|
||||||
focusManager.clearFocus(force = true)
|
focusManager.clearFocus(force = true)
|
||||||
// Submit the exact field text (plus any accepted suggestion), read here so it can't lag behind
|
// Submit the exact field text (plus any accepted suggestion).
|
||||||
// fast input such as keyboard autofill.
|
eventSink(ConfirmAccountProviderEvents.Continue(accountProviderToSubmit))
|
||||||
eventSink(ConfirmAccountProviderEvents.Continue(input + suggestionSuffix))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Once a validation / login error has been dismissed, return focus and the keyboard to the field so
|
// Once a validation / login error has been dismissed, return focus and the keyboard to the field so
|
||||||
@@ -115,9 +120,9 @@ fun ConfirmAccountProviderView(
|
|||||||
iconStyle = BigIcon.Style.Default(CompoundIcons.UserProfileSolid()),
|
iconStyle = BigIcon.Style.Default(CompoundIcons.UserProfileSolid()),
|
||||||
title = stringResource(
|
title = stringResource(
|
||||||
id = if (state.isAccountCreation) {
|
id = if (state.isAccountCreation) {
|
||||||
CommonStrings.screen_select_server_title_register
|
CommonStrings.screen_change_server_title_register
|
||||||
} else {
|
} else {
|
||||||
CommonStrings.screen_select_server_title_login
|
CommonStrings.screen_change_server_title_login
|
||||||
}
|
}
|
||||||
),
|
),
|
||||||
subTitle = null,
|
subTitle = null,
|
||||||
@@ -153,13 +158,13 @@ fun ConfirmAccountProviderView(
|
|||||||
.focusRequester(focusRequester)
|
.focusRequester(focusRequester)
|
||||||
.onTabOrEnterKeyFocusNext(focusManager)
|
.onTabOrEnterKeyFocusNext(focusManager)
|
||||||
.testTag(TestTags.changeServerServer),
|
.testTag(TestTags.changeServerServer),
|
||||||
label = stringResource(id = CommonStrings.screen_select_server_textfield_header),
|
label = stringResource(id = CommonStrings.screen_change_server_textfield_header),
|
||||||
placeholder = stringResource(id = CommonStrings.screen_select_server_textfield_placeholder),
|
placeholder = stringResource(id = CommonStrings.screen_change_server_textfield_placeholder),
|
||||||
supportingText = stringResource(
|
supportingText = stringResource(
|
||||||
id = if (state.isAccountCreation) {
|
id = if (state.isAccountCreation) {
|
||||||
CommonStrings.screen_select_server_textfield_footer_register
|
CommonStrings.screen_change_server_textfield_footer_register
|
||||||
} else {
|
} else {
|
||||||
CommonStrings.screen_select_server_textfield_footer_login
|
CommonStrings.screen_change_server_textfield_footer_login
|
||||||
}
|
}
|
||||||
),
|
),
|
||||||
visualTransformation = ghostTransformation,
|
visualTransformation = ghostTransformation,
|
||||||
|
|||||||
+47
@@ -349,6 +349,53 @@ class ConfirmAccountProviderPresenterTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `present - offers matrix_org as an autocomplete suggestion even without any history`() = runTest {
|
||||||
|
val presenter = createConfirmAccountProviderPresenter(
|
||||||
|
appPreferencesStore = InMemoryAppPreferencesStore(homeserverHistory = emptyList()),
|
||||||
|
)
|
||||||
|
presenter.test {
|
||||||
|
val initialState = awaitItem()
|
||||||
|
initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("https://matr"))
|
||||||
|
val suggestionState = awaitState { it.accountProviderInput == "https://matr" }
|
||||||
|
assertThat(suggestionState.accountProviderSuggestion).isEqualTo(AuthenticationConfig.MATRIX_ORG_URL)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `present - offers matrix_org without the scheme as an autocomplete suggestion`() = runTest {
|
||||||
|
val presenter = createConfirmAccountProviderPresenter(
|
||||||
|
appPreferencesStore = InMemoryAppPreferencesStore(homeserverHistory = emptyList()),
|
||||||
|
)
|
||||||
|
presenter.test {
|
||||||
|
val initialState = awaitItem()
|
||||||
|
initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("matr"))
|
||||||
|
val suggestionState = awaitState { it.accountProviderInput == "matr" }
|
||||||
|
assertThat(suggestionState.accountProviderSuggestion).isEqualTo("matrix.org")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `present - continue prepends https to a bare account provider`() = runTest {
|
||||||
|
val submittedUrls = mutableListOf<String>()
|
||||||
|
val authenticationService = FakeMatrixAuthenticationService(
|
||||||
|
setHomeserverResult = { url ->
|
||||||
|
submittedUrls.add(url)
|
||||||
|
Result.success(aMatrixHomeServerDetails(supportsOAuthLogin = true))
|
||||||
|
},
|
||||||
|
)
|
||||||
|
val presenter = createConfirmAccountProviderPresenter(
|
||||||
|
matrixAuthenticationService = authenticationService,
|
||||||
|
)
|
||||||
|
presenter.test {
|
||||||
|
val initialState = awaitItem()
|
||||||
|
initialState.eventSink(ConfirmAccountProviderEvents.Continue("matrix.org"))
|
||||||
|
awaitLoginMode { it is AsyncData.Success }
|
||||||
|
assertThat(submittedUrls.first()).isEqualTo(AuthenticationConfig.MATRIX_ORG_URL)
|
||||||
|
cancelAndIgnoreRemainingEvents()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Awaits until the emitted state's login mode matches [predicate], skipping the intermediate
|
* Awaits until the emitted state's login mode matches [predicate], skipping the intermediate
|
||||||
* account-provider validation states, and returns that state.
|
* account-provider validation states, and returns that state.
|
||||||
|
|||||||
@@ -10,10 +10,4 @@
|
|||||||
<string name="a11y_state_collapsed">"Collapsed"</string>
|
<string name="a11y_state_collapsed">"Collapsed"</string>
|
||||||
<string name="a11y_active">Active</string>
|
<string name="a11y_active">Active</string>
|
||||||
<string name="a11y_inactive">Inactive</string>
|
<string name="a11y_inactive">Inactive</string>
|
||||||
<string name="screen_select_server_title_login">"Enter your account provider"</string>
|
|
||||||
<string name="screen_select_server_title_register">"Choose an account provider"</string>
|
|
||||||
<string name="screen_select_server_textfield_header">"Account provider"</string>
|
|
||||||
<string name="screen_select_server_textfield_placeholder">"example.com"</string>
|
|
||||||
<string name="screen_select_server_textfield_footer_login">"If your account is provided by your company or a community, then change the account provider (example: companyname.com)."</string>
|
|
||||||
<string name="screen_select_server_textfield_footer_register">"Create an account on matrix.org for free. If you want to create an account with a different provider, then change it accordingly (example: communityname.org)."</string>
|
|
||||||
</resources>
|
</resources>
|
||||||
|
|||||||
Reference in New Issue
Block a user