Skip to content

Remove my wallet account 10342#10365

Closed
rgordon95 wants to merge 2 commits intoMetaMask:developfrom
rgordon95:remove-my-wallet-account-10342
Closed

Remove my wallet account 10342#10365
rgordon95 wants to merge 2 commits intoMetaMask:developfrom
rgordon95:remove-my-wallet-account-10342

Conversation

@rgordon95
Copy link
Copy Markdown
Contributor

@rgordon95 rgordon95 commented Feb 4, 2021 •
edited
Loading

Fixes: #10342

Explanation: Removed the code in accordance with the instructions in the ticket

Manual testing steps:

  • Go to 'Settings'
  • Click on 'Contact'
  • See the section is removed from the ui
  • see the locales and related display function are also removed

@rgordon95 rgordon95 requested a review from a team as a code owner February 4, 2021 03:52
@rgordon95 rgordon95 requested a review from darkwing February 4, 2021 03:52
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 4, 2021 •
edited
Loading

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@rgordon95
Copy link
Copy Markdown
Contributor Author

rgordon95 commented Feb 4, 2021

I have read the CLA Document and I hereby sign the CLA

Gudahtt
Gudahtt requested changes Feb 4, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the contribution! This is a good start, but there is more dead code here to delete.

Now that you've deleted the history.push(CONTACT_MY_ACCOUNTS_ROUTE) line, that route is now unreachable. The same goes for CONTACT_MY_ACCOUNTS_EDIT_ROUTE and CONTACT_MY_ACCOUNTS_VIEW_ROUTE, which are only accessible from that page. The my-accounts component directory can be deleted entirely. The logic in edit-contact and add-contact can be simplified to no longer account for whether you're on the MyAccounts page.

@rgordon95 rgordon95 force-pushed the remove-my-wallet-account-10342 branch from 84227fe to 140f4fe Compare February 6, 2021 17:56
@rgordon95
Copy link
Copy Markdown
Contributor Author

rgordon95 commented Feb 6, 2021

Will ping you when I'm finished with the changes and conflicts, should be this weekend.

@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Feb 9, 2021 •
edited
Loading

Sure, no rush! Please take whatever time you need, and feel free to ask if you'd like any help from us, or if you'd like one of us to take this over.

Lots of conflicts but they're probably all due to us updating our eslint rules to turn semicolons on (sorry about that 😅 ).

I'm going to put this back into draft for now until you've updated it. Feel free to move it back out of draft when it's ready again.

@Gudahtt Gudahtt marked this pull request as draft February 9, 2021 16:04
@BboyAkers
Copy link
Copy Markdown
Contributor

BboyAkers commented Mar 9, 2021

Is this still in the works? 😃

@BboyAkers BboyAkers mentioned this pull request Mar 21, 2021
Merged
@danjm danjm closed this in #10680 Apr 27, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Gudahtt Gudahtt Gudahtt requested changes

@darkwing darkwing Awaiting requested review from darkwing

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove "My Wallet Account" section in Settings > Contacts

3 participants

@rgordon95 @Gudahtt @BboyAkers