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/warn when replacing #2176

Merged
merged 42 commits into from
Feb 11, 2021
Merged

Feature/warn when replacing #2176

merged 42 commits into from
Feb 11, 2021

Conversation

rickycodes
Copy link
Member

@rickycodes rickycodes commented Jan 28, 2021

Description

Adequately warn user when replacing wallet:

Screen.record.from.2021-01-27.20.01.49.mp4

Checklist

  • There is a related GitHub issue

Issue

Resolves #2115

@rickycodes rickycodes requested a review from a team as a code owner January 28, 2021 01:06
@rickycodes rickycodes added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Jan 28, 2021
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.

  • Issue 1:
    The wording seems a little bit incomplete where we say "setup new"

image

Maybe "and setup a new wallet" or "and setup a new one"
cc: @cjeria @omnat

  • Issue 2:
    Mispelling

image

Should be "permanently"

  • Issue 3:
    On a smaller screen the modal is stretched out (example iPhone 5s)

Screen Shot 2021-01-29 at 6 04 31 PM

  • Issue 4:
    Delete confirmation doesn't pop up on iOS; seen here = http://recordit.co/A0w226up6u

  • Issue 5:
    In the figma mock up, it states to re-use the 'Creating wallet' loading screen from onboarding flow, and then it says not to allow going back on the onboarding carousel from here. Also I didn't see the notification that my wallet was erased. All 3 are not implemented; seen here = http://recordit.co/DeEz3hGhIh

Screen Shot 2021-01-29 at 6 17 43 PM

@ibrahimtaveras00 ibrahimtaveras00 added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Jan 29, 2021
@rickycodes rickycodes force-pushed the feature/warn-when-replacing branch 2 times, most recently from 834e607 to c7b3750 Compare February 2, 2021 18:17
@cjeria
Copy link

cjeria commented Feb 2, 2021

Issue 1: Let's go with "Can’t log in? You can ERASE your current wallet and set up a new one".

Issue 3: Let's try reducing the left and right margins of the modal so all sides have equal spacing (the area around the modal showing the background). Experiment with 8px, 16px or 24px to see what looks best.
image

@rickycodes rickycodes force-pushed the feature/warn-when-replacing branch 4 times, most recently from dba806f to 70a5ed6 Compare February 4, 2021 19:37
@rickycodes rickycodes added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Feb 10, 2021

constructor(props) {
super(props);
this.steps = props.navigation.getParam('steps', undefined);
props.navigation.setParams({
headerLeft: <View />
});
Copy link
Member Author

Choose a reason for hiding this comment

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

this is just me undoing a previous silly thing i did.

Copy link
Contributor

@estebanmino estebanmino left a comment

Choose a reason for hiding this comment

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

👌

Copy link
Contributor

@estebanmino estebanmino left a comment

Choose a reason for hiding this comment

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

lgtm

@omnat omnat removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Feb 11, 2021
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.

QA Passed 👍🏽

@ibrahimtaveras00 ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Feb 11, 2021
@rickycodes rickycodes merged commit 9701249 into develop Feb 11, 2021
@rickycodes rickycodes deleted the feature/warn-when-replacing branch February 11, 2021 19:54
estebanmino added a commit that referenced this pull request Feb 16, 2021
* Remove best deals badge from WYRE transfers. (#2228)

Co-authored-by: Shivam Chaturvedi <corsairshivam@gmail.com>

* Handle `balanceError` case (#2148)

* Handle `balanceError` case

* Adjust error case

* Update handling balanceError

* Remove etherscan link

* Add metamask-controllers from disk

* Add translation strings

* Remove etherscanUrl

* Revert "Add metamask-controllers from disk"

This reverts commit 75ac0c68b99e5bf2ff190c20cccad4df74a8806f.

* Only use this style for error case

* contract metadata images bump (#2233)

* contractmetadata

* snaps

* remotesvg

* Feature/warn when replacing (#2176)

* mock login changes

* mock warning modal

* mock delete modal

* mock delete warning and submitDelete

* Add isTextDelete

* Add autoFocus

* clean up

* "delete"

* undo design changes

* Add en translations

* Update unit tests

* fix

* Make this async await and catch and log failure

* Remove 12-word

* Adjust lineHeight

* fix typo

* Add logic to show delete notification

* Hide back button

* Disable hardware back button press

* Add notification

* Add closeButtonDisabled functionality

* Update styles

* Hide back button better on Congratulations screen

* Update translation

* Add breakpoint for small devices

* add missing "a"

* fix warning modal

* Clean up BaseNotification component

* Remove added styles

* Remove eslint-disable-next-line

* Use InteractionManager

* Remove empty onHide

* update empty callback

* Cleanup

* Add flexDirection: 'row'

* Move styles to WarningExistingUserModal

* Dismiss keyboard

* Add strings to locales

* Simplify

* Add loading screen for delete

* Actually delete wallet

* Fix modal tap area

Co-authored-by: ricky <ricky@macbook.lan>

* v1.0.11 (#2238)

* version

* bump

* Update CHANGELOG.md

* update

* Update CHANGELOG

* Remove disclosure PR

* Fix circleci apk (#2240)

* resouceclass

* revert

* Revert "revert"

This reverts commit 60df2aa.

* changelog

* gaba

* swaps-tx

* MM_FOX_CODE

* swaptxs

* works

* fix

* TransactionController from controllers (#2243)

* TransactionController from controllers

* query

* changelog

* tests

* 577

* 577

* details

* transactionType

* token

* byaction

* allworking

* ceanup

* locales

* utils

* check

* edgecase

* test

* tests

* approvals

* filter

* more

Co-authored-by: Pedro Pablo Aste Kompen <wachunei@gmail.com>
Co-authored-by: Shivam Chaturvedi <corsairshivam@gmail.com>
Co-authored-by: ricky <ricky.miller@gmail.com>
Co-authored-by: ricky <ricky@macbook.lan>
rickycodes added a commit that referenced this pull request Feb 25, 2021
* develop: (48 commits)
  Fix input state (#2265)
  bump android versionCode (#2260)
  bugfix/use bignumber for transfer deeplinks (#2257)
  Fix account list scroll (#2256)
  TransactionController from controllers (#2243)
  Fix circleci apk (#2240)
  v1.0.11 (#2238)
  Feature/warn when replacing (#2176)
  contract metadata images bump (#2233)
  Handle `balanceError` case (#2148)
  Remove best deals badge from WYRE transfers. (#2228)
  @metamask/controllers@6.0.1 (#2217)
  Fix cloudflare redirects (#2180)
  Merge pull request from GHSA-wmvx-96jh-gcr4
  Update @metamask/contract-metadata (#2203)
  React Native update to 0.63 (#2078)
  Create dependabot.yml (#2204)
  Fix "Text strings must be rendered within a <Text> component" (#2193)
  Use navigate instead of push (#2191)
  Add fiat on ramp modal close button extra hit area (#2174)
  ...
@estebanmino estebanmino mentioned this pull request Mar 30, 2021
3 tasks
rickycodes added a commit that referenced this pull request Jan 31, 2022
* mock login changes

* mock warning modal

* mock delete modal

* mock delete warning and submitDelete

* Add isTextDelete

* Add autoFocus

* clean up

* "delete"

* undo design changes

* Add en translations

* Update unit tests

* fix

* Make this async await and catch and log failure

* Remove 12-word

* Adjust lineHeight

* fix typo

* Add logic to show delete notification

* Hide back button

* Disable hardware back button press

* Add notification

* Add closeButtonDisabled functionality

* Update styles

* Hide back button better on Congratulations screen

* Update translation

* Add breakpoint for small devices

* add missing "a"

* fix warning modal

* Clean up BaseNotification component

* Remove added styles

* Remove eslint-disable-next-line

* Use InteractionManager

* Remove empty onHide

* update empty callback

* Cleanup

* Add flexDirection: 'row'

* Move styles to WarningExistingUserModal

* Dismiss keyboard

* Add strings to locales

* Simplify

* Add loading screen for delete

* Actually delete wallet

* Fix modal tap area

Co-authored-by: ricky <ricky@macbook.lan>
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.

Warn me sufficiently when I am replacing my wallet
5 participants