Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Find all strings for i18n - Closes #557 #759

Merged
merged 36 commits into from
Sep 21, 2017

Conversation

yasharAyari
Copy link
Contributor

Closes #557

Copy link
Contributor

@slaweet slaweet left a comment

Choose a reason for hiding this comment

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

Thanks, @yasharAyari .

Please see some comments below.

Also, there should be an e2e test for switching language.

@@ -21,7 +22,7 @@ class ForgingTitle extends React.Component {
{this.props.account.delegate.username}
</h2>
<span>
<LiskAmount val={this.props.statistics.total} roundTo={2} /> LSK Earned
<LiskAmount val={this.props.statistics.total} roundTo={2} /> LSK {this.props.t('Earned')}
Copy link
Contributor

Choose a reason for hiding this comment

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

LSK should be part of the translated string {this.props.t('LSK Earned') because other languages can have a different word order.

className='verify-message'
onClick={() => props.setActiveDialog({
title: 'Verify message',
title: props.t('verify-message'),
Copy link
Contributor

Choose a reason for hiding this comment

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

This key should be the same as the one two lines ago - 'Verify message'

<IconMenu
className={`${styles.iconButton} ${offlineStyle.disableWhenOffline}`}
icon='language' position='topRight'>
<MenuItem caption='english' onClick={() => i18n.changeLanguage('en')} />
Copy link
Contributor

Choose a reason for hiding this comment

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

The first letter of caption should be capitalized caption='English'

className={`${styles.iconButton} ${offlineStyle.disableWhenOffline}`}
icon='language' position='topRight'>
<MenuItem caption='english' onClick={() => i18n.changeLanguage('en')} />
<MenuItem caption='deutsch' onClick={() => i18n.changeLanguage('de')} />
Copy link
Contributor

Choose a reason for hiding this comment

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

The first letter of caption should be capitalized caption='Deutsch'

@@ -1,4 +1,49 @@
{
"send": "send",
Copy link
Contributor

Choose a reason for hiding this comment

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

S in send should be capitalized: "send": "Send", It's the same on the button, but different as the send modal title.

<span className='title'>
{ cardObj.label === 'Last 24 hours' ?
this.props.t(cardObj.label) :
this.props.t('Last x days', { day: cardObj.label }) }
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition is ugly and hacky. I would move the definition of statCardObjects inside ForgingStats.render so that it can use this.props.tand here the code can stay the way it was.

Copy link
Contributor

Choose a reason for hiding this comment

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

The key should be Last {{day}} days for better readability. Or maybe Last {{count}} days as well as the value.

@@ -64,6 +65,12 @@ const Header = props => (
title: props.t('send'),
childComponent: Send,
})}>{props.t('send')}</Button>
<IconMenu
className={`${styles.iconButton} ${offlineStyle.disableWhenOffline}`}
icon='language' position='topRight'>
Copy link
Contributor

Choose a reason for hiding this comment

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

The IconMenu can use selectable and selected properties to indicate selected language.
http://react-toolbox.com/#/components/menu

slaweet
slaweet previously approved these changes Sep 20, 2017
@slaweet slaweet self-assigned this Sep 21, 2017
@slaweet slaweet merged commit 4a7d73f into development Sep 21, 2017
@slaweet slaweet deleted the 557-find-all-strings-for-i18n branch September 21, 2017 10:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants