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

Reduce wallet page routing complexity #13068

Merged
merged 6 commits into from
May 20, 2022

Conversation

josheleonard
Copy link
Contributor

@josheleonard josheleonard commented Apr 19, 2022

Resolves brave/brave-browser#22402
Resolves brave/brave-browser#22588
Resolves brave/brave-browser#22589

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@josheleonard josheleonard self-assigned this Apr 19, 2022
@github-actions github-actions bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Apr 19, 2022
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard changed the title chore: reduce wallet page routing complexity Reduce wallet page routing complexity Apr 19, 2022
@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch from 2a70b94 to 0fec5f8 Compare April 19, 2022 20:01
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

Copy link
Contributor

@Douglashdaniel Douglashdaniel left a comment

Choose a reason for hiding this comment

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

Pulled to test locally
Found some bugs that need to be addressed.

  • Session Route is not being persisted. (example: While the wallet is locked, enter the route brave://wallet/crypto/accounts, it will redirect you to unlock the wallet. After you unlock the wallet it should direct you to brave://wallet/crypto/accounts not brave://wallet/crypto/portfolio

  • Add account button is not working on the Asset Detail page.

  • While the wallet is unlocked go to brave://wallet/crypto/portfolio/braveisawesome, if the token doesn't exist it should redirect back to brave://wallet/crypto/portfolio/ which it looks like it's trying to do, but /braveisawesome stays persisted and the Nav bar disappears.

  • Restore button isn't working on the brave://wallet/crypto/unlock page

  • Restore button isn't working on the brave://wallet/crypto/onboarding page

  • After creating a password for a new wallet it shows the backup-wallet page but the route should be brave://wallet/crypto/backup-wallet

  • After creating a wallet the skip button isn't working on the backup-wallet page

  • After creating a wallet the recovery phrase is empty on the next step.

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch from 0fec5f8 to 31a4e39 Compare April 20, 2022 00:09
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch 14 times, most recently from 78046e8 to c27cddd Compare April 21, 2022 03:42
@josheleonard
Copy link
Contributor Author

Pulled to test locally Found some bugs that need to be addressed.

  • Session Route is not being persisted. (example: While the wallet is locked, enter the route brave://wallet/crypto/accounts, it will redirect you to unlock the wallet. After you unlock the wallet it should direct you to brave://wallet/crypto/accounts not brave://wallet/crypto/portfolio
  • Add account button is not working on the Asset Detail page.
  • While the wallet is unlocked go to brave://wallet/crypto/portfolio/braveisawesome, if the token doesn't exist it should redirect back to brave://wallet/crypto/portfolio/ which it looks like it's trying to do, but /braveisawesome stays persisted and the Nav bar disappears.
  • Restore button isn't working on the brave://wallet/crypto/unlock page
  • Restore button isn't working on the brave://wallet/crypto/onboarding page
  • After creating a password for a new wallet it shows the backup-wallet page but the route should be brave://wallet/crypto/backup-wallet
  • After creating a wallet the skip button isn't working on the backup-wallet page
  • After creating a wallet the recovery phrase is empty on the next step.

Fixed in commit: c27cddd

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch 2 times, most recently from f4b36bf to 7b1736f Compare April 21, 2022 18:17
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch 3 times, most recently from 54d9cad to 2f6d211 Compare May 18, 2022 00:36
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

Copy link
Contributor

@Douglashdaniel Douglashdaniel left a comment

Choose a reason for hiding this comment

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

LGTM!

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch 3 times, most recently from 31b3f66 to 9fb9517 Compare May 18, 2022 16:46
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch from 9fb9517 to b531d7f Compare May 18, 2022 21:45
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch from b531d7f to 8e302d0 Compare May 19, 2022 17:24
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch from 8e302d0 to 84564c7 Compare May 19, 2022 20:16
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch from 84564c7 to e387ce2 Compare May 19, 2022 22:06
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore-refactor-wallet-pages-routing branch from e387ce2 to 99d445a Compare May 20, 2022 18:25
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard added this to the 1.41.x - Nightly milestone May 20, 2022
@josheleonard josheleonard merged commit c9b6715 into master May 20, 2022
@josheleonard josheleonard deleted the chore-refactor-wallet-pages-routing branch May 20, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/storybook-url Deploy storybook and provide a unique URL for each build
Projects
Archived in project
4 participants