Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Extract i18n strings in views/* #4695

Merged
merged 42 commits into from Mar 2, 2017
Merged

Extract i18n strings in views/* #4695

merged 42 commits into from Mar 2, 2017

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Feb 27, 2017

Part of https://github.com/ethcore/parity/issues/4704

  • Extract all i18n strings in the ~/views folder
  • Convert to icons in ~/ui/Icons as applicable

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Feb 27, 2017
Copy link
Contributor

@ngotchac ngotchac left a comment

Choose a reason for hiding this comment

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

Good stuff, thanks ! 👍

</span>
)
} }
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not having 2 spans next to each other instead of this strange pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question - the reality is that we cannot make any assumptions based on the order in any languages. This gives the flexibility to change the ordering.

Actually a real example - when we did the POC for German we ran into a number of issues like this, e.g. "the following" or "in the list below" - doesn't always make sense to go directly from the one to the other.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that's true, but here it's just an IdentityIcon, so I guess it wouldn't make sense to put it on the right in english, and on the left in german, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point yes, I must must have dozed off.

</span>
)
} }
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

<span className={ styles.detail }>
<FormattedMessage
id='wallet.details.requiredOwnersNumber'
defaultMessage='{number} owners'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have something to would be number dependant ? (owner vs. owners)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I understand the question?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, upon re-reading I get it - plurals. Good point.

@ngotchac ngotchac added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 28, 2017
@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Feb 28, 2017
@ngotchac ngotchac added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 28, 2017
@gavofyork gavofyork merged commit 5dd406a into master Mar 2, 2017
@gavofyork gavofyork deleted the jg-i18n-views branch March 2, 2017 11:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants