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

Transfer Wizard Static page (with minor improvements) #3066

Merged
merged 5 commits into from
Jun 20, 2023
Merged

Conversation

hcho112
Copy link
Contributor

@hcho112 hcho112 commented Jun 18, 2023

This PR intend to improve the transfer wizard experience by following changes:

  • Upgrade wallet-selector account-export package
  • Added a new static page related to transfer-wizard
  • Added another step after clean up key modal during transfer wizard
  • Minor context change on banner (more change coming soon)

Wallet-selector update:
image

Static page related changes:
image
image
image
image
image

@hcho112 hcho112 requested a review from andy-haynes June 18, 2023 23:07
@render
Copy link

render bot commented Jun 18, 2023

@netlify
Copy link

netlify bot commented Jun 18, 2023

Deploy Preview for glittering-pavlova-0e9247 ready!

Name Link
🔨 Latest commit e2183fa
🔍 Latest deploy log https://app.netlify.com/sites/glittering-pavlova-0e9247/deploys/649222febf5c4300080a168e
😎 Deploy Preview https://deploy-preview-3066--glittering-pavlova-0e9247.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Jun 18, 2023

Deploy Preview for unrivaled-zabaione-2fe19c ready!

Name Link
🔨 Latest commit e2183fa
🔍 Latest deploy log https://app.netlify.com/sites/unrivaled-zabaione-2fe19c/deploys/649222fe29a81f00080877eb
😎 Deploy Preview https://deploy-preview-3066--unrivaled-zabaione-2fe19c.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@andy-haynes andy-haynes left a comment

Choose a reason for hiding this comment

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

Couple comments inline otherwise looks good 🚀

export const TransferWizardWrapper = () => {
return (
<Container>
<h6>The following should be devised as a page on the wallet.near.org site to inform users about TW.</h6>
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a placeholder, can we remove?

return (
<Container>
<h6>The following should be devised as a page on the wallet.near.org site to inform users about TW.</h6>
<h1 >Migrating from the Near Wallet</h1>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<h1 >Migrating from the Near Wallet</h1>
<h1>Migrating from the Near Wallet</h1>

Also should it be just "Near Wallet" instead of "the Near Wallet"?

<tr>
<Td>
<p><b>Step2. Clean Up Your Keys</b></p>
<p>Next, NEAR wants to reduce the amount of apps that you&rsquo;ve shared keys with, similar to revoking access for third-parties. This may cause you to be disconnected from some apps.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a few different escape codes in here: rsquo, nbsp, and amp. Can you please double check that these are all rendering correctly and consistently with the rest of the text? Or just replace with plain characters 👍

@@ -1920,6 +1920,10 @@
"rotatedKeyTooltip": "This key will be used to transfer the account to a new wallet.",
"currentAccessKeyTooltip": "This key will be automatically deleted when transfer process is completed."
},
"cleanKeysComplete": {
"title": "Your keys have been cleaned up!",
"desc": "You will be redirect to the next step shortly to export accounts to other wallet."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"desc": "You will be redirect to the next step shortly to export accounts to other wallet."
"desc": "You will be redirected to the next step to export accounts to another wallet shortly."

@@ -116,6 +118,10 @@ const WalletMigration = ({ open, onClose }) => {
handleSetActiveView(WALLET_MIGRATION_VIEWS.MIGRATE_ACCOUNTS);
};

const natvigateToCleanKeysComplete = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

💄 should be navigateToCleanKeysComplete

@hcho112
Copy link
Contributor Author

hcho112 commented Jun 20, 2023

@andy-haynes thank you for detailed review!

@hcho112 hcho112 requested a review from andy-haynes June 20, 2023 22:11
Copy link
Contributor

@andy-haynes andy-haynes left a comment

Choose a reason for hiding this comment

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

Looks great! :shipit:

@andy-haynes andy-haynes merged commit 9eaa104 into master Jun 20, 2023
13 checks passed
@andy-haynes andy-haynes deleted the FAST-115 branch June 20, 2023 22:14
@andy-haynes andy-haynes mentioned this pull request Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants