guppy
guppy copied to clipboard
Chinese translations and intl demo
Related Issue: 66
Summary:
We have completed the Chinese translations and put all the translation-related files in the locale
directory, as we've discussed in the issue. These files are connected to the components by intl.reducer.js
and react-intl-redux
.
The next step will be to modify all of the components with messages. We've already done this part with the IntroScreen and the CreateNewProjectWizard/BuildPanel component. You can take them as a simple demo. This pull request will keep being updated in the following days to include the change of other components.
By the way, do you think we should submit the pull request to the i18n-app-menu-item
branch? Or can you create another branch (e.g. i18n
) for the following updates?
Screenshots/GIFs:
Hi @zianke: Thanks for the update. After briefly looking at the code it looks great so far. I'll do a detailed review later.
I have just some questions/comments:
-
intl.reducer
I think it's OK to have it & do it as you have implemented it - just wondering what benefit we would have if we would useintlReducer
from react-intl-redux e.g. dispatchupdateIntl({...})
- is this only doing the state updating? -
setState
warning after initial load (see screenshot below) - can'tsetState
on an unmounted component. Not sure why we're getting that message. Update OK, this is not related to i18n and can be fixed by adding_isMounted
inInitialization.js
component to avoid setting state on unmounted component. Like mentioned in this post. I have added it to thefeature-i18n
branch. - You have changed the currently selected lanuage from
app-settings.reducer
tointl.reducer
. I think that's working but I'd like to have the currently selectedlocale
inapp-settings
reducer to keep every setting there. I understand why you did it as it's easier to get theinitialLocale
. We can improve this with aapp-loaded.saga
later - it's OK to keep it as it is.
Sure, I can create a new branch i18n
based on i18n-app-menu-item
so we can merge the next updates there. Maybe we can also change the target of this PR to the new branch - I'll check if it's possible to change.
@zianke any progress on this? What do you think is this ready for merge to branch feature-i18n
?
I think I can fix the one open inline comment on the branch.
We can create follow up PRs if you're having updates. I'd like to keep the branch i18n a bit longer active until we're having some languages ready.
I'd like to start with German translation and a guide for how to add more languages.
@AWolf81 Sorry for the delay of reply. It has been two busy weeks. Hopefully I'll have some time these days to continue on the tasks.
react-intl-redux is just a simple wrapper over react-intl to implement redux binding. For basic react-intl, we need to set locale and messages with <IntlProvider locale={...} messages={...}>
. Since we have IntlProvider
in src/index.js
, however, it's not easy to access locale or messages and update them in src/index.js
. react-intl-redux can instead read from the redux state, more specifically, the values of keys intl.locale
and intl.messages
. That's why I wrote a intl.reducer
so that the values appear in the redux state with the specified keys instead of something like app-settings.locale
, otherwise react-intl-redux cannot read them correctly. It's possible to move the locale value back to app-settings.reducer
, but we may need to change something on react-intl-redux. I'm currently reading the source code of react-intl-redux to see if we could customize the keys from which locale and messages are read from redux.
Another important question to consider is how to implement internationalization in non-component code. For example, there are some hard-coded strings in types.js
. This has been a popular issue of react-intl, with many discussions here. You may want to have a look first. Once we find a best solution to this problem, I'll update a non-component js file in this pull request to reflect it.