Merge pull request #3829 from element-hq/feature/bma/verificationIteration

Verification UI / UX iteration
This commit is contained in:
Benoit Marty
2024-11-07 16:42:28 +01:00
committed by GitHub
17 changed files with 98 additions and 130 deletions

View File

@@ -80,14 +80,14 @@ fun IncomingVerificationView(
@Composable
private fun IncomingVerificationHeader(step: Step) {
val iconStyle = when (step) {
Step.Canceled,
Step.Canceled -> BigIcon.Style.AlertSolid
is Step.Initial -> BigIcon.Style.Default(CompoundIcons.LockSolid())
is Step.Verifying -> BigIcon.Style.Default(CompoundIcons.Reaction())
Step.Completed -> BigIcon.Style.SuccessSolid
Step.Failure -> BigIcon.Style.AlertSolid
}
val titleTextId = when (step) {
Step.Canceled -> CommonStrings.common_verification_cancelled
Step.Canceled -> R.string.screen_session_verification_request_failure_title
is Step.Initial -> R.string.screen_session_verification_request_title
is Step.Verifying -> when (step.data) {
is SessionVerificationData.Decimals -> R.string.screen_session_verification_compare_numbers_title
@@ -97,7 +97,7 @@ private fun IncomingVerificationHeader(step: Step) {
Step.Failure -> R.string.screen_session_verification_request_failure_title
}
val subtitleTextId = when (step) {
Step.Canceled -> R.string.screen_session_verification_cancelled_subtitle
Step.Canceled -> R.string.screen_session_verification_request_failure_subtitle
is Step.Initial -> R.string.screen_session_verification_request_subtitle
is Step.Verifying -> when (step.data) {
is SessionVerificationData.Decimals -> R.string.screen_session_verification_compare_numbers_subtitle

View File

@@ -104,6 +104,7 @@ class VerifySelfSessionPresenter @AssistedInject constructor(
fun handleEvents(event: VerifySelfSessionViewEvents) {
Timber.d("Verification user action: ${event::class.simpleName}")
when (event) {
VerifySelfSessionViewEvents.UseAnotherDevice -> stateAndDispatch.dispatchAction(StateMachineEvent.UseAnotherDevice)
VerifySelfSessionViewEvents.RequestVerification -> stateAndDispatch.dispatchAction(StateMachineEvent.RequestVerification)
VerifySelfSessionViewEvents.StartSasVerification -> stateAndDispatch.dispatchAction(StateMachineEvent.StartSasVerification)
VerifySelfSessionViewEvents.ConfirmVerification -> stateAndDispatch.dispatchAction(StateMachineEvent.AcceptChallenge)
@@ -134,6 +135,9 @@ class VerifySelfSessionPresenter @AssistedInject constructor(
isLastDevice = encryptionService.isLastDevice.value
)
}
VerifySelfSessionStateMachine.State.UseAnotherDevice -> {
VerifySelfSessionState.Step.UseAnotherDevice
}
StateMachineState.RequestingVerification,
StateMachineState.StartingSasVerification,
StateMachineState.SasVerificationStarted,

View File

@@ -26,6 +26,7 @@ data class VerifySelfSessionState(
// FIXME canEnterRecoveryKey value is never read.
data class Initial(val canEnterRecoveryKey: Boolean, val isLastDevice: Boolean = false) : Step
data object UseAnotherDevice : Step
data object Canceled : Step
data object AwaitingOtherDeviceResponse : Step
data object Ready : Step

View File

@@ -38,12 +38,14 @@ class VerifySelfSessionStateMachine @Inject constructor(
init {
spec {
inState<State.Initial> {
on { _: Event.UseAnotherDevice, state ->
state.override { State.UseAnotherDevice.andLogStateChange() }
}
}
inState<State.UseAnotherDevice> {
on { _: Event.RequestVerification, state ->
state.override { State.RequestingVerification.andLogStateChange() }
}
on { _: Event.StartSasVerification, state ->
state.override { State.StartingSasVerification.andLogStateChange() }
}
}
inState<State.RequestingVerification> {
onEnterEffect {
@@ -64,9 +66,6 @@ class VerifySelfSessionStateMachine @Inject constructor(
}
}
inState<State.Canceled> {
on { _: Event.RequestVerification, state ->
state.override { State.RequestingVerification.andLogStateChange() }
}
on { _: Event.Reset, state ->
state.override { State.Initial.andLogStateChange() }
}
@@ -119,6 +118,7 @@ class VerifySelfSessionStateMachine @Inject constructor(
on { _: Event.Cancel, state: MachineState<State> ->
when (state.snapshot) {
State.Initial, State.Completed, State.Canceled -> state.noChange()
State.UseAnotherDevice -> state.override { State.Initial.andLogStateChange() }
// For some reason `cancelVerification` is not calling its delegate `didCancel` method so we don't pass from
// `Canceling` state to `Canceled` automatically anymore
else -> {
@@ -144,6 +144,9 @@ class VerifySelfSessionStateMachine @Inject constructor(
/** The initial state, before verification started. */
data object Initial : State
/** Let the user know that they need to get ready on their other session. */
data object UseAnotherDevice : State
/** Waiting for verification acceptance. */
data object RequestingVerification : State
@@ -175,6 +178,9 @@ class VerifySelfSessionStateMachine @Inject constructor(
}
sealed interface Event {
/** User wants to use another session. */
data object UseAnotherDevice : Event
/** Request verification. */
data object RequestVerification : Event

View File

@@ -56,6 +56,9 @@ open class VerifySelfSessionStateProvider : PreviewParameterProvider<VerifySelfS
aVerifySelfSessionState(
step = Step.Skipped
),
aVerifySelfSessionState(
step = Step.UseAnotherDevice
),
// Add other state here
)
}

View File

@@ -66,7 +66,9 @@ fun VerifySelfSessionView(
fun cancelOrResetFlow() {
when (step) {
is Step.Canceled -> state.eventSink(VerifySelfSessionViewEvents.Reset)
is Step.AwaitingOtherDeviceResponse, Step.Ready -> state.eventSink(VerifySelfSessionViewEvents.Cancel)
is Step.AwaitingOtherDeviceResponse,
Step.UseAnotherDevice,
Step.Ready -> state.eventSink(VerifySelfSessionViewEvents.Cancel)
is Step.Verifying -> {
if (!step.state.isLoading()) {
state.eventSink(VerifySelfSessionViewEvents.DeclineVerification)
@@ -159,7 +161,9 @@ fun VerifySelfSessionView(
private fun VerifySelfSessionHeader(step: Step) {
val iconStyle = when (step) {
Step.Loading -> error("Should not happen")
is Step.Initial, Step.AwaitingOtherDeviceResponse -> BigIcon.Style.Default(CompoundIcons.LockSolid())
is Step.Initial -> BigIcon.Style.Default(CompoundIcons.LockSolid())
Step.UseAnotherDevice -> BigIcon.Style.Default(CompoundIcons.Devices())
Step.AwaitingOtherDeviceResponse -> BigIcon.Style.Default(CompoundIcons.Devices())
Step.Canceled -> BigIcon.Style.AlertSolid
Step.Ready, is Step.Verifying -> BigIcon.Style.Default(CompoundIcons.Reaction())
Step.Completed -> BigIcon.Style.SuccessSolid
@@ -167,8 +171,10 @@ private fun VerifySelfSessionHeader(step: Step) {
}
val titleTextId = when (step) {
Step.Loading -> error("Should not happen")
is Step.Initial, Step.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_title
Step.Canceled -> CommonStrings.common_verification_cancelled
is Step.Initial -> R.string.screen_identity_confirmation_title
Step.UseAnotherDevice -> R.string.screen_session_verification_use_another_device_title
Step.AwaitingOtherDeviceResponse -> R.string.screen_session_verification_waiting_another_device_title
Step.Canceled -> CommonStrings.common_verification_failed
Step.Ready -> R.string.screen_session_verification_compare_emojis_title
Step.Completed -> R.string.screen_identity_confirmed_title
is Step.Verifying -> when (step.data) {
@@ -179,8 +185,10 @@ private fun VerifySelfSessionHeader(step: Step) {
}
val subtitleTextId = when (step) {
Step.Loading -> error("Should not happen")
is Step.Initial, Step.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_subtitle
Step.Canceled -> R.string.screen_session_verification_cancelled_subtitle
is Step.Initial -> R.string.screen_identity_confirmation_subtitle
Step.UseAnotherDevice -> R.string.screen_session_verification_use_another_device_subtitle
Step.AwaitingOtherDeviceResponse -> R.string.screen_session_verification_waiting_another_device_subtitle
Step.Canceled -> R.string.screen_session_verification_failed_subtitle
Step.Ready -> R.string.screen_session_verification_ready_subtitle
Step.Completed -> R.string.screen_identity_confirmed_subtitle
is Step.Verifying -> when (step.data) {
@@ -252,7 +260,7 @@ private fun VerifySelfSessionBottomMenu(
Button(
modifier = Modifier.fillMaxWidth(),
text = stringResource(R.string.screen_identity_use_another_device),
onClick = { eventSink(VerifySelfSessionViewEvents.RequestVerification) },
onClick = { eventSink(VerifySelfSessionViewEvents.UseAnotherDevice) },
)
}
Button(
@@ -267,18 +275,26 @@ private fun VerifySelfSessionBottomMenu(
)
}
}
is Step.UseAnotherDevice -> {
VerificationBottomMenu {
Button(
modifier = Modifier.fillMaxWidth(),
text = stringResource(CommonStrings.action_start_verification),
onClick = { eventSink(VerifySelfSessionViewEvents.RequestVerification) },
)
// Placeholder so the 1st button keeps its vertical position
Spacer(modifier = Modifier.height(40.dp))
}
}
is Step.Canceled -> {
VerificationBottomMenu {
Button(
modifier = Modifier.fillMaxWidth(),
text = stringResource(R.string.screen_session_verification_positive_button_canceled),
onClick = { eventSink(VerifySelfSessionViewEvents.RequestVerification) },
)
TextButton(
modifier = Modifier.fillMaxWidth(),
text = stringResource(CommonStrings.action_cancel),
text = stringResource(CommonStrings.action_done),
onClick = onCancelClick,
)
// Placeholder so the 1st button keeps its vertical position
Spacer(modifier = Modifier.height(40.dp))
}
}
is Step.Ready -> {
@@ -302,6 +318,7 @@ private fun VerifySelfSessionBottomMenu(
text = stringResource(R.string.screen_identity_waiting_on_other_device),
onClick = {},
showProgress = true,
enabled = false,
)
// Placeholder so the 1st button keeps its vertical position
Spacer(modifier = Modifier.height(40.dp))

View File

@@ -8,6 +8,7 @@
package io.element.android.features.verifysession.impl.outgoing
sealed interface VerifySelfSessionViewEvents {
data object UseAnotherDevice : VerifySelfSessionViewEvents
data object RequestVerification : VerifySelfSessionViewEvents
data object StartSasVerification : VerifySelfSessionViewEvents
data object ConfirmVerification : VerifySelfSessionViewEvents

View File

@@ -35,6 +35,10 @@
<string name="screen_session_verification_request_title">"Verification requested"</string>
<string name="screen_session_verification_they_dont_match">"They dont match"</string>
<string name="screen_session_verification_they_match">"They match"</string>
<string name="screen_session_verification_use_another_device_subtitle">"Make sure you have the app open in the other device before starting verification from here."</string>
<string name="screen_session_verification_use_another_device_title">"Open the app on another verified device"</string>
<string name="screen_session_verification_waiting_another_device_subtitle">"You should see a popup on the other device. Start the verification from there now."</string>
<string name="screen_session_verification_waiting_another_device_title">"Start verification on the other device"</string>
<string name="screen_session_verification_waiting_to_accept_subtitle">"Accept the request to start the verification process in your other session to continue."</string>
<string name="screen_session_verification_waiting_to_accept_title">"Waiting to accept request"</string>
<string name="screen_signout_in_progress_dialog_content">"Signing out…"</string>

View File

@@ -7,10 +7,7 @@
package io.element.android.features.verifysession.impl.outgoing
import app.cash.molecule.RecompositionMode
import app.cash.molecule.moleculeFlow
import app.cash.turbine.ReceiveTurbine
import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import io.element.android.features.logout.api.LogoutUseCase
import io.element.android.features.logout.test.FakeLogoutUseCase
@@ -33,6 +30,7 @@ import io.element.android.tests.testutils.WarmUpRule
import io.element.android.tests.testutils.lambda.lambdaError
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value
import io.element.android.tests.testutils.test
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.junit.Rule
@@ -48,9 +46,7 @@ class VerifySelfSessionPresenterTest {
val presenter = createVerifySelfSessionPresenter(
service = unverifiedSessionService(),
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
awaitItem().run {
assertThat(step).isEqualTo(Step.Initial(false))
assertThat(displaySkipButton).isTrue()
@@ -65,9 +61,7 @@ class VerifySelfSessionPresenterTest {
service = unverifiedSessionService(),
buildMeta = buildMeta,
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
assertThat(awaitItem().displaySkipButton).isFalse()
}
}
@@ -83,9 +77,7 @@ class VerifySelfSessionPresenterTest {
emitRecoveryState(RecoveryState.INCOMPLETE)
}
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
assertThat(awaitItem().step).isEqualTo(Step.Initial(true))
resetLambda.assertions().isCalledOnce().with(value(true))
}
@@ -100,9 +92,7 @@ class VerifySelfSessionPresenterTest {
emitRecoveryState(RecoveryState.INCOMPLETE)
}
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
assertThat(awaitItem().step).isEqualTo(Step.Initial(canEnterRecoveryKey = true, isLastDevice = true))
}
}
@@ -114,43 +104,17 @@ class VerifySelfSessionPresenterTest {
startVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
requestVerificationAndAwaitVerifyingState(service)
}
}
@Test
fun `present - Handles startSasVerification`() = runTest {
val service = unverifiedSessionService(
startVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.step).isEqualTo(Step.Initial(false))
initialState.eventSink(VerifySelfSessionViewEvents.StartSasVerification)
// Await for other device response:
assertThat(awaitItem().step).isEqualTo(Step.AwaitingOtherDeviceResponse)
service.emitVerificationFlowState(VerificationFlowState.DidStartSasVerification)
// ChallengeReceived:
service.emitVerificationFlowState(VerificationFlowState.DidReceiveVerificationData(SessionVerificationData.Emojis(emptyList())))
val verifyingState = awaitItem()
assertThat(verifyingState.step).isInstanceOf(Step.Verifying::class.java)
}
}
@Test
fun `present - Cancellation on initial state does nothing`() = runTest {
val presenter = createVerifySelfSessionPresenter(
service = unverifiedSessionService(),
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val initialState = awaitItem()
assertThat(initialState.step).isEqualTo(Step.Initial(false))
val eventSink = initialState.eventSink
@@ -167,9 +131,7 @@ class VerifySelfSessionPresenterTest {
approveVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val state = requestVerificationAndAwaitVerifyingState(service)
state.eventSink(VerifySelfSessionViewEvents.ConfirmVerification)
// Cancelling
@@ -186,9 +148,8 @@ class VerifySelfSessionPresenterTest {
requestVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
awaitItem().eventSink(VerifySelfSessionViewEvents.UseAnotherDevice)
awaitItem().eventSink(VerifySelfSessionViewEvents.RequestVerification)
service.emitVerificationFlowState(VerificationFlowState.DidFail)
assertThat(awaitItem().step).isInstanceOf(Step.AwaitingOtherDeviceResponse::class.java)
@@ -204,9 +165,7 @@ class VerifySelfSessionPresenterTest {
cancelVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val state = requestVerificationAndAwaitVerifyingState(service)
state.eventSink(VerifySelfSessionViewEvents.Cancel)
assertThat(awaitItem().step).isEqualTo(Step.Canceled)
@@ -220,35 +179,13 @@ class VerifySelfSessionPresenterTest {
startVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
requestVerificationAndAwaitVerifyingState(service)
service.emitVerificationFlowState(VerificationFlowState.DidReceiveVerificationData(SessionVerificationData.Emojis(emptyList())))
ensureAllEventsConsumed()
}
}
@Test
fun `present - Restart after cancellation returns to requesting verification`() = runTest {
val service = unverifiedSessionService(
requestVerificationLambda = { },
startVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val state = requestVerificationAndAwaitVerifyingState(service)
service.emitVerificationFlowState(VerificationFlowState.DidCancel)
assertThat(awaitItem().step).isEqualTo(Step.Canceled)
state.eventSink(VerifySelfSessionViewEvents.RequestVerification)
// Went back to requesting verification
assertThat(awaitItem().step).isEqualTo(Step.AwaitingOtherDeviceResponse)
cancelAndIgnoreRemainingEvents()
}
}
@Test
fun `present - Go back after cancellation returns to initial state`() = runTest {
val service = unverifiedSessionService(
@@ -256,9 +193,7 @@ class VerifySelfSessionPresenterTest {
startVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val state = requestVerificationAndAwaitVerifyingState(service)
service.emitVerificationFlowState(VerificationFlowState.DidCancel)
assertThat(awaitItem().step).isEqualTo(Step.Canceled)
@@ -280,9 +215,7 @@ class VerifySelfSessionPresenterTest {
approveVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val state = requestVerificationAndAwaitVerifyingState(
service,
SessionVerificationData.Emojis(emojis)
@@ -307,9 +240,7 @@ class VerifySelfSessionPresenterTest {
declineVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val state = requestVerificationAndAwaitVerifyingState(service)
state.eventSink(VerifySelfSessionViewEvents.DeclineVerification)
assertThat(awaitItem().step).isEqualTo(
@@ -330,9 +261,7 @@ class VerifySelfSessionPresenterTest {
startVerificationLambda = { },
)
val presenter = createVerifySelfSessionPresenter(service)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
val state = requestVerificationAndAwaitVerifyingState(service)
state.eventSink(VerifySelfSessionViewEvents.SkipVerification)
assertThat(awaitItem().step).isEqualTo(Step.Skipped)
@@ -352,9 +281,7 @@ class VerifySelfSessionPresenterTest {
service = service,
showDeviceVerifiedScreen = true,
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
assertThat(awaitItem().step).isEqualTo(Step.Completed)
}
}
@@ -372,9 +299,7 @@ class VerifySelfSessionPresenterTest {
service = service,
showDeviceVerifiedScreen = false,
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
skipItems(1)
assertThat(awaitItem().step).isEqualTo(Step.Skipped)
}
@@ -394,9 +319,7 @@ class VerifySelfSessionPresenterTest {
service,
logoutUseCase = FakeLogoutUseCase(signOutLambda)
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
presenter.test {
skipItems(1)
val initialItem = awaitItem()
initialItem.eventSink(VerifySelfSessionViewEvents.SignOut)
@@ -414,6 +337,9 @@ class VerifySelfSessionPresenterTest {
): VerifySelfSessionState {
var state = awaitItem()
assertThat(state.step).isEqualTo(Step.Initial(false))
state.eventSink(VerifySelfSessionViewEvents.UseAnotherDevice)
state = awaitItem()
assertThat(state.step).isEqualTo(Step.UseAnotherDevice)
state.eventSink(VerifySelfSessionViewEvents.RequestVerification)
// Await for other device response:
fakeService.emitVerificationFlowState(VerificationFlowState.DidAcceptVerificationRequest)