Details

Reviewers

kamil
angelika
bartek
ashoat

Commits

rCOMM68d3861c3304: [native] Navigate from the QR screen to the restore screen

Summary

Introduce a wrapper over the logged out screen that can be used to choose between prompt and restore flows - this allows avoiding duplicating the code. Configure the wrapper to be used as two screens - the prompt and restore.

https://linear.app/comm/issue/ENG-9679/create-a-screen-where-user-can-choose-between-usernamepassword-and
https://linear.app/comm/issue/ENG-9958/introduce-a-stack-navigator-for-the-logged-out-screen

Depends on D14067

Test Plan

A couple of scenarios - details in the video from the summary.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Mon, Nov 25, 6:23 AM

Herald added a subscriber: ashoat. · View Herald TranscriptMon, Nov 25, 6:23 AM

tomek edited the summary of this revision. (Show Details)Mon, Nov 25, 6:24 AM

tomek edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B32815: Diff 46023.Mon, Nov 25, 6:41 AM

tomek requested review of this revision.Mon, Nov 25, 6:41 AM

The code changes are perfect here, and this is what we discussed doing for the UI. But after watching the video, I'm a bit concerned... it seems like it will be easy for the user to get confused and think that the QR code screen is just getting dismissed, with no changes in LoggedOutModal.

I wonder if instead, we should consider using a stack-based approach like we have with RegistrationNavigator:

Move the QR code screen (and any associated screens) to be nested inside a new stack navigator (similar to RegistrationNavigator)
Introduce a new screen for the restore flow that has the user pick between ETH and password

What do you think?

In D14038#391176, @ashoat wrote:

The code changes are perfect here, and this is what we discussed doing for the UI. But after watching the video, I'm a bit concerned... it seems like it will be easy for the user to get confused and think that the QR code screen is just getting dismissed, with no changes in LoggedOutModal.

I wonder if instead, we should consider using a stack-based approach like we have with RegistrationNavigator:

Move the QR code screen (and any associated screens) to be nested inside a new stack navigator (similar to RegistrationNavigator)

Introduce a new screen for the restore flow that has the user pick between ETH and password

What do you think?

Agree, this would be a lot more intuitive to the users. This change will take some time, but I think it is worth it.

Create a new screen with the restore flow

tomek edited the summary of this revision. (Show Details)Tue, Nov 26, 8:28 AM

tomek edited the test plan for this revision. (Show Details)

tomek edited the summary of this revision. (Show Details)

tomek added a parent revision: D13907: [native] Introduce a restore screen.

tomek removed a parent revision: D13946: [native] Update the QR login screen to mention the primary device.

Revert setting the flag

Rebase

Harbormaster completed remote builds in B32875: Diff 46090.Tue, Nov 26, 10:13 AM

Harbormaster completed remote builds in B32876: Diff 46091.Tue, Nov 26, 10:22 AM

Harbormaster completed remote builds in B32878: Diff 46093.Tue, Nov 26, 10:41 AM

tomek edited the summary of this revision. (Show Details)Wed, Nov 27, 2:12 AM

Thanks for the updated video!

Two points of feedback:

Can we avoid using the LoggedOutModal style for any of the screens in the navigator? It feels stylistically inconsistent to push a new screen that looks like what appears below the stack navigator.
I don't think it's very clear to the user that they're entering a restore flow. Can we restyle the restore screen to look something like the registration navigator screens? A title (something like "Restore account"), some text explaining what is happening, and the same buttons at the bottom but maybe using "Restore" as the verb instead of "Sign in".

What do you think?

Can we avoid using the LoggedOutModal style for any of the screens in the navigator? It feels stylistically inconsistent to push a new screen that looks like what appears below the stack navigator.

Yeah, we can do that. In that case, I'd need to introduce new components instead of wrapping the old one.

I don't think it's very clear to the user that they're entering a restore flow. Can we restyle the restore screen to look something like the registration navigator screens? A title (something like "Restore account"), some text explaining what is happening, and the same buttons at the bottom but maybe using "Restore" as the verb instead of "Sign in".

Agree, this is consistent with the first suggestion.

Can we restyle the restore screen to look something like the registration navigator screens?

I'm not sure about this. A user enters the restore flow by going through the QR screen, which is a bit inconsistent with the registration flow (e.g. font size, alignment). I think that it would be better to follow the QR screen design so that the user doesn't experience inconsistency. We can also update the QR screen design to be consistent with the registration flow - I think this is the best solution.

tomek planned changes to this revision.Fri, Nov 29, 7:09 AM