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

Adds weekStartsOn as a DateAdaptor option #86

Merged
merged 3 commits into from
Jul 5, 2023
Merged

Adds weekStartsOn as a DateAdaptor option #86

merged 3 commits into from
Jul 5, 2023

Conversation

scottlovegrove
Copy link
Contributor

With this weekStartsOn option, it's now possible for the consuming app to start what day of the week the calendar should start at.

The reason for this is currently it starts on a Sunday, regardless of your locale, so if you're in the UK (like I am), where our week starts on a Monday in the calendar, it looks off for us.

I've done this so it defaults to sunday so that no change in functionality is required, and no input is required from existing consumers of the package.

@bruceharrison1984
Copy link
Owner

Wow, this is a great feature! I had this on the roadmap(in my head), so it's awesome you took it on.

I'll be back on Wednesday to dig into this. Thanks!

@scottlovegrove
Copy link
Contributor Author

No worries, was happy to contribute back to search an awesome calendar component.

@bruceharrison1984
Copy link
Owner

Looks like the build is borked up due to Jest/NodeJS eating up more memory than it did a few months ago:
ArtiomTr/jest-coverage-report-action#356.

I'll have to get that fixed, though I did observe all of your tests are passing.

@bruceharrison1984 bruceharrison1984 merged commit 93ef38e into bruceharrison1984:main Jul 5, 2023
@bruceharrison1984
Copy link
Owner

I'm just going to merge this in for now and unwind the build issues. Thanks!

@scottlovegrove scottlovegrove deleted the scottl/week-starts-on branch July 6, 2023 12:53
@scottlovegrove
Copy link
Contributor Author

Awesome, cheers. Do you know when you might release an updated package with this in?

@bruceharrison1984
Copy link
Owner

I'm working on cleaning the test up a bit, and it should be done after that.

I'll aim for the end of this week, but there are some rough edges with deployment because this had sat for a few months.

@bruceharrison1984
Copy link
Owner

This will likely take a bit longer. While you implemented something to control which day the calendar starts with, the events are overlayed on a separate layer. This speeds up rendering greatly, but it means it will need to be handled separately. Currently the calendar re-renders with the new start day, but the events do not.

@bruceharrison1984
Copy link
Owner

@scottlovegrove new version has been cut. The updated docs have the start day change available to test. Thanks for the PR!

@scottlovegrove
Copy link
Contributor Author

Thanks, I'll put that to use in my project this weekend :)

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.

2 participants