Page MenuHomePhabricator

[native] implement QR Auth context
ClosedPublic

Authored by kamil on Dec 11 2024, 7:37 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 19, 9:29 AM
Unknown Object (File)
Sat, Mar 15, 12:21 AM
Unknown Object (File)
Sat, Mar 15, 12:21 AM
Unknown Object (File)
Sat, Mar 15, 12:21 AM
Unknown Object (File)
Sat, Mar 15, 12:20 AM
Unknown Object (File)
Feb 12 2025, 6:24 PM
Unknown Object (File)
Feb 12 2025, 6:24 PM
Unknown Object (File)
Feb 12 2025, 6:24 PM
Subscribers
None

Details

Summary

ENG-9959.

We need to wrap this logic in context so this will be available on all the screens in the flow.

This diff only introduces the context, not changing the logic - just moving it to the context.

In ENG-9968 there is an example of how this looks like

Depends on D14101

Test Plan

Test adding secondary device - should work exactly the same.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil added inline comments.
native/account/qr-auth/qr-auth-context.js
5–7 ↗(On Diff #46323)

It will have more props later in the stack

kamil published this revision for review.Dec 11 2024, 8:28 AM
kamil edited the summary of this revision. (Show Details)
native/account/qr-auth/qr-auth-context-provider.js
55 ↗(On Diff #46323)

It's hard to review this one because you're removing a bunch of stuff (such as the permissions check), and I can't tell where (or whether) it's reintroduced

native/account/qr-auth/qr-auth-context-provider.js
1 ↗(On Diff #46323)

Phabricator says that

This file was copied from native/account/qr-auth/secondary-device-qr-code-scanner.react.js.

which is partially true, but only part of the code was copied.

What is on the right side without color is what is copied, and green is what I added but this is stuff like function definition or const contextValue....

What is on the left side marked with red color is what is left in the native/account/qr-auth/secondary-device-qr-code-scanner.react.js. (thinks I didn't copy-paste).

So e.g. the permissions check is the thing that I left unchanged.

55 ↗(On Diff #46323)

Hmm.. I am not removing anything, just copy-pasting what can be copy-pasted from secondary-device-qr-code-scanner.react.js. to qr-auth-context-provider.js. I wasn't making any changes in the logic.

This should be really easy to review, but it looks like this is some Phabricator quirk that presents those changes in a not-natural manner.

Let me know if there is anything I can do to make it easier for you.

Thanks for explaining... not sure how I missed that. You'd think after 10 years of using Phabricator I'd be better at using it 😅

This revision is now accepted and ready to land.Dec 12 2024, 10:53 AM
This revision was automatically updated to reflect the committed changes.