[native] Add a new screen to display a QR code as part of a new native login flow
ClosedPublic
Actions

Authored by rohan on Aug 22 2023, 8:55 AM.

Details

Reviewers

atul
ginsu
tomek
varun

Commits

rCOMMcac59d7adeb7: [native] Add a new screen to display a QR code as part of a new native login…

Summary

This is the native counterpart of D8821. I created a new navigator because this sign in via QR code flow will inevitably have multiple 'steps' in it (like choosing a device name once the QR code is scanned), so I decided to have it done now rather than later.

One important note: The designs are not finalized, but I've spoken with @ted and I'll do another pass on the designs in a later diff once they're ready (this is tracked in ENG-4701).

Addresses ENG-4515

Depends on D8870

Test Plan

Click on the button and expect the new screen to show

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Aug 22 2023, 8:55 AM

Herald added a subscriber: ashoat. · View Herald TranscriptAug 22 2023, 8:55 AM

rohan edited the summary of this revision. (Show Details)Aug 22 2023, 8:56 AM

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

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

Harbormaster completed remote builds in B22020: Diff 30233.Aug 22 2023, 9:22 AM

rohan requested review of this revision.Aug 22 2023, 9:22 AM

rohan added a child revision: D8934: [keyserver/lib/web/native] Support external cameras scanning the QR code.Aug 24 2023, 6:53 AM

accepting, but please address inline comments before landing

native/qr-code/qr-code-screen.react.js
16 ↗	(On Diff #30233)	Was this intentional?
native/qr-code/qr-code-sign-in-navigator.react.js
36 ↗	(On Diff #30233)	If it's possible, let's use a color variable from our design system here
47 ↗	(On Diff #30233)	Was this intentional?

This revision is now accepted and ready to land.Aug 24 2023, 9:36 AM

rohan added inline comments.Aug 24 2023, 10:03 AM

native/qr-code/qr-code-screen.react.js
16 ↗	(On Diff #30233)	Yeah the component needs to accept the `navigation` and `route` props, but I don't use them yet
native/qr-code/qr-code-sign-in-navigator.react.js
47 ↗	(On Diff #30233)	Yeah same thing as above!

Use a color from the design system

This revision was landed with ongoing or failed builds.Aug 24 2023, 10:05 AM

Closed by commit rCOMMcac59d7adeb7: [native] Add a new screen to display a QR code as part of a new native login… (authored by rohan). · Explain Why

This revision was automatically updated to reflect the committed changes.

rohan added a commit: rCOMMcac59d7adeb7: [native] Add a new screen to display a QR code as part of a new native login….

Harbormaster completed remote builds in B22059: Diff 30281.Aug 24 2023, 10:20 AM

I don't love how we're mixing modal and panel families here. I know they are separate files, but it still feels like a violation of the design system (unless I'm missing something?)

cc @ted