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

[@mantine/dates] Fix error with timezones in Month #5916

Merged

Conversation

1g0rrr
Copy link
Contributor

@1g0rrr 1g0rrr commented Mar 13, 2024

Fixes #5840

The problem was that variables startOfMonth and endOfMonth shifted by timezone once and then shifted again during clicks.
At version 7.4.2 which works ok, timezone at getMonthDays was always "undefined" there for it worked good as mentioned in the Issue. With new version of Month.tsx in 7.5.0 timezone is not undefined and so there is a double shift of date.

Is this a rigth direction to solve it?

@1g0rrr 1g0rrr changed the title Fix error with timezones in Month [@mantine/dates] Fix error with timezones in Month Mar 18, 2024
@rtivital rtivital merged commit a7829e9 into mantinedev:master Mar 26, 2024
1 check passed
@rtivital
Copy link
Member

Thanks!

Copy link

@thomson2k thomson2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -175,7 +175,6 @@ export const Month = factory<MonthFactory>((_props, ref) => {
const dates = getMonthDays({
month,
firstDayOfWeek: ctx.getFirstDayOfWeek(firstDayOfWeek),
timezone: ctx.timezone || undefined,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It breaks test. You made it more understandable and safe by taking out the "timezone" property from both the function call and the object setup. This change guarantees that you're not trying to set a property that isn't recognized in the type definition of getMonthDays or the object itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What test does it break? The CI passed.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It no longer break tests. It was fixed after my comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not get it. Is the PR fine or is there some issue that you've found?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine

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.

DatePickerInput is off by one day when using timezone
3 participants