-
Notifications
You must be signed in to change notification settings - Fork 7k
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
moment.weekdays() is localized, but order is not ISO #4579
Comments
I imagine we also need |
How useful would it be for the order to be ISO? JS arrays start at 0, not 1. The current implementation is usable using: var m = moment(...);
var nameFromDay = moment.weekdays()[m.day()];
var nameFromIsoWeekday = moment.weekdays()[m.isoWeekday() % 7]; I'm not sure exactly what you're proposing, but if you're after an array returning var m = moment(...);
var nameFromDay = moment.isoWeekdays()[(m.day() || 7) - 1];
var nameFromIsoWeekday = moment.isoWeekdays()[m.isoWeekday() - 1]; |
I should have provided a use case, you are right. My use case is that I sort data according to their weekday, in a context where (despite english conventions and language), ISO weeks are used. Like : let weekdays = moment.weekdays()
weekdays.push(weekdays.shift())
let sortFunction = d => weekdays.indexOf(d) I have to add |
You can remove line 2 if you make line 3 more complicated: let sortFunction = d => (weekdays.indexOf(d) || 7) |
A couple more thoughts: I assume there's a reason you're sorting by weekday name (e.g. you don't have a date.) If you need to handle abbreviations too, you should look at Regarding your suggestion about Tax and financial years might be a good case where the months need to be ordered differently, but they don't map to a locale... I think if you were going to add |
That would be a lot to add indeed. I think most of the time, people will stick to the locale's weekdays order, and not make a mess with ISO like we do (hopefully they will report it here if they don't). |
It has been localized in #3284, but the order is still not ISO. I guess since the order is localized IRL, we might as well keep
moment.weekdays()
that way, but it would be nice to havemoment.isoWeekdays()
, that would be both ISO and localized − same syntax asmoment().isoWeekday()
.The text was updated successfully, but these errors were encountered: