-
Notifications
You must be signed in to change notification settings - Fork 2
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
set up basic i18n #20
base: master
Are you sure you want to change the base?
Conversation
Hi! Thanks a lot for the contribution. Let me take a look and I will get back today or tomorrow. |
src/app/i18n/I18nProvider.jsx
Outdated
|
||
import i18n from './i18n'; | ||
|
||
class I18nProvider extends Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this wrapper component, I suggest using thunk action with a side effect, like this:
export const setLocale = locale => dispatch => {
dispatch({
type: SET_LOCALE,
locale,
});
i18n.changeLanguage(locale);
};
Of course, handle the SET_LOCALE action in the reducer by saving the locale in the store state.
This way you can remove this I18nProvider wrapper, not rely on the deprecated componentWillUpdate
method, and it's also a bit more inline with the Redux philosophy of keeping side effects in action creators.
src/app/i18n/i18n.js
Outdated
}, | ||
lng: 'ro', | ||
resources: { | ||
ro: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Find a way to keep the translation files separate: ro.json, en.json. Not sure how, but i18next should support it. You could also import jsons via the webpack json loader.
src/app/i18n/I18nProvider.jsx
Outdated
import React, { Component } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import { connect } from 'react-redux'; | ||
import { I18nextProvider } from 'react-i18next'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't forget to add i18next
and react-i18next
to package.json
via yarn add -E ...
(let's keep exact versions to avoid update surprises)
import { categorySelection } from './category-selection'; | ||
import { categories } from './categories'; | ||
|
||
const rootReducer = combineReducers({ | ||
i18n, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Hi Sergiu I'm back from AFK sorry about that 🏕😴. Agree on all points and this is what I have so far. Let me know what else to change, and then at the end I can migrate all the strings. I'll also need to recheck code style (I had to disable
Fixed. I'll blame that on npm version mismatch or git CLI slip.
There is now a thunked
Done as well. I can try webpack |
Hi! 👋
refs issue #17
This needs more work and I'm looking for input.
i18n.changeLanguage
is called oncomponentWillUpdate
as that's the only way of changing the language that I found (by calling that method on the onei18n
instance). Redux and lifecycle handles the rest in this case.react-i18next
components do not seem to accept a prop I could forward fromreact-redux
, so I had to make do.I didn't have where to store the state so I added a new
i18n
reducer. To change the language you'd just dispatch an action and changestate.i18n.preferredLanguage
.Let me know what needs tweaking.