signalapp/Signal-Android
signalapp/Signal-Android
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases 16 Wiki Activity

Improve pin creation UX in regV5.

Browse Source
This commit is contained in:
Greyson Parrelli 2026-06-18 16:58:38 -04:00 committed by jeffrey-signal
parent 64496d1d92
commit c55f281213
No known key found for this signature in database
6 changed files with 245 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

225
feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreen.kt
View File

@ -5,6 +5,14 @@
package org.signal.registration.screens.pincreation
import androidx.activity.compose.BackHandler
import androidx.compose.animation.AnimatedContent
import androidx.compose.animation.core.tween
import androidx.compose.animation.fadeIn
import androidx.compose.animation.fadeOut
import androidx.compose.animation.slideInHorizontally
import androidx.compose.animation.slideOutHorizontally
import androidx.compose.animation.togetherWith
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
@ -29,6 +37,8 @@ import androidx.compose.material3.TextField
import androidx.compose.material3.TopAppBarScrollBehavior
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
@ -64,9 +74,11 @@ import org.signal.registration.screens.TwoPaneRegistrationScaffold
import org.signal.registration.screens.attachDebugLogHelper
import org.signal.core.ui.R as CoreR
private const val STEP_TRANSITION_DURATION = 250
/**
* PIN creation screen for the registration flow.
* Allows users to create a new PIN for their account.
* Allows users to create a new PIN for their account, then confirm it.
*/
@Composable
fun PinCreationScreen(
@ -74,18 +86,19 @@ fun PinCreationScreen(
onEvent: (PinCreationScreenEvents) -> Unit,
modifier: Modifier = Modifier
) {
var pin by rememberSaveable { mutableStateOf("") }
val focusRequester = remember { FocusRequester() }
val canSubmitPin = pin.length >= 4
val activePin = remember { mutableStateOf("") }
val canSubmitPin = activePin.value.length >= 4
BackHandler(enabled = state.isConfirmEnabled) {
onEvent(PinCreationScreenEvents.BackToPinEntry)
}
when (val params = RegistrationScaffold.rememberLayoutParams()) {
is RegistrationScaffold.Params.OnePane -> OnePaneLayout(
params = params,
state = state,
pin = pin,
activePin = activePin,
canSubmitPin = canSubmitPin,
focusRequester = focusRequester,
onPinChanged = { pin = it },
onEvent = onEvent,
modifier = modifier
)
@ -93,19 +106,12 @@ fun PinCreationScreen(
is RegistrationScaffold.Params.TwoPane -> TwoPaneLayout(
params = params,
state = state,
pin = pin,
activePin = activePin,
canSubmitPin = canSubmitPin,
focusRequester = focusRequester,
onPinChanged = { pin = it },
onEvent = onEvent,
modifier = modifier
)
}
// autofocus PIN field on initial composition
LaunchedEffect(Unit) {
focusRequester.requestFocus()
}
}
@OptIn(ExperimentalMaterial3Api::class)
@ -113,10 +119,8 @@ fun PinCreationScreen(
private fun OnePaneLayout(
params: RegistrationScaffold.Params.OnePane,
state: PinCreationState,
pin: String,
activePin: MutableState<String>,
canSubmitPin: Boolean,
focusRequester: FocusRequester,
onPinChanged: (String) -> Unit,
onEvent: (PinCreationScreenEvents) -> Unit,
modifier: Modifier = Modifier
) {
@ -139,31 +143,23 @@ private fun OnePaneLayout(
.verticalScroll(scrollState)
.padding(params.panePadding(hasHeader = true))
) {
PinDescription(
isConfirmEnabled = state.isConfirmEnabled,
onLearnMore = { onEvent(PinCreationScreenEvents.LearnMore) }
)
PinStepTransition(isConfirmEnabled = state.isConfirmEnabled) { isConfirm ->
Column {
PinDescription(
isConfirmEnabled = isConfirm,
onLearnMore = { onEvent(PinCreationScreenEvents.LearnMore) }
)
Spacer(modifier = Modifier.height(24.dp))
Spacer(modifier = Modifier.height(24.dp))
PinInputField(
state = state,
pin = pin,
canSubmitPin = canSubmitPin,
focusRequester = focusRequester,
onPinChanged = onPinChanged,
onSubmit = { onEvent(PinCreationScreenEvents.PinSubmitted(pin)) },
modifier = Modifier.fillMaxWidth()
)
Spacer(modifier = Modifier.height(8.dp))
PinInputLabel(state)
Spacer(modifier = Modifier.height(16.dp))
KeyboardToggleButton(
state = state,
onToggleKeyboard = { onEvent(PinCreationScreenEvents.ToggleKeyboard) }
)
PinInputSection(
state = state,
isConfirm = isConfirm,
activePin = activePin,
onEvent = onEvent
)
}
}
}
},
footer = {
@ -171,7 +167,7 @@ private fun OnePaneLayout(
params = params,
canSubmitPin = canSubmitPin,
isElevated = scrollState.canScrollForward,
onNext = { onEvent(PinCreationScreenEvents.PinSubmitted(pin)) }
onNext = { onEvent(PinCreationScreenEvents.PinSubmitted(activePin.value)) }
)
}
)
@ -182,10 +178,8 @@ private fun OnePaneLayout(
private fun TwoPaneLayout(
params: RegistrationScaffold.Params.TwoPane,
state: PinCreationState,
pin: String,
activePin: MutableState<String>,
canSubmitPin: Boolean,
focusRequester: FocusRequester,
onPinChanged: (String) -> Unit,
onEvent: (PinCreationScreenEvents) -> Unit,
modifier: Modifier = Modifier
) {
@ -211,10 +205,12 @@ private fun TwoPaneLayout(
.verticalScroll(firstPaneScrollState)
.padding(paddingValues)
) {
PinDescription(
isConfirmEnabled = state.isConfirmEnabled,
onLearnMore = { onEvent(PinCreationScreenEvents.LearnMore) }
)
PinStepTransition(isConfirmEnabled = state.isConfirmEnabled) { isConfirm ->
PinDescription(
isConfirmEnabled = isConfirm,
onLearnMore = { onEvent(PinCreationScreenEvents.LearnMore) }
)
}
}
},
secondPane = { paddingValues ->
@ -226,23 +222,14 @@ private fun TwoPaneLayout(
.verticalScroll(secondPaneScrollState)
.padding(paddingValues)
) {
PinInputField(
state = state,
pin = pin,
canSubmitPin = canSubmitPin,
focusRequester = focusRequester,
onPinChanged = onPinChanged,
onSubmit = { onEvent(PinCreationScreenEvents.PinSubmitted(pin)) },
modifier = Modifier.fillMaxWidth()
)
Spacer(modifier = Modifier.height(8.dp))
PinInputLabel(state)
Spacer(modifier = Modifier.height(16.dp))
KeyboardToggleButton(
state = state,
onToggleKeyboard = { onEvent(PinCreationScreenEvents.ToggleKeyboard) }
)
PinStepTransition(isConfirmEnabled = state.isConfirmEnabled) { isConfirm ->
PinInputSection(
state = state,
isConfirm = isConfirm,
activePin = activePin,
onEvent = onEvent
)
}
}
},
footer = {
@ -250,12 +237,40 @@ private fun TwoPaneLayout(
params = params,
canSubmitPin = canSubmitPin,
isElevated = firstPaneScrollState.canScrollForward || secondPaneScrollState.canScrollForward,
onNext = { onEvent(PinCreationScreenEvents.PinSubmitted(pin)) }
onNext = { onEvent(PinCreationScreenEvents.PinSubmitted(activePin.value)) }
)
}
)
}
/**
* Animates between the create and confirm steps with a horizontal slide, as if they were separate screens.
* Moving forward to the confirm step slides in from the right; returning to the create step slides in from the left.
*/
@Composable
private fun PinStepTransition(
isConfirmEnabled: Boolean,
modifier: Modifier = Modifier,
content: @Composable (isConfirm: Boolean) -> Unit
) {
AnimatedContent(
targetState = isConfirmEnabled,
transitionSpec = {
if (targetState) {
(slideInHorizontally(animationSpec = tween(STEP_TRANSITION_DURATION)) { it } + fadeIn(tween(STEP_TRANSITION_DURATION))) togetherWith
(slideOutHorizontally(animationSpec = tween(STEP_TRANSITION_DURATION)) { -it } + fadeOut(tween(STEP_TRANSITION_DURATION)))
} else {
(slideInHorizontally(animationSpec = tween(STEP_TRANSITION_DURATION)) { -it } + fadeIn(tween(STEP_TRANSITION_DURATION))) togetherWith
(slideOutHorizontally(animationSpec = tween(STEP_TRANSITION_DURATION)) { it } + fadeOut(tween(STEP_TRANSITION_DURATION)))
}
},
label = "PinCreationStep",
modifier = modifier
) { isConfirm ->
content(isConfirm)
}
}
@Composable
private fun PinDescription(
isConfirmEnabled: Boolean,
@ -314,8 +329,54 @@ private fun PinDescription(
}
@Composable
private fun PinInputField(
private fun PinInputSection(
state: PinCreationState,
isConfirm: Boolean,
activePin: MutableState<String>,
onEvent: (PinCreationScreenEvents) -> Unit,
modifier: Modifier = Modifier
) {
var pin by remember { mutableStateOf("") }
val canSubmitPin = pin.length >= 4
val focusRequester = remember { FocusRequester() }
// Keep the footer's submit action in sync with the step currently on screen.
if (isConfirm == state.isConfirmEnabled) {
SideEffect { activePin.value = pin }
}
LaunchedEffect(Unit) {
focusRequester.requestFocus()
}
Column(modifier = modifier) {
PinInputField(
isAlphanumericKeyboard = state.isAlphanumericKeyboard,
pin = pin,
canSubmitPin = canSubmitPin,
focusRequester = focusRequester,
onPinChanged = { pin = it },
onSubmit = { onEvent(PinCreationScreenEvents.PinSubmitted(pin)) },
modifier = Modifier.fillMaxWidth()
)
Spacer(modifier = Modifier.height(8.dp))
PinInputLabel(
isConfirm = isConfirm,
isAlphanumericKeyboard = state.isAlphanumericKeyboard,
isMismatch = state.pinMismatch
)
Spacer(modifier = Modifier.height(16.dp))
KeyboardToggleButton(
isAlphanumericKeyboard = state.isAlphanumericKeyboard,
onToggleKeyboard = { onEvent(PinCreationScreenEvents.ToggleKeyboard) }
)
}
}
@Composable
private fun PinInputField(
isAlphanumericKeyboard: Boolean,
pin: String,
canSubmitPin: Boolean,
focusRequester: FocusRequester,
@ -330,7 +391,7 @@ private fun PinInputField(
textStyle = MaterialTheme.typography.bodyLarge.copy(textAlign = TextAlign.Center),
singleLine = true,
keyboardOptions = KeyboardOptions(
keyboardType = if (state.isAlphanumericKeyboard) KeyboardType.Text else KeyboardType.Number,
keyboardType = if (isAlphanumericKeyboard) KeyboardType.Text else KeyboardType.Number,
imeAction = ImeAction.Done
),
keyboardActions = KeyboardActions(onDone = { if (canSubmitPin) onSubmit() }),
@ -340,25 +401,28 @@ private fun PinInputField(
@Composable
private fun PinInputLabel(
state: PinCreationState,
isConfirm: Boolean,
isAlphanumericKeyboard: Boolean,
isMismatch: Boolean,
modifier: Modifier = Modifier
) {
Text(
text = when {
state.isConfirmEnabled -> stringResource(R.string.PinCreationScreen__reenter_pin)
state.isAlphanumericKeyboard -> stringResource(R.string.PinCreationScreen__pin_at_least_4_characters)
isConfirm -> stringResource(R.string.PinCreationScreen__reenter_pin)
isMismatch -> stringResource(R.string.PinCreationScreen__pins_dont_match)
isAlphanumericKeyboard -> stringResource(R.string.PinCreationScreen__pin_at_least_4_characters)
else -> stringResource(R.string.PinCreationScreen__pin_at_least_4_digits)
},
style = MaterialTheme.typography.bodyMedium,
textAlign = TextAlign.Center,
color = MaterialTheme.colorScheme.onSurfaceVariant,
color = if (!isConfirm && isMismatch) MaterialTheme.colorScheme.error else MaterialTheme.colorScheme.onSurfaceVariant,
modifier = modifier.fillMaxWidth()
)
}
@Composable
private fun KeyboardToggleButton(
state: PinCreationState,
isAlphanumericKeyboard: Boolean,
onToggleKeyboard: () -> Unit,
modifier: Modifier = Modifier
) {
@ -372,7 +436,7 @@ private fun KeyboardToggleButton(
modifier = Modifier.padding(end = 8.dp)
)
Text(
text = if (state.isAlphanumericKeyboard) {
text = if (isAlphanumericKeyboard) {
stringResource(R.string.PinCreationScreen__switch_to_numeric)
} else {
stringResource(R.string.PinCreationScreen__switch_to_alphanumeric)
@ -511,3 +575,14 @@ private fun PinCreationScreenConfirmPreview() {
)
}
}
@AllDevicePreviews
@Composable
private fun PinCreationScreenMismatchPreview() {
Previews.Preview {
PinCreationScreen(
state = PinCreationState(pinMismatch = true),
onEvent = {}
)
}
}

1
feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreenEvents.kt
View File

@ -12,4 +12,5 @@ sealed class PinCreationScreenEvents {
data object ToggleKeyboard : PinCreationScreenEvents()
data object LearnMore : PinCreationScreenEvents()
data object OptOut : PinCreationScreenEvents()
data object BackToPinEntry : PinCreationScreenEvents()
}

4
feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationState.kt
View File

@ -11,9 +11,11 @@ import org.signal.core.util.censor
data class PinCreationState(
val isAlphanumericKeyboard: Boolean = false,
val isConfirmEnabled: Boolean = false,
val pinMismatch: Boolean = false,
val firstPin: String? = null,
val accountEntropyPool: AccountEntropyPool? = null
) {
override fun toString(): String {
return "PinCreationState(isAlphanumericKeyboard=$isAlphanumericKeyboard, isConfirmEnabled=$isConfirmEnabled, accountEntropyPool=${accountEntropyPool?.displayValue?.censor()})"
return "PinCreationState(isAlphanumericKeyboard=$isAlphanumericKeyboard, isConfirmEnabled=$isConfirmEnabled, pinMismatch=$pinMismatch, firstPin=${firstPin?.let { "${it.length} chars" }}, accountEntropyPool=${accountEntropyPool?.displayValue?.censor()})"
}
}

25
feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt
View File

@ -54,9 +54,24 @@ class PinCreationViewModel(
suspend fun applyEvent(state: PinCreationState, event: PinCreationScreenEvents) {
when (event) {
is PinCreationScreenEvents.PinSubmitted -> {
_state.value = state.copy(isConfirmEnabled = false)
val result = applyPinSubmitted(state, event.pin)
_state.value = result
when {
!state.isConfirmEnabled -> {
Log.d(TAG, "[PinSubmitted] First PIN entered. Asking the user to confirm it.")
_state.value = state.copy(firstPin = event.pin, isConfirmEnabled = true, pinMismatch = false)
}
event.pin != state.firstPin -> {
Log.w(TAG, "[PinSubmitted] Confirmation PIN did not match. Returning to PIN creation.")
_state.value = state.copy(isConfirmEnabled = false, firstPin = null, pinMismatch = true)
}
else -> {
Log.d(TAG, "[PinSubmitted] Confirmation PIN matched.")
_state.value = state.copy(pinMismatch = false)
val result = applyPinSubmitted(state, event.pin)
_state.value = result
}
}
}
is PinCreationScreenEvents.ToggleKeyboard -> {
@ -68,6 +83,10 @@ class PinCreationViewModel(
is PinCreationScreenEvents.LearnMore -> {
// Handled by the navigation layer, which opens the help URL directly.
}
is PinCreationScreenEvents.BackToPinEntry -> {
_state.value = state.copy(isConfirmEnabled = false, firstPin = null, pinMismatch = false)
}
is PinCreationScreenEvents.OptOut -> {
_state.value = state.copy(isConfirmEnabled = false)
applyOptOut()

2
feature/registration/src/main/res/values/strings.xml
View File

@ -333,6 +333,8 @@
<string name="PinCreationScreen__pin_at_least_4_characters">PIN must be at least 4 characters</string>
<!-- Hint shown below the create PIN field when the user is confirming their new Signal PIN. -->
<string name="PinCreationScreen__reenter_pin">Re-enter PIN</string>
<!-- Error shown below the PIN field when the confirmation PIN does not match the one the user first entered. -->
<string name="PinCreationScreen__pins_dont_match">PINs don\'t match. Try again.</string>
<!-- Overflow menu item that opens a help article about Signal PINs. -->
<string name="PinCreationScreen__learn_more_about_pins">Learn more about PINs</string>
<!-- Title of the dialog confirming that the user wants to disable their Signal PIN. -->

74
feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationViewModelTest.kt
View File

@ -8,13 +8,17 @@ package org.signal.registration.screens.pincreation
import assertk.assertThat
import assertk.assertions.hasSize
import assertk.assertions.isEqualTo
import assertk.assertions.isFalse
import assertk.assertions.isNull
import assertk.assertions.isTrue
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.mockk
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.runTest
@ -61,17 +65,73 @@ class PinCreationViewModelTest {
Dispatchers.resetMain()
}
private fun TestScope.collectStates(): List<PinCreationState> {
val states = mutableListOf<PinCreationState>()
backgroundScope.launch(testDispatcher) { viewModel.state.collect { states.add(it) } }
return states
}
// ==================== PIN Confirmation Tests ====================
@Test
fun `first PinSubmitted enters confirm mode and does not back up`() = runTest(testDispatcher) {
val states = collectStates()
val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate())
viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("123456"))
coVerify(exactly = 0) { mockRepository.setNewlyCreatedPin(any(), any(), any<MasterKey>()) }
assertThat(emittedParentEvents).hasSize(0)
assertThat(states.last().isConfirmEnabled).isTrue()
assertThat(states.last().pinMismatch).isFalse()
}
@Test
fun `mismatched confirmation PIN returns to creation with error and does not back up`() = runTest(testDispatcher) {
val states = collectStates()
val confirmState = PinCreationState(
accountEntropyPool = AccountEntropyPool.generate(),
isConfirmEnabled = true,
firstPin = "123456"
)
viewModel.applyEvent(confirmState, PinCreationScreenEvents.PinSubmitted("999999"))
coVerify(exactly = 0) { mockRepository.setNewlyCreatedPin(any(), any(), any<MasterKey>()) }
assertThat(emittedParentEvents).hasSize(0)
assertThat(states.last().isConfirmEnabled).isFalse()
assertThat(states.last().pinMismatch).isTrue()
assertThat(states.last().firstPin).isNull()
}
@Test
fun `BackToPinEntry returns to creation step and clears the first PIN`() = runTest(testDispatcher) {
val states = collectStates()
val confirmState = PinCreationState(
accountEntropyPool = AccountEntropyPool.generate(),
isConfirmEnabled = true,
firstPin = "123456",
pinMismatch = true
)
viewModel.applyEvent(confirmState, PinCreationScreenEvents.BackToPinEntry)
assertThat(states.last().isConfirmEnabled).isFalse()
assertThat(states.last().firstPin).isNull()
assertThat(states.last().pinMismatch).isFalse()
}
// ==================== PinSubmitted Success Tests ====================
@Test
fun `PinSubmitted with valid AEP and successful SVR backup hands off to finishRegistrationOrCreateProfile`() = runTest(testDispatcher) {
fun `matching confirmation PIN with valid AEP and successful SVR backup hands off to finishRegistrationOrCreateProfile`() = runTest(testDispatcher) {
val aep = AccountEntropyPool.generate()
val initialState = PinCreationState(accountEntropyPool = aep)
val confirmState = PinCreationState(accountEntropyPool = aep, isConfirmEnabled = true, firstPin = "123456")
coEvery { mockRepository.setNewlyCreatedPin(any(), any(), any<MasterKey>()) } returns
RequestResult.Success(null)
viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("123456"))
viewModel.applyEvent(confirmState, PinCreationScreenEvents.PinSubmitted("123456"))
coVerify { mockRepository.setRestoreDecision(RestoreDecision.NEW_ACCOUNT) }
coVerify { mockRepository.finishRegistrationOrCreateProfile(parentEventEmitter, any()) }
@ -81,9 +141,9 @@ class PinCreationViewModelTest {
@Test
fun `PinSubmitted with null AEP emits ResetState`() = runTest(testDispatcher) {
val initialState = PinCreationState(accountEntropyPool = null)
val confirmState = PinCreationState(accountEntropyPool = null, isConfirmEnabled = true, firstPin = "123456")
viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("123456"))
viewModel.applyEvent(confirmState, PinCreationScreenEvents.PinSubmitted("123456"))
assertThat(emittedParentEvents).hasSize(1)
assertThat(emittedParentEvents.first()).isEqualTo(RegistrationFlowEvent.ResetState)
@ -94,12 +154,12 @@ class PinCreationViewModelTest {
@Test
fun `PinSubmitted with NotRegistered error emits ResetState`() = runTest(testDispatcher) {
val aep = AccountEntropyPool.generate()
val initialState = PinCreationState(accountEntropyPool = aep)
val confirmState = PinCreationState(accountEntropyPool = aep, isConfirmEnabled = true, firstPin = "123456")
coEvery { mockRepository.setNewlyCreatedPin(any(), any(), any<MasterKey>()) } returns
RequestResult.NonSuccess(NetworkController.BackupMasterKeyError.NotRegistered)
viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("123456"))
viewModel.applyEvent(confirmState, PinCreationScreenEvents.PinSubmitted("123456"))
assertThat(emittedParentEvents).hasSize(1)
assertThat(emittedParentEvents.first()).isEqualTo(RegistrationFlowEvent.ResetState)