[pickers] Add support for timezones on the adapters #9068
Conversation
flaviendelangle
added
the
component: pickers
Netlify deploy previewNetlify deploy preview: https://deploy-preview-9068--material-ui-x.netlify.app/ Updated pagesNo updates. These are the results for the performance tests:
|
flaviendelangle
force-pushed
the
timezone-adapters
branch
2 times, most recently
from
c4755a2 to
7124db5
Compare
flaviendelangle
force-pushed
the
timezone-adapters
branch
from
7124db5 to
2a715af
Compare
| }; | ||
|
|
||
| public isWithinRange = (value: Dayjs, [start, end]: [Dayjs, Dayjs]) => { | ||
| return value.isBetween(start, end, null, '[]'); | ||
| return value >= start && value <= end; |
There was a problem hiding this comment.
We don't need the power of isBefore and isAfter because we are not passing any unit.
The main advantage of this change is that isBetween / isAfter are very slow on dayjs with the timezone plugin, so I'm trying to use them as little as possible
| @@ -259,36 +296,42 @@ export class AdapterLuxon implements MuiPickersAdapter<DateTime, string> { | |||
| return false; | |||
| } | |||
|
|
|||
| return this.date(value)!.equals(this.date(comparing)!); | |||
| return +this.date(value)! === +this.date(comparing)!; | |||
There was a problem hiding this comment.
Technically speaking, this is a breaking change for the Luxon adapter, because now isEqual returns true if you have 2 dates with the same timestamp but a different zone.
I think we can consider it to be a bug fix because the other adapters are not working the same way and are already based on the timestamp.
| // Both dates below have the same timestamp, but they are not in the same year when represented in their respective timezone. | ||
| // The adapter should still consider that they are in the same year. |
There was a problem hiding this comment.
This comment is somewhat disturbing. Would it be true to say that the isSameYear relies on the timezone of the first/second date provided?
There was a problem hiding this comment.
Would it be true to say that the isSameYear relies on the timezone of the first/second date provided?
Yes, all the isSameXXX isAfterXXX and isBeforeXXX are applied on the timezone of the 1st param.
isSameYear answers the question "for any A and B, is A and B in the same year when displayed in the timezone of A"
There was a problem hiding this comment.
It's good for me.
I don't have a strong opinion is isAfterDay should consider the TZ of the first or second argument. But it could be precise in the method comment:
- Check if the day of the reference date is after the day of the second date.
+ Check if the timestamp of the reference date is after the day of the second date (using the TZ of the second date).
packages/x-date-pickers/src/tests/describeGregorianAdapter/testCalculations.ts
Outdated
Show resolved
Hide resolved
packages/x-date-pickers/src/tests/describeGregorianAdapter/testCalculations.ts
Outdated
Show resolved
Hide resolved
packages/x-date-pickers/src/tests/describeGregorianAdapter/testCalculations.ts
Show resolved
Hide resolved
packages/x-date-pickers/src/tests/describeGregorianAdapter/testCalculations.ts
Show resolved
Hide resolved
Extracted from #8261
Part of #8484
This PR focuses on the adapters and should not include any behavior changes on the pickers themselves.
dateWithTimezone(will replacedatein v7),getTimezoneandsetTimezone