Page MenuHomePhabricator

alert for browsers not using english language
ClosedPublic

Authored by kieran709 on Tue, Oct 12, 23:16.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC972a3813af0f: alert for browsers not using english language
Summary

related to task: T1895
Browsers that do not return 'en-' from navigators.language check will see an error prompting them to add English to avoid translation errors.

Test Plan

remove English from browser's list of languages
add any other language
npm start
navigate to settings tab
verify whether alert appears on page.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Tue, Oct 12, 23:16
bytesofman added inline comments.
web/cashtab/src/components/Configure/Configure.js
244 ↗(On Diff #30474)

Better variable name. For the way this is being used in the app, a good one would be showTranslationWarning

That way, if showTranslationWarning is false...the translation warning is not shown. Makes sense.

286 ↗(On Diff #30474)

This kind of console.log statement is important for testing, but needs to be removed for production. Only potentially useful console.log statements should go to prod.

287 ↗(On Diff #30474)

Be more descriptive with your variable names.

For example, this could be something like detectedBrowserLang

288 ↗(On Diff #30474)

Don't need this console.log for prod

291 ↗(On Diff #30474)

Same same, remove this return console.log

523 ↗(On Diff #30474)

Use this text for the alert description:

Please do not translate your seed phrase. Store your seed phrase in English. You must re-enter these exact English words to restore your wallet from seed.

This revision now requires changes to proceed.Tue, Oct 12, 23:23

responding to review feedback

bytesofman added inline comments.
web/cashtab/src/components/Configure/Configure.js
287 ↗(On Diff #30475)

Is this the actual browser code for Indonesian?

Let's go with this approach:

  • Look for navigator.language , the string, instead of navigator.languages, the array
  • Show the alert if navigator.language does not contain the substring en- ; that way stuff like en-US or en-GB is okay, but any variant that doesn't include en- would get the alert
This revision now requires changes to proceed.Tue, Oct 12, 23:37

responding to review feedback

kieran709 edited the test plan for this revision. (Show Details)
kieran709 retitled this revision from alert for browsers using indonesian language to alert for browsers not using english language.Wed, Oct 13, 02:34
This revision is now accepted and ready to land.Wed, Oct 13, 16:37