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

UI component updates #4010

Merged
merged 9 commits into from
Jan 5, 2017
Merged

UI component updates #4010

merged 9 commits into from
Jan 5, 2017

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Jan 2, 2017

  • Expand tests for some of the slightly less than straight-forward components (multi-rendering logic) - BlockStatus, ConfirmDialog, TxHash
  • Add missing tests for IdentityName/IdentityIcon where missed in bug-fix PRs (e.g. Display 0x00..00 as null #3950 & children)
  • Cleanup some pre-existing PropType warnings in ui/* tests
  • Updated strings to be i18n capable in touched components

Not critical for 1.5 merge.

@jacogr jacogr added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M7-ui A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Jan 2, 2017
: <FormattedMessage
id='ui.txHash.confirmations'
defaultMessage='{count} confirmations'
values={ { count } } />
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't there something for formatting variations based on singular/plurals/etc. ? This binary 1 vs. many won't work in some other languages

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is a more general problem with our i18n setup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is native support in react-intl, honestly I cannot quite remember how it works, but will take a look and update accordingly.

@ngotchac ngotchac added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 4, 2017
@ngotchac
Copy link
Contributor

ngotchac commented Jan 4, 2017

One small comment. Looks good otherwise.

@ngotchac ngotchac added A8-looksgood 🦄 Pull request is reviewed well. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Jan 5, 2017
@jacogr jacogr merged commit ddeb06d into master Jan 5, 2017
@jacogr jacogr deleted the jg-ui-updates branch January 5, 2017 11:06
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