i18n fix

[thaerlabs]

This will solve both #44 and #116.

The issue is that a the time of the instantiation of WEEKDAYS and MONTHS, lang will be default en..

So now when calling require('moment/locale/ar') somewhere else, the WEEKDAYS and MONTHS will be based on the global moment app locale...

[AlanFoster]

Thanks for the PR!

Would we be able to add tests for this new work too? If you need any help with that, just ping us and we can help get you set up :+1:

[thaerlabs]

@AlanFoster for sure tests have to be added, and I was thinking about adding a page in the demo to demonstrate the locale change, and I think the docs can also be improved

[thaerlabs]

@AlanFoster @rolyatmax I've updated the example, so now we can set locale as a prop #44 . Requiring moment/locale/[whatever] outside won't affect the component. The locale prop will take care of loading locale file, and trigger a re-render as required by @rolyatmax, also the dropdowns of month and year are fine #116.. So last but not least, tests should be added. I think I won't have much time during the week, help is appreciated in this context :dancers:

[ghost]

@AlanFoster could this pr be merged? excited to see the new feature. :+1: :)

[AlanFoster]

Do you feel #56 is enough to implement the new locale support, or does this PR add new functionality that is missing within #56 ?

@miracle2k @rolyatmax @haerlabs As you guys have more context on i18n support i'd love your 2¢ on this

[miracle2k]

The differences between my patch in 56 and this one aren't so big. The require() issue might need to be addressed. I am +0 on the _setupLocale cache.

This PR adds a nice example, and it seems @thaerlabs has more interesting in moving this forward.

[AlanFoster]

Great, thanks for the input @miracle2k :+1:

Getting i18n support would obviously be great to have merged and released; are you happy to do the remaining work for this PR @thaerlabs to help get this PR shipped?

[miracle2k]

Can we get this merged? Seems @thaerlabs made some changes.

[AlanFoster]

@thaerlabs Looks good to me; Would you mind adding some quick tests then we can get this merged in? :)

[thaerlabs]

@AlanFoster I'll work on the tests ASAP. I think also that we should enhance the documentation, especially for the fact that this feature requires the user to require the moment/locale manually, otherwise setting the prop won't be enough to change the datepicker. This approach is better than dynamic require the locale in the _setupLocale function when componentWillReceiveProps, because when webpack bundles, it will put there all the moment/locale/**, as it doesn't know which one to pic.

[thaerlabs]

@AlanFoster can you help me in testing the months dropdown and the weekdays text? I want to assert against the text content of the months and weekdays when changing locale

[fluse]

when will this merged?

[evgenTraytyak]

@AlanFoster ping

[AlanFoster]

@thaerlabs Would love to get this out; What testing difficulties did you have? :+1:

[evgenTraytyak]

@thaerlabs russian locale ru doesn't work :( Uncaught TypeError: Expected Array or iterable object of values: [object Object]

belarusian locale be too..

[onzag]

@thaerlabs is this merged? how can I check this? do you have a bit of documentation? I need to use this in production :+1:

Sorry I'm a bit of a newbie with git I can't find the repo.

[maullerz]

Uncaught TypeError: Expected Array or iterable object of values: [object Object] belarusian locale be too..

its because in moment@2.11.0 they change format of weekdays and months for some locales RU https://github.com/moment/moment/blob/develop/locale/ru.js so its now objects, not arrays

i didn`t found any issue or PR about that, only this commit

[fluse]

@AlanFoster please merge branches!!!!!!!

[thaerlabs]

@AlanFoster I added tests for the month and year headers. Please check it and provide your feedback!

[thaerlabs]

@evgenTraytyak @maullerz the problem is with the locale definition for both ru and be. Somebody should fix the format and open a pull request to moment. I don't think it's a good idea to work around this bug in moment in the datepicker component, as if they fix it, it'll break again the datepicker. Plus we should add a special case for those locales with a different format, which will make the component more fragile. that's my thought..

[maullerz]

nvm, see @wmertens comment

[ashmigelski]

Guys, you cant merge with ru locale issue. And more over there's no bug in moment.js...

Here's my workaround, just add isArray check for lang._weekdays and lang._months in https://github.com/onefinestay/react-daterange-picker/blob/master/src/calendar/CalendarMonth.jsx#L12

    const lang = moment().localeData();

    let weekdays = lang._weekdays;
    if (!Array.isArray(weekdays)) {
        weekdays = weekdays.standalone;
    }
    let months = lang._months;
    if (!Array.isArray(months)) {
        months = months.standalone;
    }

    const WEEKDAYS = Immutable.List(weekdays).zip(Immutable.List(lang._weekdaysShort));
    const MONTHS = Immutable.List(months);

[maullerz]

Guys, you cant merge with ru locale issue. And more over there's no bug in moment.js...

why? can you explain?

[ashmigelski]

why? can you explain?

I explained it 3 month ago https://github.com/onefinestay/react-daterange-picker/pull/119#discussion_r56464224

In short

• datepicker expect that _monthNames can be only an array.
• moment support different locales and some of them have month names in several forms.
• for such locales _monthNames becomes an object with arrays inside

If you want datepicker work well with all locations, you have to add check is locale is simple (_monthNames just array) or complex (_monthNames is an object and real month names can be found inside, like _monthNames['standalone'] )

Code in my prev comment

[wmertens]

Moment has an API to get the months and weekdays as an array. Call that instead of relying on Moment internals: http://momentjs.com/docs/#/i18n/listing-months-weekdays/

[thaerlabs]

@wmertens great hint! I'll fix that ASAP

[AlanFoster]

@thaerlabs Let me know when you've had the time to apply the PR feedback from @wmertens After a squash of commits I'll try to get this in! :+1:

[AlanFoster]

@thaerlabs Requires a rebase :) Let me know if I can do anything to help out :+1:

[maullerz]

@thaerlabs any news about that? :pray:

[andywer]

@AlanFoster @thaerlabs Is there anything left to do? We also stumbled upon the localization issue here.

[maullerz]

@andywer fyi there is rebased and fixed PR forked from this - #150 so in my project i use react-daterange-picker from this branch - https://github.com/maullerz/react-daterange-picker.git#dist Temp workaround while we all waiting updates of original repo.

[AlanFoster]

Closing this in favour of #150

Thanks for the hard work guys; I'll aim to release this as 2.x