Page MenuHomePhabricator
Differential D11782

Let the user skip backup message generation and record it in alert store
ClosedPublic
Actions

Authored by marcin on Thu, Apr 25, 10:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 2, 6:28 AM
Unknown Object (File)
Thu, May 2, 5:17 AM
Unknown Object (File)
Wed, May 1, 11:48 AM
Unknown Object (File)
Tue, Apr 30, 3:21 PM
Unknown Object (File)
Mon, Apr 29, 10:52 AM
Unknown Object (File)
Mon, Apr 29, 10:37 AM
Unknown Object (File)
Sun, Apr 28, 7:46 PM
Unknown Object (File)
Thu, Apr 25, 11:24 AM
View All 11 Files
Subscribers
ashoat
tomek

Details

Reviewers
ginsu
ashoat
Commits
rCOMMdabc6a21b8a4: Let the user skip backup message generation and record it in alert store
Summary

This differential enables user to skip siwe backup message generation. Additionally alert store is used to prevent the screen from appearing too frequently.

Test Plan
  1. Build the app
  2. Log in as SIWE user
  3. Ensure that there is a Skip button on a backup message screen.
  4. Click skip button. Ensure that screen disappears.
  5. Play with the app. Kill and re-open it multiple times. Ensure that screen doesn't appear again.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

marcin created this revision.Thu, Apr 25, 10:17 AM
Herald added a subscriber: tomek. · View Herald TranscriptThu, Apr 25, 10:17 AM
marcin added a comment.Thu, Apr 25, 10:18 AM

Updated screen looks like this:

Simulator Screenshot - iPhone 15 Pro - 2024-04-25 at 19.14.19.png (2×1 px, 134 KB)

marcin attached a referenced file: F1663625: Simulator Screenshot - iPhone 15 Pro - 2024-04-25 at 19.14.19.png. (Show Details)Thu, Apr 25, 10:18 AM
Harbormaster completed remote builds in B28475: Diff 39503.Thu, Apr 25, 10:34 AM
marcin requested review of this revision.Thu, Apr 25, 10:34 AM
ashoat resigned from this revision.Thu, Apr 25, 10:45 AM

Resigning to let @ginsu review the alert parts. Overall looks good to me, but I think the skippedByUser state can be removed. @marcin – in case you disagree with that, can you re-add me (if possible) or just delay landing until I can confirm that it's good to land?

native/account/registration/missing-registration-data/missing-siwe-backup-message.react.js
24 ↗(On Diff #39503)

Why do we need this state? Can't we just call props.navigation.goBack() directly in onSkip?

ginsu accepted this revision.Thu, Apr 25, 3:11 PM
ginsu added a subscriber: ashoat.

alert code looks good, pls make sure to respond + address @ashoat's comments before landing

This revision is now accepted and ready to land.Thu, Apr 25, 3:11 PM
marcin added a parent revision: D11781: Move MissingRegistrationDataHandler to NavigationHandler.Fri, Apr 26, 1:19 AM
marcin added a child revision: D11713: Introduce msg_backup field to BackupItem.
marcin added inline comments.Fri, Apr 26, 1:24 AM
native/account/registration/missing-registration-data/missing-siwe-backup-message.react.js
24 ↗(On Diff #39503)

Probably this state is not necessary and it is over engineering here. It looks like setSIWEBackupSecrets state is not necessary as well.

marcin updated this revision to Diff 39535.Fri, Apr 26, 6:19 AM

Remove unecessary useState

Harbormaster completed remote builds in B28500: Diff 39535.Fri, Apr 26, 6:35 AM
ashoat accepted this revision.Sat, Apr 27, 9:03 AM
ashoat added inline comments.
native/account/registration/missing-registration-data/missing-siwe-backup-message.react.js
30 ↗(On Diff #39535)

It is always good to be more specific in the dep list, so that we can avoid unnecessary recomputations

35 ↗(On Diff #39535)
41 ↗(On Diff #39535)

I wonder if you could just pass props.navigation.goBack here

native/account/registration/siwe-backup-message-creation.react.js
33 ↗(On Diff #39535)

You might need to change this to => mixed in order for the last comment to work. In general accepting => mixed is a more flexible API... you don't actually care what is returned, you just want to call the function

marcin updated this revision to Diff 39639.Mon, Apr 29, 3:35 AM

Rebase before landing

Harbormaster completed remote builds in B28566: Diff 39639.Mon, Apr 29, 3:52 AM
marcin updated this revision to Diff 39644.Mon, Apr 29, 4:24 AM

Rebase before landing

Harbormaster completed remote builds in B28570: Diff 39644.Mon, Apr 29, 4:40 AM
Closed by commit rCOMMdabc6a21b8a4: Let the user skip backup message generation and record it in alert store (authored by marcin). · Explain WhyMon, Apr 29, 4:55 AM
This revision was automatically updated to reflect the committed changes.
marcin added a commit: rCOMMdabc6a21b8a4: Let the user skip backup message generation and record it in alert store.

Revision Contents
Changeset List

PathSize
lib/
types/
2 lines
utils/
10 lines
native/
account/
registration/
missing-registration-data/
27 lines
14 lines
12 lines
redux/
20 lines

Diff 39648

View Options

lib/types/alert-types.js

Loading...
View Options

lib/utils/push-alerts.js

Loading...
View Options

native/account/registration/missing-registration-data/missing-registration-data-handler.react.js

Loading...
View Options

native/account/registration/missing-registration-data/missing-siwe-backup-message.react.js

Loading...
View Options

native/account/registration/siwe-backup-message-creation.react.js

Loading...
View Options

native/redux/persist.js

Loading...