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

Weekdays() is not localised and does not return days in the ISO week day order #3284

Closed
curiosity101 opened this Issue Jul 4, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@curiosity101

curiosity101 commented Jul 4, 2016

It seems like these are probably two separate but related issues.

moment.weekdays() is not localised even though the docs imply it should be.
See this section: http://momentjs.com/docs/#/i18n/locale-data/

I tried moment.locale('en-gb').weekdays() and got 'weekdays is not a function'.

Also moment.weekdays() does not return the days in the ISO week day order. Even if it is localised I'd have though it makes more sense that by default the order should follow the ISO week day order.

@icambron

This comment has been minimized.

Member

icambron commented Jul 5, 2016

Hmm, it should be:

moment.localeData('en-gb').weekdays()

but that also throws an error.

@icambron icambron added the Bug label Jul 5, 2016

@curiosity101

This comment has been minimized.

curiosity101 commented Jul 5, 2016

Sorry that was a typo on my part. I did mean I tried:
moment.localeData('en-gb').weekdays()

Thanks for looking into it

@theawesomenayak

This comment has been minimized.

Contributor

theawesomenayak commented Jul 5, 2016

Doing a moment.localeData('en-gb').weekdays() simply throws TypeError: Cannot read property 'day' of undefined.

In fact, it seems none of the locale functions would work without a moment passed as parameter.

@icambron

This comment has been minimized.

Member

icambron commented Jul 10, 2016

So I think what happened here is that we changed an internal signature change but failed to change the apparently untested static locale listing functions like weekdays() to use that new signature. A PR fixing them would be gladly accepted.

@theawesomenayak

This comment has been minimized.

Contributor

theawesomenayak commented Jul 15, 2016

Should this be changed in the documentation? The documentation surely is misleading (the method signatures).

Or the code? weekdays() should return an array with all days of week for that locale?

I can create a PR accordingly.

@icambron

This comment has been minimized.

Member

icambron commented Jul 19, 2016

Or the code? weekdays() should return an array with all days of week for that locale?

The code, I think.

@mj1856

This comment has been minimized.

Member

mj1856 commented Aug 12, 2016

Tracked in #3312

@mj1856 mj1856 closed this Aug 12, 2016

ichernev added a commit that referenced this issue Sep 1, 2016

ichernev added a commit that referenced this issue Sep 1, 2016

Merge pull request #3312 from sibennayak:fix/3284
[bugfix] locales: Enable locale-datnwithout moment ta getters without moment (fixes #3284)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment