diff --git a/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/authenticator/TotpCodeEntryViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/authenticator/TotpCodeEntryViewModelTest.kt index f0a2bc7005..4d2699a312 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/authenticator/TotpCodeEntryViewModelTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/authenticator/TotpCodeEntryViewModelTest.kt @@ -137,9 +137,7 @@ class TotpCodeEntryViewModelTest { } private fun enterCode(viewModel: TotpCodeEntryViewModel, code: String) { - code.forEachIndexed { index, digit -> - viewModel.onEvent(TotpCodeEntryEvent.CodeEntryEvent(CodeEntryFieldEvents.DigitChanged(index, digit.toString()))) - } + viewModel.onEvent(TotpCodeEntryEvent.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged(code))) } private fun createViewModel() = TotpCodeEntryViewModel(repository = repository) diff --git a/feature/app-settings/src/main/java/org/signal/appsettings/totpcodeentry/TotpCodeEntryScreen.kt b/feature/app-settings/src/main/java/org/signal/appsettings/totpcodeentry/TotpCodeEntryScreen.kt index 5332db0129..6a82463f86 100644 --- a/feature/app-settings/src/main/java/org/signal/appsettings/totpcodeentry/TotpCodeEntryScreen.kt +++ b/feature/app-settings/src/main/java/org/signal/appsettings/totpcodeentry/TotpCodeEntryScreen.kt @@ -124,7 +124,7 @@ private fun TotpCodeEntryState.Error.message(): String? = when (this) { private fun TotpCodeEntryScreenPreview() { Previews.Preview { TotpCodeEntryScreen( - state = TotpCodeEntryState(codeEntry = CodeEntryFieldState(digits = listOf("1", "2", "3", "4", "5", "6"))), + state = TotpCodeEntryState(codeEntry = CodeEntryFieldState(code = "123456")), onEvent = {} ) } @@ -136,7 +136,7 @@ private fun TotpCodeEntryScreenErrorPreview() { Previews.Preview { TotpCodeEntryScreen( state = TotpCodeEntryState( - codeEntry = CodeEntryFieldState(digits = listOf("1", "2", "3", "4", "5", "6")), + codeEntry = CodeEntryFieldState(code = "123456"), error = TotpCodeEntryState.Error.IncorrectCode ), onEvent = {} diff --git a/feature/app-settings/src/test/java/org/signal/appsettings/totpcodeentry/TotpCodeEntryScreenTest.kt b/feature/app-settings/src/test/java/org/signal/appsettings/totpcodeentry/TotpCodeEntryScreenTest.kt index 04c705e69f..5d4436ab29 100644 --- a/feature/app-settings/src/test/java/org/signal/appsettings/totpcodeentry/TotpCodeEntryScreenTest.kt +++ b/feature/app-settings/src/test/java/org/signal/appsettings/totpcodeentry/TotpCodeEntryScreenTest.kt @@ -29,8 +29,8 @@ import org.signal.uicomponents.codeentryfield.CodeEntryFieldTestTags class TotpCodeEntryScreenTest { companion object { - private val FULL_CODE = CodeEntryFieldState(digits = listOf("1", "2", "3", "4", "5", "6")) - private val PARTIAL_CODE = CodeEntryFieldState(digits = listOf("1", "2", "3", "", "", "")) + private val FULL_CODE = CodeEntryFieldState(code = "123456") + private val PARTIAL_CODE = CodeEntryFieldState(code = "123") } @get:Rule @@ -60,10 +60,10 @@ class TotpCodeEntryScreenTest { fun whenITypeInTheCodeField_thenIExpectAForwardedCodeEntryEvent() { setContent(TotpCodeEntryState()) - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0)).performTextInput("1") + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.ROOT).performTextInput("1") composeTestRule.waitForIdle() - assertThat(events).contains(TotpCodeEntryEvent.CodeEntryEvent(CodeEntryFieldEvents.DigitChanged(0, "1"))) + assertThat(events).contains(TotpCodeEntryEvent.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("1"))) } @Test @@ -77,7 +77,7 @@ class TotpCodeEntryScreenTest { fun givenASubmissionInFlight_whenScreenDisplayed_thenTheCodeFieldIsDisabled() { setContent(TotpCodeEntryState(codeEntry = FULL_CODE, submitting = true)) - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0)).assertIsNotEnabled() + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.ROOT).assertIsNotEnabled() composeTestRule.onNodeWithTag(TotpCodeEntryTestTags.BUTTON_NEXT).assertIsNotEnabled() } diff --git a/feature/registration/src/main/java/org/signal/registration/screens/totpentry/TotpEntryScreen.kt b/feature/registration/src/main/java/org/signal/registration/screens/totpentry/TotpEntryScreen.kt index 98d3d09405..5e759ce162 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/totpentry/TotpEntryScreen.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/totpentry/TotpEntryScreen.kt @@ -234,10 +234,7 @@ private fun TotpEntryScreenPartiallyFilledPreview() { Previews.Preview { TotpEntryScreen( state = TotpEntryState( - codeEntry = CodeEntryFieldState( - digits = listOf("4", "1", "8", "3", "7", ""), - focusedDigitIndex = 5 - ) + codeEntry = CodeEntryFieldState(code = "41837") ), onEvent = {} ) diff --git a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreen.kt b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreen.kt index 368ffef361..47ef23a41c 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreen.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreen.kt @@ -6,7 +6,6 @@ package org.signal.registration.screens.verificationcode import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row @@ -20,8 +19,6 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.wrapContentWidth import androidx.compose.foundation.rememberScrollState -import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.foundation.verticalScroll import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.MaterialTheme @@ -30,23 +27,17 @@ import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.material3.TextButton -import androidx.compose.material3.TextField -import androidx.compose.material3.TextFieldDefaults import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.focus.FocusRequester -import androidx.compose.ui.focus.focusRequester -import androidx.compose.ui.input.key.Key -import androidx.compose.ui.input.key.key -import androidx.compose.ui.input.key.onKeyEvent +import androidx.compose.ui.autofill.ContentType +import androidx.compose.ui.autofill.contentType import androidx.compose.ui.platform.LocalResources import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import androidx.lifecycle.Lifecycle @@ -63,6 +54,7 @@ import org.signal.registration.screens.TwoPaneRegistrationScaffold import org.signal.registration.screens.attachDebugLogHelper import org.signal.registration.screens.shared.ContactSupportDialog import org.signal.registration.test.TestTags +import org.signal.uicomponents.codeentryfield.CodeEntryField import kotlin.time.Duration.Companion.seconds /** @@ -76,7 +68,6 @@ fun VerificationCodeScreen( onEvent: (VerificationCodeScreenEvents) -> Unit, modifier: Modifier = Modifier ) { - val focusRequesters = remember { List(VerificationCodeState.CODE_LENGTH) { FocusRequester() } } val snackbarHostState = remember { SnackbarHostState() } val resources = LocalResources.current @@ -89,15 +80,6 @@ fun VerificationCodeScreen( } } - LaunchedEffect(state.autoFillCode) { - val code = state.autoFillCode ?: return@LaunchedEffect - - if (code.length == VerificationCodeState.CODE_LENGTH && code.all { it.isDigit() } && !state.isSubmittingCode) { - onEvent(VerificationCodeScreenEvents.DigitChanged(0, code)) - } - onEvent(VerificationCodeScreenEvents.ConsumeAutoFillCode) - } - LaunchedEffect(state.snackbars) { val (message, dismissedEvent) = when { state.snackbars.incorrectVerificationCode -> resources.getString(R.string.VerificationCodeScreen__incorrect_code) to VerificationCodeScreenEvents.IncorrectVerificationCodeSnackbarDismissed @@ -114,10 +96,6 @@ fun VerificationCodeScreen( RequestCodeErrorDialogs(state.dialogs, onEvent) - LaunchedEffect(state.focusedDigitIndex) { - focusRequesters[state.focusedDigitIndex].requestFocus() - } - LifecycleEventEffect(Lifecycle.Event.ON_RESUME) { onEvent(VerificationCodeScreenEvents.Foregrounded) } @@ -145,7 +123,6 @@ fun VerificationCodeScreen( is RegistrationScaffold.Params.OnePane -> OnePaneLayout( params = layoutParams, innerPadding = innerPadding, - focusRequesters = focusRequesters, state = state, onEvent = onEvent ) @@ -153,7 +130,6 @@ fun VerificationCodeScreen( is RegistrationScaffold.Params.TwoPane -> TwoPaneLayout( params = layoutParams, innerPadding = innerPadding, - focusRequesters = focusRequesters, state = state, onEvent = onEvent ) @@ -209,7 +185,6 @@ private fun RequestCodeErrorDialogs(dialogs: VerificationCodeState.Dialogs, onEv private fun OnePaneLayout( params: RegistrationScaffold.Params.OnePane, innerPadding: PaddingValues, - focusRequesters: List, state: VerificationCodeState, onEvent: (VerificationCodeScreenEvents) -> Unit ) { @@ -232,11 +207,7 @@ private fun OnePaneLayout( Spacer(modifier = Modifier.height(32.dp)) - CodeField( - focusRequesters = focusRequesters, - state = state, - emitter = onEvent - ) + CodeField(state, onEvent) Spacer(modifier = Modifier.height(32.dp)) @@ -266,7 +237,6 @@ private fun OnePaneLayout( private fun TwoPaneLayout( params: RegistrationScaffold.Params.TwoPane, innerPadding: PaddingValues, - focusRequesters: List, state: VerificationCodeState, onEvent: (VerificationCodeScreenEvents) -> Unit ) { @@ -296,11 +266,7 @@ private fun TwoPaneLayout( .verticalScroll(secondPaneScrollState) .padding(paddingValues) ) { - CodeField( - focusRequesters = focusRequesters, - state = state, - emitter = onEvent - ) + CodeField(state, onEvent) Spacer(modifier = Modifier.height(32.dp)) @@ -343,77 +309,24 @@ private fun TroubleButton(onEvent: (VerificationCodeScreenEvents) -> Unit) { } @Composable -private fun CodeField( - focusRequesters: List, - state: VerificationCodeState, - emitter: (VerificationCodeScreenEvents) -> Unit -) { - val digits = state.digits +private fun CodeField(state: VerificationCodeState, onEvent: (VerificationCodeScreenEvents) -> Unit) { + Column(modifier = Modifier.fillMaxWidth()) { + CodeEntryField( + state = state.codeEntry, + onEvent = { onEvent(VerificationCodeScreenEvents.CodeEntryEvent(it)) }, + enabled = !state.isSubmittingCode, + digitSpacing = 4.dp, + separatorPadding = 8.dp, + modifier = Modifier.contentType(ContentType.SmsOtpCode) + ) - Box( - modifier = Modifier.fillMaxWidth(), - contentAlignment = Alignment.Center - ) { - Column(modifier = Modifier.align(Alignment.Center)) { - Row( + if (state.isSubmittingCode) { + Spacer(modifier = Modifier.height(16.dp)) + CircularProgressIndicator( modifier = Modifier - .fillMaxWidth() - .testTag(TestTags.VERIFICATION_CODE_INPUT), - horizontalArrangement = Arrangement.Center, - verticalAlignment = Alignment.CenterVertically - ) { - for (i in 0..2) { - DigitField( - value = digits[i], - onValueChange = { newValue -> emitter(VerificationCodeScreenEvents.DigitChanged(i, newValue)) }, - focusRequester = focusRequesters[i], - testTag = when (i) { - 0 -> TestTags.VERIFICATION_CODE_DIGIT_0 - 1 -> TestTags.VERIFICATION_CODE_DIGIT_1 - else -> TestTags.VERIFICATION_CODE_DIGIT_2 - }, - modifier = Modifier.weight(1f, fill = false), - enabled = !state.isSubmittingCode - ) - if (i < 2) { - Spacer(modifier = Modifier.width(4.dp)) - } - } - - Text( - text = "-", - style = MaterialTheme.typography.headlineMedium, - modifier = Modifier.padding(horizontal = 8.dp), - color = if (state.isSubmittingCode) MaterialTheme.colorScheme.onSurface.copy(alpha = 0.38f) else MaterialTheme.colorScheme.onSurface - ) - - for (i in 3..5) { - if (i > 3) { - Spacer(modifier = Modifier.width(4.dp)) - } - DigitField( - value = digits[i], - onValueChange = { newValue -> emitter(VerificationCodeScreenEvents.DigitChanged(i, newValue)) }, - focusRequester = focusRequesters[i], - testTag = when (i) { - 3 -> TestTags.VERIFICATION_CODE_DIGIT_3 - 4 -> TestTags.VERIFICATION_CODE_DIGIT_4 - else -> TestTags.VERIFICATION_CODE_DIGIT_5 - }, - modifier = Modifier.weight(1f, fill = false), - enabled = !state.isSubmittingCode - ) - } - } - - if (state.isSubmittingCode) { - Spacer(modifier = Modifier.height(16.dp)) - CircularProgressIndicator( - modifier = Modifier - .size(48.dp) - .align(Alignment.CenterHorizontally) - ) - } + .size(48.dp) + .align(Alignment.CenterHorizontally) + ) } } } @@ -505,47 +418,6 @@ private fun Description(state: VerificationCodeState, onEvent: (VerificationCode } } -@Composable -private fun DigitField( - value: String, - onValueChange: (String) -> Unit, - focusRequester: FocusRequester, - testTag: String, - modifier: Modifier = Modifier, - enabled: Boolean = true -) { - TextField( - value = value, - onValueChange = onValueChange, - modifier = modifier - .width(48.dp) - .focusRequester(focusRequester) - .testTag(testTag) - .onKeyEvent { keyEvent -> - if ((keyEvent.key == Key.Backspace || keyEvent.key == Key.Delete) && value.isEmpty()) { - onValueChange("") - true - } else { - false - } - }, - textStyle = MaterialTheme.typography.titleLarge.copy(textAlign = TextAlign.Center), - singleLine = true, - shape = RoundedCornerShape(topStart = 4.dp, topEnd = 4.dp), - keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), - enabled = enabled, - colors = TextFieldDefaults.colors( - focusedContainerColor = MaterialTheme.colorScheme.surfaceVariant, - unfocusedContainerColor = MaterialTheme.colorScheme.surfaceVariant, - disabledContainerColor = MaterialTheme.colorScheme.surfaceVariant, - focusedIndicatorColor = MaterialTheme.colorScheme.primary, - unfocusedIndicatorColor = MaterialTheme.colorScheme.outline, - disabledIndicatorColor = MaterialTheme.colorScheme.outline.copy(alpha = 0.38f), - disabledTextColor = MaterialTheme.colorScheme.onSurface.copy(alpha = 0.38f) - ) - ) -} - @AllDevicePreviews @Composable private fun VerificationCodeScreenPreview() { diff --git a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenEvents.kt b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenEvents.kt index cf256657bc..974ca80115 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenEvents.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenEvents.kt @@ -7,6 +7,8 @@ package org.signal.registration.screens.verificationcode import org.signal.core.util.censor import org.signal.registration.RegistrationFlowState +import org.signal.uicomponents.codeentryfield.CodeEntryFieldEvents +import org.signal.uicomponents.codeentryfield.CodeEntryFieldState sealed class VerificationCodeScreenEvents { /** The parent registration flow state changed and needs to be merged into this screen's state. */ @@ -16,15 +18,11 @@ sealed class VerificationCodeScreenEvents { override fun toString(): String = "CodeEntered(code=${code.censor()})" } - /** - * The raw [value] of the digit field at [index] changed. The view model interprets it: a single digit is recorded - * (submitting once the full code is present), an empty [value] is a backspace (deleting a digit and shifting the - * following ones left), and multi-character input (e.g. a pasted "123-456" or an auto-filled SMS code) populates - * every field at once and submits. - */ - data class DigitChanged(val index: Int, val value: String) : VerificationCodeScreenEvents() { - override fun toString(): String = "DigitChanged(index=$index)" - } + /** An event for the code entry field, forwarded to its presenter. */ + data class CodeEntryEvent(val event: CodeEntryFieldEvents) : VerificationCodeScreenEvents() + + /** The code entry field's presenter has a new state to mirror into this screen's state. */ + data class CodeEntryStateChanged(val codeEntryState: CodeEntryFieldState) : VerificationCodeScreenEvents() /** * A verification code was automatically retrieved from an incoming SMS via the Play Services SMS retriever. @@ -33,8 +31,6 @@ sealed class VerificationCodeScreenEvents { override fun toString(): String = "CodeAutoFilled(code=${code.censor()})" } - data object ConsumeAutoFillCode : VerificationCodeScreenEvents() - data object WrongNumber : VerificationCodeScreenEvents() data object ResendSms : VerificationCodeScreenEvents() diff --git a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeState.kt b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeState.kt index 95109f6c1e..5ddbc68731 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeState.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeState.kt @@ -7,6 +7,7 @@ package org.signal.registration.screens.verificationcode import org.signal.network.api.RegistrationApiV2.SessionMetadata import org.signal.network.api.RegistrationApiV2.VerificationCodeTransport +import org.signal.uicomponents.codeentryfield.CodeEntryFieldState import kotlin.time.Duration import kotlin.time.Duration.Companion.seconds @@ -16,34 +17,13 @@ data class VerificationCodeState( val isSubmittingCode: Boolean = false, val rateLimits: SmsAndCallRateLimits = SmsAndCallRateLimits(), val incorrectCodeAttempts: Int = 0, - val autoFillCode: String? = null, - val digits: List = List(CODE_LENGTH) { "" }, - val focusedDigitIndex: Int = 0, + val codeEntry: CodeEntryFieldState = CodeEntryFieldState(), val showContactSupportSheet: Boolean = false, val showContactSupportDialog: Boolean = false, val snackbars: Snackbars = Snackbars(), val dialogs: Dialogs = Dialogs() ) { - override fun toString(): String = "VerificationCodeState(sessionMetadata=$sessionMetadata, e164=$e164, isSubmittingCode=$isSubmittingCode, rateLimits=$rateLimits, incorrectCodeAttempts=$incorrectCodeAttempts, autoFillCode=${autoFillCode?.let { "present" }}, digitsEntered=${digits.count { it.isNotEmpty() }}, focusedDigitIndex=$focusedDigitIndex, showContactSupportSheet=$showContactSupportSheet, showContactSupportDialog=$showContactSupportDialog, snackbars=$snackbars, dialogs=$dialogs)" - - /** - * The full code as currently entered. Only meaningful when [isComplete] is true. - */ - val code: String get() = digits.joinToString("") - - /** - * True once every digit field has a value. - */ - val isComplete: Boolean get() = digits.size == CODE_LENGTH && digits.all { it.isNotEmpty() } - - companion object { - const val CODE_LENGTH = 6 - - /** - * A fully empty set of digits, used to reset the fields. - */ - fun emptyDigits(): List = List(CODE_LENGTH) { "" } - } + override fun toString(): String = "VerificationCodeState(sessionMetadata=$sessionMetadata, e164=$e164, isSubmittingCode=$isSubmittingCode, rateLimits=$rateLimits, incorrectCodeAttempts=$incorrectCodeAttempts, codeEntry=$codeEntry, showContactSupportSheet=$showContactSupportSheet, showContactSupportDialog=$showContactSupportDialog, snackbars=$snackbars, dialogs=$dialogs)" /** Transient errors from submitting a code or registering, shown as snackbars. Cleared once shown and dismissed. */ data class Snackbars( diff --git a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt index 47ef627c06..0b1509be2a 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt @@ -40,6 +40,10 @@ import org.signal.registration.RegistrationRepository import org.signal.registration.RegistrationRoute import org.signal.registration.screens.util.navigateBack import org.signal.registration.screens.util.navigateTo +import org.signal.uicomponents.codeentryfield.CodeEntryFieldAction +import org.signal.uicomponents.codeentryfield.CodeEntryFieldEvents +import org.signal.uicomponents.codeentryfield.CodeEntryFieldPresenter +import org.signal.uicomponents.codeentryfield.CodeEntryFieldState import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.minutes @@ -56,8 +60,6 @@ class VerificationCodeViewModel( companion object { private val TAG = Log.tag(VerificationCodeViewModel::class) - private const val CODE_LENGTH = VerificationCodeState.CODE_LENGTH - /** * How old the in-progress registration data can be before we assume the verification session has expired and * restart the flow. Checked whenever the screen is foregrounded. @@ -101,6 +103,8 @@ class VerificationCodeViewModel( private val _state = MutableStateFlow(VerificationCodeState()) val state: StateFlow = _state.asStateFlow() + private val codeEntryPresenter = CodeEntryFieldPresenter(viewModelScope) + init { _state .onEach { Log.d(TAG, "[State] $it") } @@ -110,6 +114,20 @@ class VerificationCodeViewModel( .onEach { onEvent(VerificationCodeScreenEvents.ParentStateChanged(it)) } .launchIn(viewModelScope) + codeEntryPresenter + .state + .onEach { onEvent(VerificationCodeScreenEvents.CodeEntryStateChanged(it)) } + .launchIn(viewModelScope) + + codeEntryPresenter + .actions + .onEach { action -> + when (action) { + is CodeEntryFieldAction.CodeEntered -> onEvent(VerificationCodeScreenEvents.CodeEntered(action.code)) + } + } + .launchIn(viewModelScope) + viewModelScope.launch { smsCodeEvents.collect { code -> onEvent(VerificationCodeScreenEvents.CodeAutoFilled(code)) @@ -125,10 +143,10 @@ class VerificationCodeViewModel( suspend fun applyEvent(state: VerificationCodeState, event: VerificationCodeScreenEvents, stateEmitter: (VerificationCodeState) -> Unit) { val result = when (event) { is VerificationCodeScreenEvents.ParentStateChanged -> applyParentState(state, event.parentState) - is VerificationCodeScreenEvents.CodeEntered -> submitCode(state, event.code, stateEmitter) - is VerificationCodeScreenEvents.DigitChanged -> applyDigitChanged(state, event.index, event.value, stateEmitter) - is VerificationCodeScreenEvents.CodeAutoFilled -> state.copy(autoFillCode = event.code) - is VerificationCodeScreenEvents.ConsumeAutoFillCode -> state.copy(autoFillCode = null) + is VerificationCodeScreenEvents.CodeEntered -> applyCodeEntered(state, event.code, stateEmitter) + is VerificationCodeScreenEvents.CodeEntryEvent -> state.also { codeEntryPresenter.onEvent(event.event) } + is VerificationCodeScreenEvents.CodeEntryStateChanged -> state.copy(codeEntry = event.codeEntryState) + is VerificationCodeScreenEvents.CodeAutoFilled -> applyCodeAutoFilled(state, event.code) is VerificationCodeScreenEvents.WrongNumber -> state.also { parentEventEmitter.navigateTo(RegistrationRoute.PhoneNumberEntry) } is VerificationCodeScreenEvents.ResendSms -> applyResendCode(state, VerificationCodeTransport.SMS) is VerificationCodeScreenEvents.CallMe -> applyResendCode(state, VerificationCodeTransport.VOICE) @@ -203,94 +221,32 @@ class VerificationCodeViewModel( ) } - /** - * Interprets the raw [value] reported by the digit field at [index] and updates the digits and focus accordingly: - * - * - an empty [value] is a backspace, deleting a digit and moving focus back - * - a single digit is recorded and focus advances, submitting once the full code is present - * - multi-character input (e.g. a pasted "123-456" or an auto-filled SMS code) populates every field at once and - * submits, all in this single reducer pass - */ - private suspend fun applyDigitChanged( - state: VerificationCodeState, - index: Int, - value: String, - stateEmitter: (VerificationCodeState) -> Unit - ): VerificationCodeState { - check(index in state.digits.indices) { "[DigitChanged] Out of bounds index $index." } - - if (value.isEmpty()) { - return deleteDigit(state, index) - } - - val currentValue = state.digits[index] - val remainder = if (currentValue.isNotEmpty()) value.replaceFirst(currentValue, "") else value - val addedDigits = remainder.filter { it.isDigit() } - - return when { - addedDigits.isEmpty() -> state - - addedDigits.length == 1 -> { - val updated = state.copy( - digits = state.digits.toMutableList().also { it[index] = addedDigits }, - focusedDigitIndex = (index + 1).coerceAtMost(CODE_LENGTH - 1) - ) - - if (updated.isComplete && !updated.isSubmittingCode) { - submitCode(updated, updated.code, stateEmitter) - } else { - updated - } - } - - else -> applyFullCode(state, addedDigits, stateEmitter) - } - } - - /** - * Populates every digit field from a full pasted or auto-filled [code] in a single reducer pass and submits it. - * Multi-character input that isn't a complete code is ignored. - */ - private suspend fun applyFullCode( - state: VerificationCodeState, - code: String, - stateEmitter: (VerificationCodeState) -> Unit - ): VerificationCodeState { - if (code.length != CODE_LENGTH) { - Log.w(TAG, "[DigitChanged] Ignoring multi-character input containing ${code.length} digits.") + private suspend fun applyCodeEntered(state: VerificationCodeState, code: String, stateEmitter: (VerificationCodeState) -> Unit): VerificationCodeState { + if (state.isSubmittingCode) { + Log.w(TAG, "[CodeEntered] Already submitting a code. Ignoring.") return state } - val updated = state.copy( - digits = code.map { it.toString() }, - focusedDigitIndex = CODE_LENGTH - 1 - ) - - return if (!updated.isSubmittingCode) { - submitCode(updated, updated.code, stateEmitter) - } else { - updated - } + return submitCode(state, code, stateEmitter) } /** - * Deletes the digit at [index] (or the previous one, if [index] is already empty), shifts any following digits left - * to fill the gap, and moves focus back. + * Places a [code] retrieved from an incoming SMS into the code field, which then reports it back as entered and + * submits it. */ - private fun deleteDigit(state: VerificationCodeState, index: Int): VerificationCodeState { - val deleteAt = if (state.digits[index].isNotEmpty()) index else index - 1 - if (deleteAt < 0) { + private fun applyCodeAutoFilled(state: VerificationCodeState, code: String): VerificationCodeState { + if (state.isSubmittingCode) { + Log.w(TAG, "[CodeAutoFilled] Already submitting a code. Ignoring the auto-filled one.") return state } - val newDigits = state.digits.toMutableList().apply { - for (j in deleteAt until CODE_LENGTH - 1) { - this[j] = this[j + 1] - } - this[CODE_LENGTH - 1] = "" + if (code.length != CodeEntryFieldState.CODE_LENGTH || !code.all { it.isDigit() }) { + Log.w(TAG, "[CodeAutoFilled] Ignoring an auto-filled code that isn't ${CodeEntryFieldState.CODE_LENGTH} digits. Length: ${code.length}") + return state } - return state.copy(digits = newDigits, focusedDigitIndex = (index - 1).coerceAtLeast(0)) + codeEntryPresenter.onEvent(CodeEntryFieldEvents.SetCode(code)) + return state } /** @@ -298,10 +254,10 @@ class VerificationCodeViewModel( */ private suspend fun submitCode(state: VerificationCodeState, code: String, stateEmitter: (VerificationCodeState) -> Unit): VerificationCodeState { stateEmitter(state.copy(isSubmittingCode = true)) - return applyCodeEntered(state, code).copy(isSubmittingCode = false) + return performSubmission(state, code).copy(isSubmittingCode = false) } - private suspend fun applyCodeEntered(inputState: VerificationCodeState, code: String): VerificationCodeState { + private suspend fun performSubmission(inputState: VerificationCodeState, code: String): VerificationCodeState { var state = inputState var sessionMetadata = state.sessionMetadata ?: return state.also { parentEventEmitter(RegistrationFlowEvent.ResetState) @@ -320,7 +276,8 @@ class VerificationCodeViewModel( is SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode -> { Log.w(TAG, "[SubmitCode] Invalid sessionId or verification code entered. This is distinct from an *incorrect* verification code. Body: ${error.message}") val newAttempts = state.incorrectCodeAttempts + 1 - return state.copy(snackbars = state.snackbars.copy(incorrectVerificationCode = true), incorrectCodeAttempts = newAttempts, digits = VerificationCodeState.emptyDigits(), focusedDigitIndex = 0) + codeEntryPresenter.onEvent(CodeEntryFieldEvents.Clear) + return state.copy(snackbars = state.snackbars.copy(incorrectVerificationCode = true), incorrectCodeAttempts = newAttempts) } is SubmitVerificationCodeError.SessionNotFound -> { Log.w(TAG, "[SubmitCode] Session not found: ${error.message}. Navigating back to phone number entry.") @@ -358,7 +315,8 @@ class VerificationCodeViewModel( if (!sessionMetadata.verified) { Log.w(TAG, "[SubmitCode] Verification code was incorrect.") val newAttempts = state.incorrectCodeAttempts + 1 - return state.copy(snackbars = state.snackbars.copy(incorrectVerificationCode = true), incorrectCodeAttempts = newAttempts, digits = VerificationCodeState.emptyDigits(), focusedDigitIndex = 0) + codeEntryPresenter.onEvent(CodeEntryFieldEvents.Clear) + return state.copy(snackbars = state.snackbars.copy(incorrectVerificationCode = true), incorrectCodeAttempts = newAttempts) } parentEventEmitter(RegistrationFlowEvent.VerificationCodeAccepted(code)) diff --git a/feature/registration/src/main/java/org/signal/registration/test/TestTags.kt b/feature/registration/src/main/java/org/signal/registration/test/TestTags.kt index 12ee574caa..9456eabe1a 100644 --- a/feature/registration/src/main/java/org/signal/registration/test/TestTags.kt +++ b/feature/registration/src/main/java/org/signal/registration/test/TestTags.kt @@ -93,13 +93,6 @@ object TestTags { const val ADD_USERNAME_NEXT_BUTTON = "add_username_next_button" // Verification Code Screen - const val VERIFICATION_CODE_INPUT = "verification_code_input" - const val VERIFICATION_CODE_DIGIT_0 = "verification_code_digit_0" - const val VERIFICATION_CODE_DIGIT_1 = "verification_code_digit_1" - const val VERIFICATION_CODE_DIGIT_2 = "verification_code_digit_2" - const val VERIFICATION_CODE_DIGIT_3 = "verification_code_digit_3" - const val VERIFICATION_CODE_DIGIT_4 = "verification_code_digit_4" - const val VERIFICATION_CODE_DIGIT_5 = "verification_code_digit_5" const val VERIFICATION_CODE_WRONG_NUMBER_BUTTON = "verification_code_wrong_number_button" const val VERIFICATION_CODE_RESEND_SMS_BUTTON = "verification_code_resend_sms_button" const val VERIFICATION_CODE_CALL_ME_BUTTON = "verification_code_call_me_button" diff --git a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt index b21e799a17..d42f79ee30 100644 --- a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt @@ -2120,8 +2120,8 @@ class RegistrationEndToEndTest { enterSignalLogin(login) // The authenticator app is the only second factor available, so the user lands straight on code entry - waitForTag(CodeEntryFieldTestTags.digit(0)) - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0)).performTextInput(totp) + waitForTag(CodeEntryFieldTestTags.ROOT) + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.ROOT).performTextInput(totp) waitFor("registration to complete") { registrationComplete } @@ -2460,8 +2460,8 @@ class RegistrationEndToEndTest { /** From the verification code screen: enters all six digits of [code], which submits automatically. */ private fun submitVerificationCode(code: String) { - waitForTag(TestTags.VERIFICATION_CODE_DIGIT_0) - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_0).performTextInput(code) + waitForTag(CodeEntryFieldTestTags.ROOT) + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.ROOT).performTextInput(code) } /** From the PIN creation screen: enters [pin], then re-enters it on the confirmation step. */ diff --git a/feature/registration/src/test/java/org/signal/registration/screens/totpentry/TotpEntryScreenTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/totpentry/TotpEntryScreenTest.kt index c03324a308..1a40bebaf7 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/totpentry/TotpEntryScreenTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/totpentry/TotpEntryScreenTest.kt @@ -57,10 +57,10 @@ class TotpEntryScreenTest { fun `entering a digit forwards a code field event`() { setContent(TotpEntryState()) - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0)).performTextInput("4") + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.ROOT).performTextInput("4") composeTestRule.waitForIdle() - assertThat(events).contains(TotpEntryScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.DigitChanged(0, "4"))) + assertThat(events).contains(TotpEntryScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("4"))) } @Test diff --git a/feature/registration/src/test/java/org/signal/registration/screens/totpentry/TotpEntryViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/totpentry/TotpEntryViewModelTest.kt index ec98cb8e46..60c48d9ac6 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/totpentry/TotpEntryViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/totpentry/TotpEntryViewModelTest.kt @@ -50,23 +50,20 @@ class TotpEntryViewModelTest { fun `code field state is mirrored into screen state`() = runTest(testDispatcher) { val viewModel = createViewModel() - viewModel.onEvent(TotpEntryScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.DigitChanged(0, "4"))) + viewModel.onEvent(TotpEntryScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("4"))) - assertThat(viewModel.state.value.codeEntry.digits[0]).isEqualTo("4") - assertThat(viewModel.state.value.codeEntry.focusedDigitIndex).isEqualTo(1) + assertThat(viewModel.state.value.codeEntry.code).isEqualTo("4") } @Test fun `a completed code is emitted and pops back to the login screen`() = runTest(testDispatcher) { val viewModel = createViewModel() - "41837".forEachIndexed { index, digit -> - viewModel.onEvent(TotpEntryScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.DigitChanged(index, digit.toString()))) - } + viewModel.onEvent(TotpEntryScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("41837"))) assertThat(sentCode()).isNull() assertThat(emittedParentEvents).isEmpty() - viewModel.onEvent(TotpEntryScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.DigitChanged(5, "2"))) + viewModel.onEvent(TotpEntryScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("418372"))) assertThat(sentCode()).isEqualTo("418372") assertThat(emittedParentEvents).containsExactly(RegistrationFlowEvent.NavigateBackToScreen(RegistrationRoute.SignalLoginCredentialEntry())) diff --git a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenTest.kt index 5b4c819533..dc70b25f30 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenTest.kt @@ -7,6 +7,7 @@ package org.signal.registration.screens.verificationcode import android.app.Application import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.assertTextEquals import androidx.compose.ui.test.junit4.createComposeRule import androidx.compose.ui.test.onNodeWithTag @@ -14,6 +15,8 @@ import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performTextInput import androidx.test.core.app.ApplicationProvider +import assertk.assertThat +import assertk.assertions.contains import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -22,6 +25,9 @@ import org.robolectric.annotation.Config import org.signal.core.ui.CoreUiDependenciesRule import org.signal.core.ui.compose.theme.SignalTheme import org.signal.registration.test.TestTags +import org.signal.uicomponents.codeentryfield.CodeEntryFieldEvents +import org.signal.uicomponents.codeentryfield.CodeEntryFieldState +import org.signal.uicomponents.codeentryfield.CodeEntryFieldTestTags /** * Tests for VerificationCodeScreen that validate event emissions and UI behavior. @@ -54,7 +60,7 @@ class VerificationCodeScreenTest { } @Test - fun `screen displays all six digit fields`() { + fun `screen displays the code field`() { // Given composeTestRule.setContent { SignalTheme { @@ -66,12 +72,7 @@ class VerificationCodeScreenTest { } // Then - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_0).assertIsDisplayed() - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_1).assertIsDisplayed() - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_2).assertIsDisplayed() - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_3).assertIsDisplayed() - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_4).assertIsDisplayed() - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_5).assertIsDisplayed() + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.ROOT).assertIsDisplayed() } @Test @@ -144,7 +145,7 @@ class VerificationCodeScreenTest { } @Test - fun `entering a digit emits DigitChanged for that field`() { + fun `typing forwards code field events`() { // Given val emittedEvents = mutableListOf() @@ -158,87 +159,52 @@ class VerificationCodeScreenTest { } // When - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_0).performTextInput("1") - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_1).performTextInput("2") + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.ROOT).performTextInput("1") composeTestRule.waitForIdle() // Then - val digitChanges = emittedEvents.filterIsInstance() - assert(digitChanges.contains(VerificationCodeScreenEvents.DigitChanged(0, "1"))) { - "Expected DigitChanged(0, 1) but got $digitChanges" - } - assert(digitChanges.contains(VerificationCodeScreenEvents.DigitChanged(1, "2"))) { - "Expected DigitChanged(1, 2) but got $digitChanges" - } + assertThat(emittedEvents).contains(VerificationCodeScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("1"))) } @Test - fun `screen renders the digits from state`() { + fun `the code field is disabled while submitting`() { // Given composeTestRule.setContent { SignalTheme { VerificationCodeScreen( - state = VerificationCodeState(digits = listOf("1", "2", "3", "4", "5", "6")), + state = VerificationCodeState(isSubmittingCode = true), onEvent = {} ) } } // Then - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_0).assertTextEquals("1") - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_5).assertTextEquals("6") + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.ROOT).assertIsNotEnabled() } @Test - fun `pasting into a field emits DigitChanged with the raw text`() { + fun `an auto-filled code is placed in the code field`() { // Given val emittedEvents = mutableListOf() composeTestRule.setContent { SignalTheme { VerificationCodeScreen( - state = VerificationCodeState(), + state = VerificationCodeState(codeEntry = CodeEntryFieldState(code = "123456", pendingOverwrite = "123456")), onEvent = { emittedEvents.add(it) } ) } } - // When - paste the entire code, including the hyphen, into the first field - composeTestRule.onNodeWithTag(TestTags.VERIFICATION_CODE_DIGIT_0).performTextInput("123-456") - + // When + composeTestRule.waitUntil(timeoutMillis = 5_000) { + emittedEvents.contains(VerificationCodeScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.OverwriteApplied)) + } composeTestRule.waitForIdle() // Then - val digitChanges = emittedEvents.filterIsInstance() - assert(digitChanges.contains(VerificationCodeScreenEvents.DigitChanged(0, "123-456"))) { - "Expected DigitChanged(0, 123-456) but got $digitChanges" - } - } - - @Test - fun `autoFillCode emits a single DigitChanged with the full code`() { - // Given - val emittedEvents = mutableListOf() - - composeTestRule.setContent { - SignalTheme { - VerificationCodeScreen( - state = VerificationCodeState(autoFillCode = "123456"), - onEvent = { emittedEvents.add(it) } - ) - } - } - - // When - the auto-fill effect populates the fields - composeTestRule.waitUntil(timeoutMillis = 5_000) { - emittedEvents.any { it is VerificationCodeScreenEvents.DigitChanged } - } - - // Then - a single event carries the whole code, rather than a burst of per-digit events - val digitChanges = emittedEvents.filterIsInstance() - assert(digitChanges == listOf(VerificationCodeScreenEvents.DigitChanged(0, "123456"))) { - "Expected a single DigitChanged(0, 123456) but got $digitChanges" - } + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0), useUnmergedTree = true).assertTextEquals("1") + composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(5), useUnmergedTree = true).assertTextEquals("6") } @Test diff --git a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt index bc8062b88f..f9d719de92 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt @@ -17,6 +17,7 @@ import io.mockk.coVerify import io.mockk.mockk import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch @@ -47,6 +48,7 @@ import org.signal.registration.RegistrationFlowState import org.signal.registration.RegistrationRepository import org.signal.registration.RegistrationRoute import org.signal.registration.VerificationCodeRequest +import org.signal.uicomponents.codeentryfield.CodeEntryFieldEvents import java.util.UUID import kotlin.time.Duration.Companion.minutes import kotlin.time.Duration.Companion.seconds @@ -194,101 +196,148 @@ class VerificationCodeViewModelTest { assertThat(emittedStates.last().snackbars).isEqualTo(VerificationCodeState.Snackbars()) } - // ==================== applyEvent: SMS Auto-Fill Tests ==================== + // ==================== Code Entry Tests ==================== @Test - fun `CodeAutoFilled stores the code in autoFillCode`() = runTest { - val initialState = VerificationCodeState() + fun `the code field state is mirrored into screen state`() = runTest(testDispatcher) { + backgroundScope.launch { viewModel.state.collect {} } + advanceUntilIdle() - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.CodeAutoFilled("123456"), - stateEmitter - ) + viewModel.onEvent(VerificationCodeScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("123"))) + advanceUntilIdle() - assertThat(emittedStates.last().autoFillCode).isEqualTo("123456") + assertThat(viewModel.state.value.codeEntry.code).isEqualTo("123") + coVerify(exactly = 0) { mockRepository.submitVerificationCode(any(), any()) } } @Test - fun `DigitChanged with pasted hyphenated text populates all digits and submits`() = runTest { - val sessionMetadata = createSessionMetadata() - val initialState = VerificationCodeState( - sessionMetadata = sessionMetadata, - e164 = "+15551234567" - ) + fun `completing the code in the code field submits it`() = runTest(testDispatcher) { + val sessionMetadata = givenIncorrectCodeSubmission() - coEvery { mockRepository.submitVerificationCode(any(), any()) } returns - RequestResult.NonSuccess( - SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode("Wrong code") - ) + backgroundScope.launch { viewModel.state.collect {} } + advanceUntilIdle() - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(0, "123-456"), - stateEmitter - ) + viewModel.onEvent(VerificationCodeScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("123456"))) + advanceUntilIdle() - coVerify { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } - assertThat(emittedStates.first().digits).isEqualTo(listOf("1", "2", "3", "4", "5", "6")) - assertThat(emittedStates.first().isSubmittingCode).isTrue() + coVerify(exactly = 1) { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } } @Test - fun `DigitChanged with a pasted plain code populates all digits and submits`() = runTest { - val sessionMetadata = createSessionMetadata() - val initialState = VerificationCodeState( - sessionMetadata = sessionMetadata, - e164 = "+15551234567" - ) + fun `an incorrect code clears the code field`() = runTest(testDispatcher) { + givenIncorrectCodeSubmission() - coEvery { mockRepository.submitVerificationCode(any(), any()) } returns - RequestResult.NonSuccess( - SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode("Wrong code") - ) + backgroundScope.launch { viewModel.state.collect {} } + advanceUntilIdle() - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(0, "123456"), - stateEmitter - ) + viewModel.onEvent(VerificationCodeScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("123456"))) + advanceUntilIdle() - coVerify { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } - assertThat(emittedStates.first().digits).isEqualTo(listOf("1", "2", "3", "4", "5", "6")) - assertThat(emittedStates.first().isSubmittingCode).isTrue() + assertThat(viewModel.state.value.codeEntry.code).isEqualTo("") + assertThat(viewModel.state.value.codeEntry.pendingOverwrite).isEqualTo("") + assertThat(viewModel.state.value.snackbars.incorrectVerificationCode).isTrue() } @Test - fun `DigitChanged with pasted text of the wrong length is ignored`() = runTest { + fun `CodeEntered while already submitting is ignored`() = runTest { val initialState = VerificationCodeState( sessionMetadata = createSessionMetadata(), - e164 = "+15551234567" + e164 = "+15551234567", + isSubmittingCode = true ) - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(0, "12-345"), - stateEmitter - ) + viewModel.applyEvent(initialState, VerificationCodeScreenEvents.CodeEntered("123456"), stateEmitter) coVerify(exactly = 0) { mockRepository.submitVerificationCode(any(), any()) } - assertThat(emittedStates.last().digits).isEqualTo(listOf("", "", "", "", "", "")) + } + + // ==================== SMS Auto-Fill Tests ==================== + + @Test + fun `CodeAutoFilled fills the code field and submits the code`() = runTest(testDispatcher) { + val sessionMetadata = givenIncorrectCodeSubmission() + + backgroundScope.launch { viewModel.state.collect {} } + advanceUntilIdle() + + viewModel.onEvent(VerificationCodeScreenEvents.CodeAutoFilled("123456")) + advanceUntilIdle() + + coVerify(exactly = 1) { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } } @Test - fun `ConsumeAutoFillCode clears autoFillCode`() = runTest { - val initialState = VerificationCodeState(autoFillCode = "123456") + fun `CodeAutoFilled places the code in the code field while it is submitted`() = runTest(testDispatcher) { + givenIncorrectCodeSubmission() + coEvery { mockRepository.submitVerificationCode(any(), any()) } coAnswers { awaitCancellation() } - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.ConsumeAutoFillCode, - stateEmitter - ) + backgroundScope.launch { viewModel.state.collect {} } + advanceUntilIdle() - assertThat(emittedStates.last().autoFillCode).isNull() + viewModel.onEvent(VerificationCodeScreenEvents.CodeAutoFilled("123456")) + advanceUntilIdle() + + assertThat(viewModel.state.value.codeEntry.code).isEqualTo("123456") + assertThat(viewModel.state.value.codeEntry.pendingOverwrite).isEqualTo("123456") + assertThat(viewModel.state.value.isSubmittingCode).isTrue() } @Test - fun `codes from the SMS retriever flow are pushed into the state`() = runTest(testDispatcher) { + fun `CodeAutoFilled replaces a partially typed code`() = runTest(testDispatcher) { + val sessionMetadata = givenIncorrectCodeSubmission() + + backgroundScope.launch { viewModel.state.collect {} } + advanceUntilIdle() + + viewModel.onEvent(VerificationCodeScreenEvents.CodeEntryEvent(CodeEntryFieldEvents.CodeChanged("98"))) + advanceUntilIdle() + viewModel.onEvent(VerificationCodeScreenEvents.CodeAutoFilled("123456")) + advanceUntilIdle() + + coVerify(exactly = 1) { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } + coVerify(exactly = 0) { mockRepository.submitVerificationCode(any(), "98") } + } + + @Test + fun `CodeAutoFilled with a code longer than the field is ignored`() = runTest(testDispatcher) { + givenIncorrectCodeSubmission() + + backgroundScope.launch { viewModel.state.collect {} } + advanceUntilIdle() + + viewModel.onEvent(VerificationCodeScreenEvents.CodeAutoFilled("12345678")) + advanceUntilIdle() + + coVerify(exactly = 0) { mockRepository.submitVerificationCode(any(), any()) } + assertThat(viewModel.state.value.codeEntry.code).isEqualTo("") + assertThat(viewModel.state.value.codeEntry.pendingOverwrite).isNull() + } + + @Test + fun `CodeAutoFilled with a code shorter than the field is ignored`() = runTest(testDispatcher) { + givenIncorrectCodeSubmission() + + backgroundScope.launch { viewModel.state.collect {} } + advanceUntilIdle() + + viewModel.onEvent(VerificationCodeScreenEvents.CodeAutoFilled("123")) + advanceUntilIdle() + + assertThat(viewModel.state.value.codeEntry.code).isEqualTo("") + assertThat(viewModel.state.value.codeEntry.pendingOverwrite).isNull() + } + + @Test + fun `CodeAutoFilled while submitting is ignored`() = runTest(testDispatcher) { + viewModel.applyEvent(VerificationCodeState(isSubmittingCode = true), VerificationCodeScreenEvents.CodeAutoFilled("123456"), stateEmitter) + advanceUntilIdle() + + assertThat(viewModel.state.value.codeEntry.pendingOverwrite).isNull() + } + + @Test + fun `codes from the SMS retriever flow are submitted`() = runTest(testDispatcher) { + val sessionMetadata = givenIncorrectCodeSubmission() val smsCodes = MutableSharedFlow(extraBufferCapacity = 1) val vm = VerificationCodeViewModel(mockRepository, parentState, parentEventEmitter, smsCodes) @@ -298,191 +347,7 @@ class VerificationCodeViewModelTest { smsCodes.emit("123456") advanceUntilIdle() - assertThat(vm.state.value.autoFillCode).isEqualTo("123456") - } - - @Test - fun `DigitChanged with a full code dispatched through the event channel submits it in a single pass`() = runTest(testDispatcher) { - val sessionMetadata = createSessionMetadata() - parentState.value = RegistrationFlowState( - sessionMetadata = sessionMetadata, - sessionE164 = "+15551234567" - ) - - coEvery { mockRepository.submitVerificationCode(any(), any()) } returns - RequestResult.NonSuccess( - SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode("Wrong code") - ) - - backgroundScope.launch { viewModel.state.collect {} } - advanceUntilIdle() - - viewModel.onEvent(VerificationCodeScreenEvents.DigitChanged(0, "123456")) - advanceUntilIdle() - - coVerify { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } - } - - // ==================== applyEvent: DigitChanged Tests ==================== - - @Test - fun `DigitChanged records the value at the given index`() = runTest { - val initialState = VerificationCodeState() - - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(2, "7"), - stateEmitter - ) - - assertThat(emittedStates.last().digits).isEqualTo(listOf("", "", "7", "", "", "")) - } - - @Test - fun `DigitChanged advances the focused digit index`() = runTest { - val initialState = VerificationCodeState() - - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(2, "7"), - stateEmitter - ) - - assertThat(emittedStates.last().focusedDigitIndex).isEqualTo(3) - } - - @Test - fun `DigitChanged with an empty value moves the focused digit index back`() = runTest { - val initialState = VerificationCodeState(digits = listOf("1", "2", "3", "", "", "")) - - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(2, ""), - stateEmitter - ) - - assertThat(emittedStates.last().focusedDigitIndex).isEqualTo(1) - } - - @Test - fun `DigitChanged with an out-of-bounds index throws`() = runTest { - var threw = false - try { - viewModel.applyEvent( - VerificationCodeState(), - VerificationCodeScreenEvents.DigitChanged(9, "7"), - stateEmitter - ) - } catch (e: IllegalStateException) { - threw = true - } - - assertThat(threw).isTrue() - } - - @Test - fun `DigitChanged completing the code submits it`() = runTest { - val sessionMetadata = createSessionMetadata() - val initialState = VerificationCodeState( - sessionMetadata = sessionMetadata, - e164 = "+15551234567", - digits = listOf("1", "2", "3", "4", "5", "") - ) - - coEvery { mockRepository.submitVerificationCode(any(), any()) } returns - RequestResult.NonSuccess( - SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode("Wrong code") - ) - - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(5, "6"), - stateEmitter - ) - - coVerify { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } - assertThat(emittedStates.first().isSubmittingCode).isTrue() - assertThat(emittedStates.last().isSubmittingCode).isEqualTo(false) - } - - @Test - fun `DigitChanged does not submit until the code is complete`() = runTest { - val initialState = VerificationCodeState( - sessionMetadata = createSessionMetadata(), - e164 = "+15551234567", - digits = listOf("1", "2", "3", "4", "", "") - ) - - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(4, "5"), - stateEmitter - ) - - coVerify(exactly = 0) { mockRepository.submitVerificationCode(any(), any()) } - assertThat(emittedStates.last().isSubmittingCode).isEqualTo(false) - } - - @Test - fun `an incorrect code clears the entered digits`() = runTest { - val initialState = VerificationCodeState( - sessionMetadata = createSessionMetadata(), - e164 = "+15551234567", - digits = listOf("1", "2", "3", "4", "5", "") - ) - - coEvery { mockRepository.submitVerificationCode(any(), any()) } returns - RequestResult.NonSuccess( - SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode("Wrong code") - ) - - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(5, "6"), - stateEmitter - ) - - assertThat(emittedStates.last().digits).isEqualTo(listOf("", "", "", "", "", "")) - assertThat(emittedStates.last().snackbars.incorrectVerificationCode).isTrue() - } - - @Test - fun `DigitChanged with an empty value clears the digit at the index`() = runTest { - val initialState = VerificationCodeState(digits = listOf("1", "2", "3", "", "", "")) - - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(2, ""), - stateEmitter - ) - - assertThat(emittedStates.last().digits).isEqualTo(listOf("1", "2", "", "", "", "")) - } - - @Test - fun `DigitChanged with an empty value shifts the following digits left`() = runTest { - val initialState = VerificationCodeState(digits = listOf("1", "2", "3", "4", "5", "6")) - - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(2, ""), - stateEmitter - ) - - assertThat(emittedStates.last().digits).isEqualTo(listOf("1", "2", "4", "5", "6", "")) - } - - @Test - fun `DigitChanged with an empty value on an empty field clears the previous digit`() = runTest { - val initialState = VerificationCodeState(digits = listOf("1", "2", "", "", "", "")) - - viewModel.applyEvent( - initialState, - VerificationCodeScreenEvents.DigitChanged(2, ""), - stateEmitter - ) - - assertThat(emittedStates.last().digits).isEqualTo(listOf("1", "", "", "", "", "")) + coVerify(exactly = 1) { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } } // ==================== applyEvent: WrongNumber Tests ==================== @@ -1415,4 +1280,15 @@ class VerificationCodeViewModelTest { entitlements = null, reregistration = reregistration ) + + private fun givenIncorrectCodeSubmission(): SessionMetadata { + val sessionMetadata = createSessionMetadata() + parentState.value = RegistrationFlowState( + sessionMetadata = sessionMetadata, + sessionE164 = "+15551234567" + ) + coEvery { mockRepository.submitVerificationCode(any(), any()) } returns + RequestResult.NonSuccess(SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode("Wrong code")) + return sessionMetadata + } } diff --git a/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryField.kt b/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryField.kt index 6e6790c295..9a6ad49e39 100644 --- a/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryField.kt +++ b/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryField.kt @@ -6,54 +6,84 @@ package org.signal.uicomponents.codeentryfield import androidx.annotation.VisibleForTesting +import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.text.BasicTextField import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.foundation.text.input.InputTransformation +import androidx.compose.foundation.text.input.TextFieldBuffer +import androidx.compose.foundation.text.input.TextFieldLineLimits +import androidx.compose.foundation.text.input.placeCursorAtEnd +import androidx.compose.foundation.text.input.rememberTextFieldState +import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd +import androidx.compose.foundation.text.selection.LocalTextSelectionColors +import androidx.compose.foundation.text.selection.TextSelectionColors import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text -import androidx.compose.material3.TextField -import androidx.compose.material3.TextFieldDefaults import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.drawBehind import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusRequester -import androidx.compose.ui.input.key.Key -import androidx.compose.ui.input.key.key -import androidx.compose.ui.input.key.onKeyEvent +import androidx.compose.ui.focus.onFocusChanged +import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.SolidColor import androidx.compose.ui.platform.testTag +import androidx.compose.ui.semantics.hideFromAccessibility +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.text.TextRange +import androidx.compose.ui.text.TextStyle import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import org.signal.core.ui.compose.DayNightPreviews import org.signal.core.ui.compose.Previews import org.signal.uicomponents.codeentryfield.CodeEntryFieldState.Companion.CODE_LENGTH private val DIGIT_WIDTH = 48.dp +private val DIGIT_HEIGHT = 60.dp private val DIGIT_SPACING = 8.dp private val SEPARATOR_PADDING = 18.dp private val DIGIT_SHAPE = RoundedCornerShape(topStart = 4.dp, topEnd = 4.dp) +private val HIDDEN_SELECTION_COLORS = TextSelectionColors(handleColor = Color.Transparent, backgroundColor = Color.Transparent) @VisibleForTesting object CodeEntryFieldTestTags { + /** The text field itself, for entering text in tests. */ const val ROOT = "code-entry-field" + /** The box showing the digit at [index]. Only found in the unmerged semantics tree. */ fun digit(index: Int): String = "code-entry-field-digit-$index" } /** - * A [CODE_LENGTH]-digit code input, laid out as one box per digit in XXX-XXX format. Focus follows along as the user - * types, and a pasted code fills every box at once. + * A [CODE_LENGTH]-digit code input, laid out as one box per digit in XXX-XXX format. * - * Driven entirely by a [CodeEntryFieldPresenter], which owns the state handed in here and decides what the events sent - * back out of here actually do. + * Under the hood, this is a single invisible text field. It captures all the taps and keystrokes and whatnot, and then + * we split the submitted code and render it into the different boxes manually. If you try to do 6 distinct text boxes, + * you tend to run into weird latency issues as focus jumps between the boxes. + * + * Driven by a [CodeEntryFieldPresenter], which owns the state handed in here and decides what the events sent back out + * of here actually do. [modifier] is applied to the text field, so things like autofill content types can be set on it. */ @Composable fun CodeEntryField( @@ -61,20 +91,85 @@ fun CodeEntryField( onEvent: (CodeEntryFieldEvents) -> Unit, modifier: Modifier = Modifier, enabled: Boolean = true, - isError: Boolean = false + isError: Boolean = false, + digitSpacing: Dp = DIGIT_SPACING, + separatorPadding: Dp = SEPARATOR_PADDING ) { - val focusRequesters = remember { List(CODE_LENGTH) { FocusRequester() } } + val textFieldState = rememberTextFieldState(initialText = state.code) + val focusRequester = remember { FocusRequester() } + val currentOnEvent by rememberUpdatedState(onEvent) + var isFocused by remember { mutableStateOf(false) } - LaunchedEffect(state.focusedDigitIndex, enabled) { + LaunchedEffect(textFieldState) { + snapshotFlow { textFieldState.text.toString() } + .collect { currentOnEvent(CodeEntryFieldEvents.CodeChanged(it)) } + } + + LaunchedEffect(textFieldState) { + snapshotFlow { textFieldState.selection } + .collect { selection -> + if (selection != TextRange(textFieldState.text.length)) { + textFieldState.edit { placeCursorAtEnd() } + } + } + } + + LaunchedEffect(state.pendingOverwrite) { + val code = state.pendingOverwrite ?: return@LaunchedEffect + textFieldState.setTextAndPlaceCursorAtEnd(code) + currentOnEvent(CodeEntryFieldEvents.OverwriteApplied) + } + + LaunchedEffect(enabled) { if (enabled) { - focusRequesters[state.focusedDigitIndex.coerceIn(0, CODE_LENGTH - 1)].requestFocus() + focusRequester.requestFocus() } } + CompositionLocalProvider(LocalTextSelectionColors provides HIDDEN_SELECTION_COLORS) { + BasicTextField( + state = textFieldState, + enabled = enabled, + inputTransformation = CodeInputTransformation, + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), + lineLimits = TextFieldLineLimits.SingleLine, + textStyle = TextStyle(color = Color.Transparent), + cursorBrush = SolidColor(Color.Transparent), + modifier = modifier + .fillMaxWidth() + .focusRequester(focusRequester) + .onFocusChanged { isFocused = it.isFocused } + .testTag(CodeEntryFieldTestTags.ROOT), + decorator = { innerTextField -> + Box(contentAlignment = Alignment.Center) { + DigitBoxes( + code = textFieldState.text, + activeIndex = if (isFocused) textFieldState.text.length.coerceAtMost(CODE_LENGTH - 1) else null, + enabled = enabled, + isError = isError, + digitSpacing = digitSpacing, + separatorPadding = separatorPadding + ) + Box(modifier = Modifier.matchParentSize()) { + innerTextField() + } + } + } + ) + } +} + +@Composable +private fun DigitBoxes( + code: CharSequence, + activeIndex: Int?, + enabled: Boolean, + isError: Boolean, + digitSpacing: Dp, + separatorPadding: Dp +) { Row( - modifier = modifier - .fillMaxWidth() - .testTag(CodeEntryFieldTestTags.ROOT), + modifier = Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.Center, verticalAlignment = Alignment.CenterVertically ) { @@ -83,65 +178,99 @@ fun CodeEntryField( Text( text = "-", style = MaterialTheme.typography.headlineMedium, - color = MaterialTheme.colorScheme.onSurface, - modifier = Modifier.padding(horizontal = SEPARATOR_PADDING) + color = if (enabled) MaterialTheme.colorScheme.onSurface else MaterialTheme.colorScheme.onSurface.copy(alpha = 0.38f), + modifier = Modifier.padding(horizontal = separatorPadding) ) } else if (index > 0) { - Spacer(modifier = Modifier.width(DIGIT_SPACING)) + Spacer(modifier = Modifier.width(digitSpacing)) } - DigitField( - value = state.digits.getOrElse(index) { "" }, - onValueChange = { onEvent(CodeEntryFieldEvents.DigitChanged(index, it)) }, - focusRequester = focusRequesters[index], + DigitBox( + digit = code.getOrNull(index)?.toString() ?: "", + isActive = index == activeIndex, enabled = enabled, isError = isError, - modifier = Modifier - .weight(1f, fill = false) - .testTag(CodeEntryFieldTestTags.digit(index)) + testTag = CodeEntryFieldTestTags.digit(index), + modifier = Modifier.weight(1f, fill = false) ) } } } @Composable -private fun DigitField( - value: String, - onValueChange: (String) -> Unit, - focusRequester: FocusRequester, +private fun DigitBox( + digit: String, + isActive: Boolean, enabled: Boolean, isError: Boolean, + testTag: String, modifier: Modifier = Modifier ) { - TextField( - value = value, - onValueChange = onValueChange, + val indicatorColor = when { + !enabled -> MaterialTheme.colorScheme.outline.copy(alpha = 0.38f) + isError -> MaterialTheme.colorScheme.error + isActive -> MaterialTheme.colorScheme.primary + else -> MaterialTheme.colorScheme.outline + } + val indicatorThickness = if (isActive) 2.dp else 1.dp + + Box( + contentAlignment = Alignment.Center, modifier = modifier .width(DIGIT_WIDTH) - .focusRequester(focusRequester) - .onKeyEvent { keyEvent -> - if ((keyEvent.key == Key.Backspace || keyEvent.key == Key.Delete) && value.isEmpty()) { - onValueChange("") - true - } else { - false - } - }, - textStyle = MaterialTheme.typography.titleLarge.copy(textAlign = TextAlign.Center), - enabled = enabled, - isError = isError, - singleLine = true, - shape = DIGIT_SHAPE, - keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), - colors = TextFieldDefaults.colors( - focusedContainerColor = MaterialTheme.colorScheme.surfaceVariant, - unfocusedContainerColor = MaterialTheme.colorScheme.surfaceVariant, - disabledContainerColor = MaterialTheme.colorScheme.surfaceVariant, - errorContainerColor = MaterialTheme.colorScheme.surfaceVariant, - focusedIndicatorColor = MaterialTheme.colorScheme.primary, - unfocusedIndicatorColor = MaterialTheme.colorScheme.outline + .height(DIGIT_HEIGHT) + .background(color = MaterialTheme.colorScheme.surfaceVariant, shape = DIGIT_SHAPE) + .drawBehind { + val thickness = indicatorThickness.toPx() + val y = size.height - thickness / 2 + drawLine( + color = indicatorColor, + start = Offset(0f, y), + end = Offset(size.width, y), + strokeWidth = thickness + ) + } + ) { + Text( + text = digit, + style = MaterialTheme.typography.titleLarge, + textAlign = TextAlign.Center, + color = if (enabled) MaterialTheme.colorScheme.onSurface else MaterialTheme.colorScheme.onSurface.copy(alpha = 0.38f), + modifier = Modifier + .fillMaxWidth() + .testTag(testTag) + .semantics { hideFromAccessibility() } ) - ) + } +} + +/** + * Keeps the field to at most [CODE_LENGTH] digits, dropping anything else (like the dash in a pasted "123-456"). + * Pasting a full code replaces whatever was already entered, and any other input that would overflow the field is + * rejected. + */ +private object CodeInputTransformation : InputTransformation { + override fun TextFieldBuffer.transformInput() { + val digits = asCharSequence().filter { it.isDigit() }.toString() + val original = originalText.toString() + val insertedLength = digits.length - original.length + + val sanitized = when { + digits.length <= CODE_LENGTH -> digits + digits.startsWith(original) && insertedLength >= CODE_LENGTH -> digits.substring(original.length, original.length + CODE_LENGTH) + else -> null + } + + if (sanitized == null) { + revertAllChanges() + return + } + + if (sanitized != asCharSequence().toString()) { + replace(0, length, sanitized) + } + placeCursorAtEnd() + } } @DayNightPreviews @@ -149,7 +278,7 @@ private fun DigitField( private fun CodeEntryFieldPreview() { Previews.Preview { CodeEntryField( - state = CodeEntryFieldState(digits = listOf("4", "1", "8", "3", "7", "2")), + state = CodeEntryFieldState(code = "418372"), onEvent = {} ) } @@ -160,7 +289,7 @@ private fun CodeEntryFieldPreview() { private fun CodeEntryFieldPartiallyFilledPreview() { Previews.Preview { CodeEntryField( - state = CodeEntryFieldState(digits = listOf("4", "1", "8", "", "", ""), focusedDigitIndex = 3), + state = CodeEntryFieldState(code = "418"), onEvent = {} ) } @@ -171,9 +300,21 @@ private fun CodeEntryFieldPartiallyFilledPreview() { private fun CodeEntryFieldErrorPreview() { Previews.Preview { CodeEntryField( - state = CodeEntryFieldState(digits = listOf("4", "1", "8", "3", "7", "2")), + state = CodeEntryFieldState(code = "418372"), onEvent = {}, isError = true ) } } + +@DayNightPreviews +@Composable +private fun CodeEntryFieldDisabledPreview() { + Previews.Preview { + CodeEntryField( + state = CodeEntryFieldState(code = "418372"), + onEvent = {}, + enabled = false + ) + } +} diff --git a/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldEvents.kt b/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldEvents.kt index f705a2beb0..eab0daaf2e 100644 --- a/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldEvents.kt +++ b/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldEvents.kt @@ -15,10 +15,19 @@ import org.signal.core.util.censor */ sealed interface CodeEntryFieldEvents { - /** - * The raw [value] of the digit field at [index] changed. - */ - data class DigitChanged(val index: Int, val value: String) : CodeEntryFieldEvents { - override fun toString(): String = "DigitChanged(index=$index, value=${value.censor()})" + /** The contents of the field changed to [code]. */ + data class CodeChanged(val code: String) : CodeEntryFieldEvents { + override fun toString(): String = "CodeChanged(code=${code.censor()})" } + + /** The field has applied [CodeEntryFieldState.pendingOverwrite]. */ + data object OverwriteApplied : CodeEntryFieldEvents + + /** Replaces the code in the field with [code], such as one auto-filled from an SMS. */ + data class SetCode(val code: String) : CodeEntryFieldEvents { + override fun toString(): String = "SetCode(code=${code.censor()})" + } + + /** Clears the field, such as after the entered code was rejected. */ + data object Clear : CodeEntryFieldEvents } diff --git a/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldPresenter.kt b/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldPresenter.kt index 1c05401747..da1f4da7fa 100644 --- a/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldPresenter.kt +++ b/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldPresenter.kt @@ -18,8 +18,8 @@ import org.signal.core.util.logging.Log import org.signal.uicomponents.codeentryfield.CodeEntryFieldState.Companion.CODE_LENGTH /** - * All of the logic behind a [CodeEntryField]: turning the raw text each digit field reports into a code, moving focus - * along as the user types, and saying when the code is finished. + * All of the logic behind a [CodeEntryField]: tracking the code the field reports, letting the host replace or clear + * it, and saying when the code is finished. * * Meant to be held by the view model of whichever screen shows the field, which feeds it events, mirrors [state] into * its own state, and carries out [actions]. @@ -40,88 +40,35 @@ class CodeEntryFieldPresenter( override suspend fun processEvent(event: CodeEntryFieldEvents) { when (event) { - is CodeEntryFieldEvents.DigitChanged -> { - applyDigitChanged(event.index, event.value) - } + is CodeEntryFieldEvents.CodeChanged -> applyCodeChanged(event.code) + is CodeEntryFieldEvents.OverwriteApplied -> _state.update { it.copy(pendingOverwrite = null) } + is CodeEntryFieldEvents.SetCode -> applySetCode(event.code) + is CodeEntryFieldEvents.Clear -> applySetCode("") } } /** - * Interprets the raw [value] reported by the digit field at [index] and updates the digits and focus accordingly: - * - * - an empty [value] is a backspace, deleting a digit and moving focus back - * - a single digit is recorded and focus advances - * - multi-character input (e.g. a pasted code) populates every field at once - * - * Once every field has a value, the completed code is emitted. + * Records what the field now holds. While an overwrite is pending, the field's contents are about to be replaced, so + * anything it reports in the meantime is stale and ignored. */ - private suspend fun applyDigitChanged(index: Int, value: String) { - check(index in _state.value.digits.indices) { "[DigitChanged] Out of bounds index $index." } - - if (value.isEmpty()) { - deleteDigit(index) + private suspend fun applyCodeChanged(code: String) { + val state = _state.value + if (state.pendingOverwrite != null || code == state.code) { return } - val currentValue = _state.value.digits[index] - val remainder = if (currentValue.isNotEmpty()) value.replaceFirst(currentValue, "") else value - val addedDigits = remainder.filter { it.isDigit() } - - when { - addedDigits.isEmpty() -> Unit - - addedDigits.length == 1 -> { - _state.update { - it.copy( - digits = it.digits.toMutableList().also { digits -> digits[index] = addedDigits }, - focusedDigitIndex = (index + 1).coerceAtMost(CODE_LENGTH - 1) - ) - } - emitCodeIfComplete() - } - - else -> applyFullCode(addedDigits) - } - } - - /** - * Populates every digit field from a full pasted [code] at once. Multi-character input that isn't a complete code - * is ignored. - */ - private suspend fun applyFullCode(code: String) { - if (code.length != CODE_LENGTH) { - Log.w(TAG, "[DigitChanged] Ignoring multi-character input containing ${code.length} digits.") - return - } - - _state.update { - it.copy( - digits = code.map { digit -> digit.toString() }, - focusedDigitIndex = CODE_LENGTH - 1 - ) - } + _state.update { it.copy(code = code) } emitCodeIfComplete() } - /** - * Deletes the digit at [index] (or the previous one, if [index] is already empty), shifts any following digits left - * to fill the gap, and moves focus back. - */ - private fun deleteDigit(index: Int) { - val digits = _state.value.digits - val deleteAt = if (digits[index].isNotEmpty()) index else index - 1 - if (deleteAt < 0) { + private suspend fun applySetCode(code: String) { + if (code.length > CODE_LENGTH || !code.all { it.isDigit() }) { + Log.w(TAG, "[SetCode] Ignoring a code that isn't up to $CODE_LENGTH digits. Length: ${code.length}") return } - val newDigits = digits.toMutableList().apply { - for (j in deleteAt until CODE_LENGTH - 1) { - this[j] = this[j + 1] - } - this[CODE_LENGTH - 1] = "" - } - - _state.update { it.copy(digits = newDigits, focusedDigitIndex = (index - 1).coerceAtLeast(0)) } + _state.update { it.copy(code = code, pendingOverwrite = code) } + emitCodeIfComplete() } private suspend fun emitCodeIfComplete() { diff --git a/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldState.kt b/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldState.kt index ce53f01dab..622e28ff59 100644 --- a/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldState.kt +++ b/lib/ui-components/src/main/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldState.kt @@ -12,22 +12,16 @@ package org.signal.uicomponents.codeentryfield * Reminder that this is logged, so don't put the code itself in the toString. */ data class CodeEntryFieldState( - val digits: List = emptyDigits(), - val focusedDigitIndex: Int = 0 + val code: String = "", + /** Set when the code was replaced from outside the field. The field applies it, then reports [CodeEntryFieldEvents.OverwriteApplied]. */ + val pendingOverwrite: String? = null ) { - override fun toString(): String = "CodeEntryFieldState(digitsEntered=${digits.count { it.isNotEmpty() }}, focusedDigitIndex=$focusedDigitIndex)" + override fun toString(): String = "CodeEntryFieldState(digitsEntered=${code.length}, hasPendingOverwrite=${pendingOverwrite != null})" - /** - * The full code as currently entered. Only meaningful when [isComplete] is true. - */ - val code: String get() = digits.joinToString("") - - val isComplete: Boolean get() = digits.size == CODE_LENGTH && digits.all { it.isNotEmpty() } + val isComplete: Boolean get() = code.length == CODE_LENGTH companion object { const val CODE_LENGTH = 6 - - fun emptyDigits(): List = List(CODE_LENGTH) { "" } } } diff --git a/lib/ui-components/src/test/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldPresenterTest.kt b/lib/ui-components/src/test/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldPresenterTest.kt index 2a8f38013a..cc6f0a123e 100644 --- a/lib/ui-components/src/test/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldPresenterTest.kt +++ b/lib/ui-components/src/test/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldPresenterTest.kt @@ -10,6 +10,7 @@ import assertk.assertions.containsExactly import assertk.assertions.isEmpty import assertk.assertions.isEqualTo import assertk.assertions.isFalse +import assertk.assertions.isNull import assertk.assertions.isTrue import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.launchIn @@ -27,103 +28,91 @@ class CodeEntryFieldPresenterTest { private val emittedActions = mutableListOf() @Test - fun `initial state is empty with focus on the first digit`() = runTest(testDispatcher) { + fun `initial state is empty`() = runTest(testDispatcher) { val presenter = createPresenter() - assertThat(presenter.state.value.digits).isEqualTo(CodeEntryFieldState.emptyDigits()) - assertThat(presenter.state.value.focusedDigitIndex).isEqualTo(0) + assertThat(presenter.state.value.code).isEqualTo("") + assertThat(presenter.state.value.pendingOverwrite).isNull() assertThat(presenter.state.value.isComplete).isFalse() } @Test - fun `entering a digit records it and advances focus`() = runTest(testDispatcher) { + fun `a partial code is recorded without being emitted`() = runTest(testDispatcher) { val presenter = createPresenter() - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(0, "4")) + presenter.onEvent(CodeEntryFieldEvents.CodeChanged("418")) - assertThat(presenter.state.value.digits[0]).isEqualTo("4") - assertThat(presenter.state.value.focusedDigitIndex).isEqualTo(1) + assertThat(presenter.state.value.code).isEqualTo("418") + assertThat(emittedActions).isEmpty() } @Test - fun `entering the final digit emits the code`() = runTest(testDispatcher) { + fun `completing the code emits it`() = runTest(testDispatcher) { val presenter = createPresenter() - "41837".forEachIndexed { index, digit -> - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(index, digit.toString())) - } - assertThat(emittedActions).isEmpty() - - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(5, "2")) + presenter.onEvent(CodeEntryFieldEvents.CodeChanged("41837")) + presenter.onEvent(CodeEntryFieldEvents.CodeChanged("418372")) assertThat(presenter.state.value.isComplete).isTrue() assertThat(emittedActions).containsExactly(CodeEntryFieldAction.CodeEntered("418372")) } @Test - fun `pasting a full code populates every field and emits the code`() = runTest(testDispatcher) { + fun `the same complete code is only emitted once`() = runTest(testDispatcher) { val presenter = createPresenter() - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(0, "418372")) + presenter.onEvent(CodeEntryFieldEvents.CodeChanged("418372")) + presenter.onEvent(CodeEntryFieldEvents.CodeChanged("418372")) - assertThat(presenter.state.value.digits).isEqualTo(listOf("4", "1", "8", "3", "7", "2")) - assertThat(presenter.state.value.focusedDigitIndex).isEqualTo(5) assertThat(emittedActions).containsExactly(CodeEntryFieldAction.CodeEntered("418372")) } @Test - fun `pasting an incomplete code is ignored`() = runTest(testDispatcher) { + fun `SetCode replaces the code, asks the field to show it, and emits it when complete`() = runTest(testDispatcher) { val presenter = createPresenter() + presenter.onEvent(CodeEntryFieldEvents.CodeChanged("12")) - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(0, "4183")) + presenter.onEvent(CodeEntryFieldEvents.SetCode("418372")) - assertThat(presenter.state.value.digits).isEqualTo(CodeEntryFieldState.emptyDigits()) - assertThat(emittedActions).isEmpty() + assertThat(presenter.state.value.code).isEqualTo("418372") + assertThat(presenter.state.value.pendingOverwrite).isEqualTo("418372") + assertThat(emittedActions).containsExactly(CodeEntryFieldAction.CodeEntered("418372")) } @Test - fun `non-digit input is ignored`() = runTest(testDispatcher) { + fun `SetCode with something other than a code is ignored`() = runTest(testDispatcher) { val presenter = createPresenter() - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(0, "a")) + presenter.onEvent(CodeEntryFieldEvents.SetCode("4183721")) + presenter.onEvent(CodeEntryFieldEvents.SetCode("418-372")) - assertThat(presenter.state.value.digits).isEqualTo(CodeEntryFieldState.emptyDigits()) - assertThat(presenter.state.value.focusedDigitIndex).isEqualTo(0) + assertThat(presenter.state.value.code).isEqualTo("") + assertThat(presenter.state.value.pendingOverwrite).isNull() } @Test - fun `a backspace deletes the digit and shifts the following ones left`() = runTest(testDispatcher) { + fun `Clear empties the code and asks the field to clear`() = runTest(testDispatcher) { val presenter = createPresenter() + presenter.onEvent(CodeEntryFieldEvents.CodeChanged("418372")) - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(0, "4")) - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(1, "1")) - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(2, "8")) + presenter.onEvent(CodeEntryFieldEvents.Clear) - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(1, "")) - - assertThat(presenter.state.value.digits).isEqualTo(listOf("4", "8", "", "", "", "")) - assertThat(presenter.state.value.focusedDigitIndex).isEqualTo(0) + assertThat(presenter.state.value.code).isEqualTo("") + assertThat(presenter.state.value.pendingOverwrite).isEqualTo("") } @Test - fun `a backspace on an empty field deletes the previous digit`() = runTest(testDispatcher) { + fun `what the field reports while an overwrite is pending is ignored`() = runTest(testDispatcher) { val presenter = createPresenter() + presenter.onEvent(CodeEntryFieldEvents.SetCode("418372")) - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(0, "4")) - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(1, "")) + presenter.onEvent(CodeEntryFieldEvents.CodeChanged("12")) + presenter.onEvent(CodeEntryFieldEvents.OverwriteApplied) + presenter.onEvent(CodeEntryFieldEvents.CodeChanged("418372")) - assertThat(presenter.state.value.digits).isEqualTo(CodeEntryFieldState.emptyDigits()) - assertThat(presenter.state.value.focusedDigitIndex).isEqualTo(0) - } - - @Test - fun `a backspace on the first empty field does nothing`() = runTest(testDispatcher) { - val presenter = createPresenter() - - presenter.onEvent(CodeEntryFieldEvents.DigitChanged(0, "")) - - assertThat(presenter.state.value.digits).isEqualTo(CodeEntryFieldState.emptyDigits()) - assertThat(presenter.state.value.focusedDigitIndex).isEqualTo(0) + assertThat(presenter.state.value.code).isEqualTo("418372") + assertThat(presenter.state.value.pendingOverwrite).isNull() + assertThat(emittedActions).containsExactly(CodeEntryFieldAction.CodeEntered("418372")) } private fun TestScope.createPresenter(): CodeEntryFieldPresenter { diff --git a/lib/ui-components/src/test/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldTest.kt b/lib/ui-components/src/test/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldTest.kt index 0e08a0dace..743ad85fc5 100644 --- a/lib/ui-components/src/test/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldTest.kt +++ b/lib/ui-components/src/test/java/org/signal/uicomponents/codeentryfield/CodeEntryFieldTest.kt @@ -14,6 +14,8 @@ import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performTextInput import assertk.assertThat import assertk.assertions.contains +import assertk.assertions.containsOnly +import assertk.assertions.isEqualTo import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -30,52 +32,106 @@ class CodeEntryFieldTest { private val events = mutableListOf() + private val codeChanges: List + get() = events.filterIsInstance().map { it.code } + @Test fun `field displays one box per digit`() { setContent(CodeEntryFieldState()) for (index in 0 until CODE_LENGTH) { - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(index)).assertIsDisplayed() + digit(index).assertIsDisplayed() } } @Test - fun `field renders the digits from state`() { - setContent(CodeEntryFieldState(digits = listOf("4", "1", "8", "3", "7", "2"))) + fun `field renders the initial code from state`() { + setContent(CodeEntryFieldState(code = "418")) - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0)).assertTextEquals("4") - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(5)).assertTextEquals("2") + digit(0).assertTextEquals("4") + digit(2).assertTextEquals("8") + digit(3).assertTextEquals("") } @Test - fun `entering a digit emits DigitChanged for that box`() { + fun `typing emits the whole code after each keystroke`() { setContent(CodeEntryFieldState()) - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0)).performTextInput("4") - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(1)).performTextInput("1") + field().performTextInput("4") + composeTestRule.waitForIdle() + field().performTextInput("1") composeTestRule.waitForIdle() - assertThat(events).contains(CodeEntryFieldEvents.DigitChanged(0, "4")) - assertThat(events).contains(CodeEntryFieldEvents.DigitChanged(1, "1")) + assertThat(codeChanges).contains("4") + assertThat(codeChanges.last()).isEqualTo("41") } @Test - fun `pasting into a box emits DigitChanged with the raw text`() { + fun `rapid keystrokes are all kept even when the state never catches up`() { setContent(CodeEntryFieldState()) - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0)).performTextInput("418-372") + "418372".forEach { field().performTextInput(it.toString()) } composeTestRule.waitForIdle() - assertThat(events).contains(CodeEntryFieldEvents.DigitChanged(0, "418-372")) + assertThat(codeChanges.last()).isEqualTo("418372") + digit(0).assertTextEquals("4") + digit(5).assertTextEquals("2") + } + + @Test + fun `pasting a hyphenated code keeps only the digits`() { + setContent(CodeEntryFieldState()) + + field().performTextInput("418-372") + composeTestRule.waitForIdle() + + assertThat(codeChanges.last()).isEqualTo("418372") + } + + @Test + fun `pasting a full code over a partial code replaces it`() { + setContent(CodeEntryFieldState(code = "12")) + + field().performTextInput("418372") + composeTestRule.waitForIdle() + + assertThat(codeChanges.last()).isEqualTo("418372") + } + + @Test + fun `typing past a full code is ignored`() { + setContent(CodeEntryFieldState(code = "418372")) + + field().performTextInput("7") + composeTestRule.waitForIdle() + + assertThat(codeChanges).containsOnly("418372") + } + + @Test + fun `a pending overwrite replaces the field contents and is acknowledged`() { + setContent(CodeEntryFieldState(code = "12", pendingOverwrite = "418372")) + + composeTestRule.waitUntil(timeoutMillis = 5_000) { + events.any { it is CodeEntryFieldEvents.OverwriteApplied } + } + composeTestRule.waitForIdle() + + assertThat(codeChanges.last()).isEqualTo("418372") + digit(5).assertTextEquals("2") } @Test fun `a disabled field cannot be typed in`() { setContent(CodeEntryFieldState(), enabled = false) - composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0)).assertIsNotEnabled() + field().assertIsNotEnabled() } + private fun field() = composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.ROOT) + + private fun digit(index: Int) = composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(index), useUnmergedTree = true) + private fun setContent(state: CodeEntryFieldState, enabled: Boolean = true) { composeTestRule.setContent { CodeEntryField(