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

First day option #271

Merged
merged 6 commits into from
Mar 8, 2021
Merged

Conversation

elliscwc
Copy link
Contributor

@elliscwc elliscwc commented Mar 2, 2021

this PR fixes #266
it allows setting first day of week from Sunday (0) to Saturday (6), I have tested with moment locale option changed, no issue so far

@peacechen
Copy link
Collaborator

Thanks @elliscwc for adding this much requested feature.

There are currently users who rely on the startFromMonday. Please add that back, but internally use firstDay set appropriately. That way all the calculations use your new generalized code.

The previous code used 1-based days of the week, whereas your code is 0-based. Does that have any ramifications with ISO dates vs non-ISO dates?

@elliscwc
Copy link
Contributor Author

elliscwc commented Mar 2, 2021

Ok, I added back the startFromMonday, will take it as consideration first only then firstDay prop.

Since the first item of WEEKDAYS array order in Utils is from 'Sun' or 0, it should not have issue. Also if it is '0', the starting index will use firstWeekDay.

However I have changed the getWeekdaysOrder to return ISO weekdays for customDayHeaderStyles.

@peacechen
Copy link
Collaborator

Thanks @elliscwc. This looks good. I'll merge after testing it

@peacechen peacechen merged commit 1c869e8 into stephy:master Mar 8, 2021
peacechen pushed a commit that referenced this pull request Mar 8, 2021
Preserves `startFromMonday` prop
@peacechen
Copy link
Collaborator

Published in 7.1.1

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

Successfully merging this pull request may close these issues.

More day options for start of the week
2 participants