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

ooni-wui 2.2.0-rc.1 #37

Merged
merged 59 commits into from Feb 21, 2017
Merged

ooni-wui 2.2.0-rc.1 #37

merged 59 commits into from Feb 21, 2017

Conversation

hellais
Copy link
Contributor

@hellais hellais commented Feb 21, 2017

I am going to use this branch to stage the changes for ooni-wui 2.2.0-rc.1

hellais and others added 30 commits February 12, 2017 16:53
* Now the uber boring part of replacing every single string with
  `FormattedMessage` 😱
* Fixes to NDT
* Fixes to WebConnectivity
Added support for translations
* agrabeli/feature/intl:
  Added support for translation of WhatsApp test
* src/components/Deck/Deck.js
* src/components/Deck/DeckContainer.js
* src/components/Notification/Notification.js
* src/components/Settings/SharingOptions.js
* src/routes/Dashboard/components/Dashboard.js
… feature/intl

* 'feature/intl' of github.com:TheTorProject/ooni-wui:
  s/defaultValue/defaultMessage/orry
  Make sure also Headers.js is i18n ready
Gotcha: JSX interprets strings in arguments (such as
defaultMessage='foo') as HTML strings and not as javascript strings.
This means that \n is not a valid HTML string and hence we need to wrap
it them in {'foo'} to make them work properly.
* Better responsiveness in navbar
"onboard.welcome.learnMore": "Learn more",
"onboard.welcome.alreadyUnderstand": "I already understand the risks, take me to my dashboard.",
"settings.title": "Settings"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the default is already English, isn't this duplicating strings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually the master translation file from which all the other translations are built.

This file is the one that I upload to transifex for translations and that people use for translating.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of defaultMessage='foo' then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifying the content inline. This file here is generated from the <FormatMessages> by means of the bin/translate.js script.

In theory I could also not commit this file and it would still work, however it's best to commit it so that we can setup transifex to pull in translations automatically from the source code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, got it! I was wondering it was a duplicate (hence, maintenance effort)

@@ -0,0 +1,3 @@
export const getLanguage = () => {
return navigator.language
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this function used? It seems I cannot find it elsewhere in the diff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good call I missed removing this in a refactor.

Copy link
Contributor

@bassosimone bassosimone left a comment

Choose a reason for hiding this comment

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

Overall looks good except for a couple of things that I asked for inline

bassosimone and others added 4 commits February 21, 2017 18:47
Copy link
Contributor

@bassosimone bassosimone left a comment

Choose a reason for hiding this comment

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

🐰 (i.e. "follow the white rabbit")

@hellais
Copy link
Contributor Author

hellais commented Feb 21, 2017

Merging!

@hellais hellais merged commit 27f17cf into master Feb 21, 2017
@hellais hellais deleted the release/v2.2.0-rc.1 branch February 21, 2017 20:09
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.

None yet

3 participants