Fix #9 add localization to login component using react-intl package #22
Conversation
jessemao
changed the title
| } | ||
| } | ||
|
|
||
| Input.propTypes = { |
There was a problem hiding this comment.
Can you use the static PropType syntax here
| render() { | ||
| const {theme, username, password, remember, onUsernameChange, onPasswordChange, onRememberToggle, onLogin, locale} = this.props; | ||
| return ( | ||
| <IntlProvider locale={locale} messages={getLocaleMessage(locale)}> |
There was a problem hiding this comment.
I feel wrapping every component with localization wrapper feels like an overkill. Instead, can we do this once is App and use redux store to save the locale and use connect to listen to it
There was a problem hiding this comment.
Agree, putting this in Redux is a good solution because we can dynamically load it from the server.
| onUsernameChange: PropTypes.func, | ||
| onPasswordChange: PropTypes.func, | ||
| onRememberToggle: PropTypes.func, | ||
| onLogin: PropTypes.func, |
There was a problem hiding this comment.
All these are required properties?
| @@ -0,0 +1,11 @@ | |||
| export default { | |||
There was a problem hiding this comment.
Can we make these json files?
| "react-redux": "^4.4.6", | ||
| "react-router": "^3.0.0", | ||
| "redux": "^3.6.0", | ||
| "redux-thunk": "^2.1.0" | ||
| } |
There was a problem hiding this comment.
Why do we have all of these external dependencies all of a sudden? We shouldn't need any of these... Specifically, our only external dependency would be React, but we webpack everything up when we build, so our exports can be used anywhere regardless of environment.
| import { | ||
| intlShape, | ||
| injectIntl | ||
| } from 'react-intl'; |
There was a problem hiding this comment.
Not really a fan of this style except when you are importing a lot of items. We should decide if we like this and consider allowing/disallowing it in our linting rules.
| } | ||
| } | ||
|
|
||
| Input.propTypes = { |
There was a problem hiding this comment.
This should be static propTypes = {...} inside the class.
| placeholder: PropTypes.object, | ||
| }; | ||
|
|
||
| Input.defaultProps = { |
There was a problem hiding this comment.
static defaultProps = {..}
| @@ -0,0 +1,148 @@ | |||
| import React, { Component, PropTypes } from 'react'; | |||
There was a problem hiding this comment.
Currently not putting .jsx extensions on code. If we want to, we should be consistent about it. I do not have a strong opinion on whether we should or should not.
| import enMessage from './l10n/en_US'; | ||
|
|
||
| addLocaleData(enLocaleData); | ||
| addLocaleData(zhLocaleData); |
There was a problem hiding this comment.
Are we bundling all of the locale data inside webpack? Won't this make our exports huge?
| render() { | ||
| const {theme, username, password, remember, onUsernameChange, onPasswordChange, onRememberToggle, onLogin, locale} = this.props; | ||
| return ( | ||
| <IntlProvider locale={locale} messages={getLocaleMessage(locale)}> |
There was a problem hiding this comment.
Agree, putting this in Redux is a good solution because we can dynamically load it from the server.
| SUBMIT: 'Submit', | ||
| FULLNAME: 'Full name', | ||
| REGISTER: 'Register', | ||
| 'placeholder.username': 'Please input your username', |
There was a problem hiding this comment.
Not a fan of mixing ALLCAPS with dot.format.
#9 Wrapped Login with react-intl