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

[CONTENT] Ledger Integration english content #5812

Merged
merged 1 commit into from
Feb 22, 2023
Merged

Conversation

gantunesr
Copy link
Member

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

Add english content for the Ledger integration.

Screenshots/Recordings

If applicable, add screenshots and/or recordings to visualize the before and after of your change

Issue

Progresses https://github.com/MetaMask/mobile-planning/issues/637

Checklist

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

@gantunesr gantunesr added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) Spot Check on the Release Build If a ticket doesn't require feature QA, but does require some form of manual spot checking No QA Needed/E2E Only Apply this label when your PR does not need any QA effort. and removed Spot Check on the Release Build If a ticket doesn't require feature QA, but does require some form of manual spot checking labels Feb 22, 2023
@gantunesr gantunesr marked this pull request as ready for review February 22, 2023 03:31
@gantunesr gantunesr requested a review from a team as a code owner February 22, 2023 03:31
Copy link

@montelaidev montelaidev left a comment

Choose a reason for hiding this comment

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

LGTM

@gantunesr gantunesr added release-6.1.0 and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Feb 22, 2023
@gantunesr gantunesr merged commit 8c63e48 into main Feb 22, 2023
@gantunesr gantunesr deleted the content/ledger branch February 22, 2023 11:51
@github-actions github-actions bot locked and limited conversation to collaborators Feb 22, 2023
Copy link
Contributor

@NicolasMassart NicolasMassart left a comment

Choose a reason for hiding this comment

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

Some comments on text style and complexity

"how_to_install_eth_app": "How to install the Ethereum app on a Ledger device",
"ledger_account_count": "You are using 1 account from your Ledger with MetaMask Mobile.",
"open_eth_app": "Please open the Ethereum app",
"open_eth_app_message_one": "We’ve detected you have the Ethereum app installed but it’s not open, please ",
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove please. This action is required to continue and not only if it pleases the user.

Suggested change
"open_eth_app_message_one": "We’ve detected you have the Ethereum app installed but it’s not open, please ",
"open_eth_app_message_one": "Ethereum app is installed but not open, please ",

"unknown_error_message": "An unexpected error occured. Please try again.",
"error_occured": "An error occured",
"how_to_install_eth_app": "How to install the Ethereum app on a Ledger device",
"ledger_account_count": "You are using 1 account from your Ledger with MetaMask Mobile.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the number hardcoded or maybe it requires some placeholder? Also if placeholder, handle plurals.

"ethereum_app_not_installed_error": "Please install the Ethereum app on your Ledger device.",
"ledger_is_locked": "Ledger is locked",
"unlock_ledger_message": "Please unlock your Ledger device",
"cannot_get_account": "Cannot get account",
Copy link
Contributor

Choose a reason for hiding this comment

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

language style matching

Suggested change
"cannot_get_account": "Cannot get account",
"cannot_get_account": "Can't get account",

"ledger_account_count": "You are using 1 account from your Ledger with MetaMask Mobile.",
"open_eth_app": "Please open the Ethereum app",
"open_eth_app_message_one": "We’ve detected you have the Ethereum app installed but it’s not open, please ",
"open_eth_app_message_two": "press the two buttons on the device to accept the prompt to open the Ethereum app to continue.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalise first word.
Sentence is too complicated. Can we simplify with like:

Suggested change
"open_eth_app_message_two": "press the two buttons on the device to accept the prompt to open the Ethereum app to continue.",
"open_eth_app_message_two": "Press both buttons on the Ledger device to open Ethereum app.",

"open_eth_app": "Please open the Ethereum app",
"open_eth_app_message_one": "We’ve detected you have the Ethereum app installed but it’s not open, please ",
"open_eth_app_message_two": "press the two buttons on the device to accept the prompt to open the Ethereum app to continue.",
"toast_bluetooth_connection_error_title": "Oops, something went wrong :/",
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that we already replaced the "oops" by something better in other prompts to make them more actionable.

"cannot_get_account": "Cannot get account",
"connect_ledger": "Connect Ledger",
"looking_for_device": "Looking for device",
"ledger_reminder_message": "Please make sure your Ledger Nano X is:",
Copy link
Contributor

Choose a reason for hiding this comment

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

We say Ledger all along, and here it's Ledger Nano X... what if we support other models later?

"available_devices": "Available devices",
"retry": "Retry",
"continue": "Continue",
"confirm_transaction_on_ledger": "Confirm transaction by pressing two buttons on your Ledger",
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
"confirm_transaction_on_ledger": "Confirm transaction by pressing two buttons on your Ledger",
"confirm_transaction_on_ledger": "Confirm transaction by pressing both buttons on your Ledger",

"bluetooth_enabled_message": "Make sure Bluetooth is enabled",
"device_unlocked_message": "Device is unlocked",
"blind_signing_message": "Blind signing is enabled",
"ledger_disconnected": "Your device got disconnected",
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
"ledger_disconnected": "Your device got disconnected",
"ledger_disconnected": "Your device is disconnected",

"ledger_disconnected": "Your device got disconnected",
"ledger_disconnected_error": "The connection to your device has been lost. Please try again.",
"unknown_error": "Unexpected error occurred.",
"unknown_error_message": "An unexpected error occured. Please try again.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Error are most of the time unexpected...

Suggested change
"unknown_error_message": "An unexpected error occured. Please try again.",
"unknown_error_message": "An unknown error occured. Please try again.",

But I would even fusion this with the error_occured one... knowing it's unexpected or unknown doesn't add anything ot the user understanding. It just makes them more anxious...

"forget_device": "Forget Ledger",
"sign_with_ledger": "Sign with Ledger",
"ledger_pending_confirmation": "Ledger is busy",
"ledger_pending_confirmation_error": "There is a pending action on your Ledger. Please clear the action first then retry.",
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
"ledger_pending_confirmation_error": "There is a pending action on your Ledger. Please clear the action first then retry.",
"ledger_pending_confirmation_error": "An action is in progress on your Ledger. Please finish or cancel the action, then retry.",

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
No QA Needed/E2E Only Apply this label when your PR does not need any QA effort. release-6.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants