Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/account backup alert flows #1681

Merged
merged 29 commits into from
Jul 30, 2020

Conversation

estebanmino
Copy link
Contributor

@estebanmino estebanmino commented Jul 7, 2020

Description

Collection of PRs related to backup alerts

https://trello.com/c/oioLEgpq/131-warn-me-if-i-want-to-sync-a-different-wallet
https://trello.com/c/yyGN7hg5/135-prompt-me-to-save-my-seedphrase-in-different-scenarios
https://trello.com/c/Nd8wuBUH/134-prompt-me-to-create-a-password-if-i-dont-have-one

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Resolves #???

* Warn me if I want to sync a different wallet

* remove existing user check from importwallet

* locales
* send to choosepassword

* gotowallet

* protect wallet modal

* text

* working

* decrypt with empty password

* hash commit
* Warn me if I want to sync a different wallet

* remove existing user check from importwallet

* locales

* update alert

* update alert

* own component

* redux

* trigger modal everywhere

* show modal only if seedphare not backed

* add more cases

* onLearnMore

* locales

* show border only if vertical
@estebanmino estebanmino marked this pull request as ready for review July 8, 2020 16:56
@estebanmino estebanmino requested a review from a team as a code owner July 8, 2020 16:56
@estebanmino estebanmino added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Jul 8, 2020
@omnat
Copy link
Contributor

omnat commented Jul 9, 2020

@andrepimenta this PR should be merged after the new bundle of seedphrase PRs is merged
cc @estebanmino @rickycodes

@ibrahimtaveras00 ibrahimtaveras00 added QA in Progress QA has started on the feature. and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Jul 10, 2020
Copy link
Member

@rickycodes rickycodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let a few small suggestions, but this looks good to me so +1

{displayCancelButton && (
<StyledButton
testID={cancelTestID}
type={cancelButtonMode}
onPress={onCancelPress}
containerStyle={[styles.button, displayConfirmButton ? styles.cancel : {}]}
containerStyle={[styles.button, verticalButtons ? {} : styles.buttonHorizontal]}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of using a ternary here, could we do?:

!verticalButtons && styles.buttonHorizontal

@@ -83,7 +89,7 @@ export default function ActionContent({
testID={confirmTestID}
type={confirmButtonMode}
onPress={onConfirmPress}
containerStyle={[styles.button, displayCancelButton ? styles.confirm : {}]}
containerStyle={[styles.button, verticalButtons ? {} : styles.buttonHorizontal]}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

<Text style={styles.protectWalletContent}>
{!this.props.passwordSet
? strings('protect_your_wallet_modal.body_for_password')
: strings('protect_your_wallet_modal.body_for_seedphrase')}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could instead be:

{strings(`protect_your_wallet_modal.body_for_${!this.props.passwordSet ? 'password' : 'seedphrase' }`)}

while this is a bit more DRY it might be easier to read the other way?

@ibrahimtaveras00 ibrahimtaveras00 added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed QA in Progress QA has started on the feature. labels Jul 16, 2020
@ibrahimtaveras00 ibrahimtaveras00 added QA in Progress QA has started on the feature. and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Jul 28, 2020
@ibrahimtaveras00 ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels Jul 30, 2020
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All fixes look good, QA Passed 👍

@ibrahimtaveras00 ibrahimtaveras00 merged commit 59f8237 into develop Jul 30, 2020
@ibrahimtaveras00 ibrahimtaveras00 deleted the feature/account-backup-flows branch July 30, 2020 02:21
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
* Warn if different wallet (#1677)

* Warn me if I want to sync a different wallet

* remove existing user check from importwallet

* locales

* Create password if not set (#1678)

* send to choosepassword

* gotowallet

* protect wallet modal

* text

* working

* decrypt with empty password

* hash commit

* Feature/backup seedphrase (#1680)

* Warn me if I want to sync a different wallet

* remove existing user check from importwallet

* locales

* update alert

* update alert

* own component

* redux

* trigger modal everywhere

* show modal only if seedphare not backed

* add more cases

* onLearnMore

* locales

* show border only if vertical

* changes

* comments

* show modal

* add warning

* Device.isIphoneX

* snaps

* AccountBackupStep1

* skip modal

* create password

* padding horizontal

* snaps

* enabled|

* fix init

* fix create password

* modal

* WarningExistingUserModal

* snaps

* AddressQRCode

* onSecureWalletModalAction

* bool

* touchable

* fix?

* snaps

* if passwordset

* protectWalletModalNotVisible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release QA Passed A successful QA run through has been done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants