Page MenuHomePhabricator

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

Authored by marcin on Apr 25 2024, 10:17 AM.
Tags
None
Referenced Files
F3358526: D11782.diff
Sun, Nov 24, 5:19 AM
Unknown Object (File)
Thu, Nov 21, 8:00 PM
Unknown Object (File)
Thu, Nov 21, 7:47 PM
Unknown Object (File)
Thu, Nov 21, 6:28 PM
Unknown Object (File)
Thu, Nov 21, 6:27 PM
Unknown Object (File)
Sun, Nov 3, 6:06 AM
Unknown Object (File)
Oct 15 2024, 1:01 AM
Unknown Object (File)
Oct 13 2024, 11:26 PM
Subscribers

Details

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

Updated screen looks like this:

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

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 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.Apr 25 2024, 3:11 PM
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.

Remove unecessary useState

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