Page MenuHomePhabricator
Differential D13907

[native] Introduce a restore screen
Needs RevisionPublic
Actions

Authored by tomek on Tue, Nov 12, 6:15 AM.
Tags
None
Referenced Files
F3400962: Screenshot 2024-12-02 at 9.48.03 AM.png
Mon, Dec 2, 10:00 AM
Unknown Object (File)
Sun, Dec 1, 5:54 PM
Unknown Object (File)
Sat, Nov 30, 12:15 PM
F3386292: restore-prompt.png
Fri, Nov 29, 4:08 AM
Unknown Object (File)
Thu, Nov 28, 8:56 PM
Unknown Object (File)
Wed, Nov 27, 3:33 PM
Unknown Object (File)
Wed, Nov 27, 3:33 PM
Unknown Object (File)
Wed, Nov 27, 3:33 PM
View All 85 Files
Subscribers
ashoat

Details

Reviewers
angelika
bartek
ashoat
kamil
Summary

Add a new screen where a user can restore the account using a password or SIWE.

https://linear.app/comm/issue/ENG-9679/create-a-screen-where-user-can-choose-between-usernamepassword-and

The restore screen

restore-prompt.png (2×1 px, 153 KB)

The prompt screen after this diff - no changes

prompt-after-restore.png (2×1 px, 1 MB)

Depends on D14066

Test Plan

Checked that this screen looks correctly and clicking the actions navigates to appropriate screens.

Diff Detail

Repository
rCOMM Comm
Branch
restore
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek created this revision.Tue, Nov 12, 6:15 AM
Herald added a subscriber: ashoat. · View Herald TranscriptTue, Nov 12, 6:15 AM
tomek edited the summary of this revision. (Show Details)Tue, Nov 12, 6:15 AM
Harbormaster completed remote builds in B32613: Diff 45747.Tue, Nov 12, 7:07 AM
tomek requested review of this revision.Tue, Nov 12, 7:07 AM
tomek updated this revision to Diff 45750.Tue, Nov 12, 7:21 AM

Update the padding

tomek edited the summary of this revision. (Show Details)Tue, Nov 12, 7:21 AM
tomek updated this revision to Diff 45751.Tue, Nov 12, 7:22 AM

Format

ashoat added a comment.Tue, Nov 12, 7:25 AM
  1. I'm not sure the "OR" thing is necessary anymore now that we only have two buttons instead of three
  2. Can we replace "Sign in" with "Sign in with password"?
Harbormaster completed remote builds in B32615: Diff 45751.Tue, Nov 12, 7:41 AM
Harbormaster completed remote builds in B32614: Diff 45750.Tue, Nov 12, 7:50 AM
tomek updated this revision to Diff 45797.Wed, Nov 13, 7:08 AM

Update copy and remove "or" section

tomek edited the summary of this revision. (Show Details)Wed, Nov 13, 7:09 AM
Harbormaster completed remote builds in B32651: Diff 45797.Wed, Nov 13, 7:26 AM
tomek updated this revision to Diff 45840.Fri, Nov 15, 9:58 AM

Rebase

Harbormaster completed remote builds in B32678: Diff 45840.Fri, Nov 15, 10:23 AM
tomek added a child revision: D13946: [native] Update the QR login screen to mention the primary device.Fri, Nov 15, 10:24 AM
kamil accepted this revision.Tue, Nov 19, 1:33 AM
This revision is now accepted and ready to land.Tue, Nov 19, 1:33 AM
tomek removed a child revision: D13946: [native] Update the QR login screen to mention the primary device.Tue, Nov 26, 8:16 AM
tomek added a child revision: D13946: [native] Update the QR login screen to mention the primary device.Tue, Nov 26, 8:17 AM
tomek removed a child revision: D13946: [native] Update the QR login screen to mention the primary device.Tue, Nov 26, 8:20 AM
tomek updated this revision to Diff 46089.Tue, Nov 26, 8:23 AM

Rebase

tomek added a child revision: D14038: [native] Navigate from the QR screen to the restore screen.Tue, Nov 26, 8:28 AM
Harbormaster completed remote builds in B32874: Diff 46089.Tue, Nov 26, 10:03 AM
tomek edited the summary of this revision. (Show Details)Fri, Nov 29, 4:06 AM
tomek added a parent revision: D14066: [native] Extract the buttons from the logged out modal.
tomek removed a parent revision: D13906: [native] Introduce a restore button.
tomek updated this revision to Diff 46130.Fri, Nov 29, 4:07 AM

Introduce a new screen

tomek edited the summary of this revision. (Show Details)Fri, Nov 29, 4:08 AM
tomek retitled this revision from [lib] Introduce a restore screen to [native] Introduce a restore screen.
tomek added inline comments.Fri, Nov 29, 4:14 AM
native/account/restore-prompt-screen.react.js
35 ↗(On Diff #46130)

This is a kind of a hack. We're using PromptButton here and on the logged-out modal. In the modal, we're displaying the buttons inside an absolutely-positioned container that contains more containers with flex-direction row. In order for the buttons to share the space evenly, we specify flex: 1 for them. This doesn't play well with the buttons here, which are displayed in a container that shrinks - the result is that the buttons collapse and take as little space as possible. There are a couple of possible solutions:

  1. Specifying flex-shrink: 0 for buttons - this breaks the logged out modal (a row with two sign-in buttons)
  2. Avoid specifying flex: 1 for buttons - also breaks the modal
  3. Specifying flex: 1 conditional for the buttons - this works, but the fact that we would need to specify a flag which determines whether we should add flex: 1 doesn't sound great
  4. The solution from this diff, where we artificially introduce a row container for each button

I'm not sure whether the solution from this diff is the best. Maybe there are some better alternatives.

tomek requested review of this revision.Fri, Nov 29, 4:14 AM
Harbormaster completed remote builds in B32899: Diff 46130.Fri, Nov 29, 4:24 AM
tomek added inline comments.Fri, Nov 29, 10:24 AM
native/account/restore-prompt-screen.react.js
55–58 ↗(On Diff #46130)

This isn't used

tomek added a child revision: D14067: [native] Introduce a password account restore screen.Fri, Nov 29, 10:40 AM
kamil resigned from this revision.Mon, Dec 2, 1:40 AM
kamil added a reviewer: ashoat.

I think @ashoat should review this to see the user-facing copy

ashoat requested changes to this revision.Mon, Dec 2, 10:00 AM

Some quick UI requests:

  1. Would it be possible to have a graphic on this screen? I like this free SVG, but we could also pick something from @expo/vector-icons:
    Screenshot 2024-12-02 at 9.48.03 AM.png (264×594 px, 30 KB)
  2. As compared with similar registration screens, the screenshot here seems to have a lot more margin between the bottom button and the home pill. Can you try to make it look more like the screens in RegistrationNavigator?
  3. Regarding the copy, I'd like to make some updates. In the screenshot it feels like a lot of text bunched together, so I tried to split it up into multiple paragraphs. Would be curious to see if this looks good in an updated screenshot:

If you've lost access to your primary device, you can try recovering your Comm account.

To proceed, select the same login method that you used during registration.

Note that after completing the recovery flow, you will logged out of all of your other devices.

native/account/restore-prompt-screen.react.js
35 ↗(On Diff #46130)

I wonder if it would be better to implement a separate set of buttons based on PrimaryButton for this flow (that's what the RegistrationNavigator screens usually use)

Potentially we could have some shared inner component that is used by both the LoggedOutModal buttons and these buttons, but they would be wrapped in slightly different ways that work in each use case

This revision now requires changes to proceed.Mon, Dec 2, 10:00 AM
ashoat added a comment.Mon, Dec 2, 10:41 AM
In D13907#392115, @ashoat wrote:
  1. Would it be possible to have a graphic on this screen? I like this free SVG, but we could also pick something from @expo/vector-icons:
    Screenshot 2024-12-02 at 9.48.03 AM.png (264×594 px, 30 KB)

I spoke to Rahul on our team, and he's interested in helping here a bit. He's going to see if he can find or design a better graphic than the ones that I found.

Revision Contents
Changeset List

PathSize
native/
account/
56 lines

Diff 45840

View Options

native/account/logged-out-modal.react.js

Loading...