APSL / redux-i18n

A simple and powerful package for translate your react applications using react-redux.
MIT License
233 stars 52 forks source link

setLanguage can't handle de-DE #17

Closed sideisra closed 7 years ago

sideisra commented 7 years ago

When dispatching a setLanguage action with payload "de-DE", the language is not set to "de" but defaults to "en". It seems that the component can't handle languages with format xx-YY.

francescarpi commented 7 years ago

Hi,

I'll check it today.

Thanks

francescarpi commented 7 years ago

Hi!

Do you have "redux-logger" or something like that for see the store before and after to dispatch action?

I have added some tests for check this and all works fine.

https://github.com/APSL/redux-i18n/commit/53db59be2fd11c20b8c23b8dc9addd621bf2df90

Can you show me your i18nState store content, please?

Thanks.

sideisra commented 7 years ago

prev state:

i18nState:{
  lang: "en"
}

action:

{
  type: "REDUX_I18N_SET_LANGUAGE",
  lang: "de-DE"
}

next state:

i18nState:{
  lang: "de-DE"
}

Thats what happens in my store and thats what i would expect. But the component seems to use the string "de-DE" as-is but can't find any translation because the translations look like this:

export const i18n = {
    translations: {
        de: {
            "Logout": "Abmelden",
        },
    },
};
francescarpi commented 7 years ago

OMG!

You are right. I'm going to think how resolve it and upload a new version soon.

Thanks

francescarpi commented 7 years ago

Hi,

I just created a new tag (1.1.2) that solve this problem. Please, try it and tell me if you find any errors.

Thank you so much.

sideisra commented 7 years ago

Thank you very much.

I tested the new version and 'de-DE' now works but only when i change the translations to:

export const i18n = {
    translations: {
        'de-DE': {
            "Logout": "Abmelden",
        },
    },
};

It works for IE but not for firefox because firefox returns 'de' as language. I think the most robust approach would be to fall back to 'de' when 'de-DE' is not available in the translations.

Should i create a new issue for that?

francescarpi commented 7 years ago

Hi!

I've modified import script for generate the file as you show me. If you run your import script (from .po to translation.js) you can see it.

Try it, please and tell me.

Best regards.

sideisra commented 7 years ago

We don't use a po file we write the translations.js directly.

francescarpi commented 7 years ago

Hi,

I'm not sure I understand to you. I just uploaded a new versión with "demo" app. Please, can you pull from github, install demo app and try it?

Tell me if works fine on chrome, firefox, IE, etc...

Best regards.

sideisra commented 7 years ago

Hi,

sorry i didn't recognise the demo. I tried it an now it works for "de-DE" but not for "de" any more. Sometimes there are multiple language codes (e.g. "de-DE", "de-CH" or even "de") which should fall back to the same translation (e.g. "de").

In my case with the new version i would have to duplicate the translation: "de" and "de-DE"

Best regards

francescarpi commented 7 years ago

Hi,

you can have as many languages as you want. For example:

export const translations = {
  'de-DE': {
    'Hello world!': 'Hallo Welt!',
  },
  'es': {
    'Hello world!': '¡Hola mundo!',
  },
  'de-CH': {
    'Hello world!': 'Hallo Welt!',
  },
  'de': {
    'Hello world!': 'Hallo Welt!',
  },
}

And then, activate the language that you need:

this.props.dispatch(setLanguage('de-CH'))
this.props.dispatch(setLanguage('de-DE'))
this.props.dispatch(setLanguage('de'))

For test "demo", you should checkout repo and:

cd demo
npm install
npm start

And go to: http://localhost:8000

Best regards.

sideisra commented 7 years ago

Hm i don't want to define a translation for every language ("de-DE", "de-CH") because the translations would be the same. They would be duplicates.

It would be much more convenient when languages like "de-DE" or "de-CH" have the fallback "de" so that i have to provide "de" only.

francescarpi commented 7 years ago

Ok. Perfect :). Now I understand to you fine. I can make that if language is not in translations dictionary and has "-", make an split and take first part.

I like it, this is a good new feature.

Thanks!

francescarpi commented 7 years ago

Hi!

You have a new version (1.1.4)

;)

Best regards.

sideisra commented 7 years ago

It works. Thank you very much :-)