Navigation Menu

Skip to content
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

New getter (and setter?) for dow #1830

Closed
CasperWSchmidt opened this issue Aug 4, 2014 · 7 comments · Fixed by #2128
Closed

New getter (and setter?) for dow #1830

CasperWSchmidt opened this issue Aug 4, 2014 · 7 comments · Fixed by #2128
Labels
Milestone

Comments

@CasperWSchmidt
Copy link

With reference to https://github.com/dangrossman/bootstrap-daterangepicker/issues/401 I would say that moment.js needs a way to get (and possibly also set) dow/first day of week. I don't know is moment.weekday(0) can somehow be used for this, but a moment.DOW(), FirstDayOfWeek() or similar seems to be missing (and quite simple to implement)

@ichernev
Copy link
Contributor

ichernev commented Aug 5, 2014

You can set the first day of week by switching to an appropriate locale or defining a new one.

For now changing the start-of-week outside / orthogonal to locale is not implemented.

@CasperWSchmidt
Copy link
Author

@ichernev I know how to set the start-of-week by using a locale (which is exactly what I'm doing), but to accommodate plugins/libs that build on moment.js a getter is needed to avoid them hooking up on internal moment.js properties (ie. _lang._week.dow). Please see the issue I linked for details on why such a getter would be a good idea.

@ichernev
Copy link
Contributor

ichernev commented Aug 5, 2014

Ah, sorry. Now I got you. Yeah, adding a setter for the language should be an easy task.

Locale.prototype.firstDayOfWeek(); // 0
Locale.prototype.firstWeekOfYearOfYear(); // 6

@ichernev ichernev added the todo label Aug 5, 2014
@ichernev ichernev added this to the 2.9 milestone Aug 5, 2014
@timrwood
Copy link
Member

It's also possible with public apis.

moment().locale('en').weekday(0).day(); // 0
moment().locale('ar').weekday(0).day(); // 6

@CasperWSchmidt
Copy link
Author

So to dynamically get the DOW one will have to use moment().locale(moment().lang()).weekday(0).day() or something like that? That seems a bit too long/difficult IMHO.

@nikoskalogridis
Copy link
Contributor

in the Eonasdan/bootstrap-datetimepicker we used to rely on ._week.dow as well which was a bit of an issue as we were peeking on a 'protected' attribute. The way we choose to go forward was to replace that with moment().startOf('w').day()

@icambron
Copy link
Member

@CasperWSchmidt no, you would just skip the locale stuff altogether:

moment().weekday(0).day();

You only need to specify the locale if you want the start of weekday in some arbitrary locale.

Also, you can cut out one step from your original solution by using the public localeData() call:

moment.localeData()._week.dow;

//or

moment.localeData('fr')._week.dow;

That's not great either, so yeah, I support adding a getter to the Locale prototype, which would result in:

moment.localeData().firstDateOfWeek();

//or

moment.localeData('fr').firstDayOfWeek();

//or even on an instance:

moment().localeData().firstDayOfWeek();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants