Closed kitfit-dave closed 4 years ago
hi, any update on this? I noticed that the component is always returning the wrong date unless initialized with current date-time or with a date contructed with Date.UTC. see this codesandbox
const [value, handleDateChange] = useState(new Date());
// ok current date and current time
const [value, handleDateChange] = useState(new Date(2019, 0, 1));
// value is 2018-12-31T23:00:00:000Z
const [value, handleDateChange] = useState(new Date(Date.UTC(2019,0,1)));
// value is correct 2019-01-01T00:00:000Z
Am i missing something very simple in the configuration of the component?
I just noticed that when trying a simple plain JS Date the behaviour is similar - but only when date is Jsonified...
const d = new Date(2019,0,1) d.toString() // 07:46:11.769 Tue Jan 01 2019 00:00:00 GMT+0100 (Central European Standard Time)
while
const d = new Date(2019,0,1) d.toJSON() // 2018-12-31T23:00:00.000Z
which is the same as
d.toISOString()
did not notice before - and attributed the issue to the component because I did not notice that in the sample sandbox provided in the docs the date is toJson. ( I still have to find a solution to my problem because I cannot simply use Date.UTC because the component is being used through a couple of other react libraries and i have no direct access to it - but I believe this is not an issue of this lib)
I, too, ran into this issue. I built a nonsense MuiDateUtilsProvider
value to work around it.
To solve the underlying problem, DatePicker
and TimePicker
value
fields must be passed as ISO8601-encoded String
, instead of JavaScript Date
. (This will require a major version bump.)
JavaScript Date
objects are timestamps -- coordinates along Earth's temporal axis. The human concepts of "date" and "time" do not represent coordinates along Earth's temporal axis. JavaScript Date
objects are not designed to encode the human concepts of "date" and "time".
To a human, a "date" is related to Earth's temporal axis, but it means something different depending on context (and geography). A person in England and a person in Canada can share a notion of "May 13, 2020" but they won't agree on the exact moments that it occurred. A Date
object can mean two different "dates" to two different humans.
To a human, a "time" is actually an infinite set of points along the temporal axis. It also means something different depending on context (and geography ... and Daylight Savings time). A person in England and a person in Canada can share a notion of "7 a.m.", but they won't agree on when it occurred.
Unlike JavaScript Date
objects, ISO8601 String
s were designed to encode the notions of "date" and "time". The values 2020-05-13
and 07:00
encode the exact concepts that a Canadian and a Brit can agree on. https://en.wikipedia.org/wiki/ISO_8601.
(The Date
value type is correct in the case of DateTimePicker
, because the human is picking a moment in time with it.)
It would depend on what you were using the date for. If you are building an international meeting planner, the date picker is picking a point in time that is the exact same point on the "temporal axis" as you put it, which should be converted between timezones for people when reading it. But if you are building a birthdate selector then you will be wanting the "notion of May 13" as someone's birthday is not considered to change with timezones. You can't solve the problem by arbitrarily supporting one interpretation over the other, it would have to be an option or something, and it's just not something that the javascript date object is build to handle. (i.e. like you said, you'd probably be best doing it with something like ISO8601, leaving off the optional timezone part to get the "notion of 7am" version")
@kitfit-dave If you're building an international meeting planner, use DateTimePicker
. @material-ui/pickers
already provides it. DateTimePicker
correctly chooses a JavaScript Date
.
This bug (and my comment above) is about DatePicker
. It incorrectly accepts and picks JavaScript Date
s. It ought to be picking ISO8601 String
s.
When a user types <DatePicker ...>
, we already know that they do not want a timestamp. If they wanted to select a timestamp, they would have used <DateTimePicker ...>
.
Quite right. I’d forgotten that there were two separate components.
Do you guys have a reproduction on the correct and incorrect behavior? As far as I can tell, the logic is correct. It sounds like a duplicate of #1526, which is about better documenting the timezone behavior.
Yeah, I put a codesandbox up there: https://codesandbox.io/s/material-ui-pickers-usage-demo-hc189
I don't think this is (just) a documentation issue, the behaviour in that code sandbox just seems wrong. The time should never anything other than midnight for the DatePicker, right? And the wrong date value is returned (i.e. not the date the person clicked on at all)
The time should never anything other than midnight for the DatePicker, right
@kitfit-dave It depends, as far as I have follow the problem, the date picker should change the year, month and day of the provided date, keeping the same time and timezone. So you can very well have a non midnight value, depending on how you format the string, say in UTC or your current timezone.
In the case of when the date is retrieved from the textbox or the date is initially empty, I would assume it takes the browser time and timezone.
@dmtrKovalenko is this description accurate?
@oliviertassinari Yes, thank you, I support the description :)
@oliviertassinari Thanks for weighing in!
The time should never anything other than midnight for the DatePicker, right
@kitfit-dave It depends, as far as I have follow the problem, the date picker should change the year, month and day of the provided date, keeping the same time and timezone. So you can very well have a non midnight value, depending on how you format the string, say in UTC or your current timezone.
I'm lost. Are you suggesting that a developer may want to let the user change the date of a timestamp using DatePicker
? As in, there would be no way for the user to change the time, but <DatePicker>
is intending to change the date? This seems bizarre, because the same date in two different time zones is two different dates. (If the developer wants to compose a timestamp selector using <DatePicker>
, then the developer should want <DatePicker>
to select an ISO8601 String.)
The problem is more fundamental: <DatePicker>
picks a timestamp when it should pick a date. But most timestamps have two dates. One of the (many) user-visible problems that arises: <DatePicker value={value}>
displays a different date depending on the user's timezone.
@adamhooper The incentive is to leave flexibility to the developers to set the time and timezone they need. The mention of the timestamp is interesting, it's one representation possible of the date returned by the date picker, it represents the number of seconds since an arbitrary reference in the timezone of the user.
For a new section of the documentation around the timezone management, I think that it would be great to have, at least, these sections:
This isn't about documentation. DatePicker
is doing a conversion most users don't want, and it's hard for users to undo it.
All users of DatePicker
are forced to:
A) get a timestamp when they expect a Date; or B) support a single time zone; or C) write convoluted workarounds (see https://github.com/mui-org/material-ui-pickers/issues/1526#issuecomment-628237301) for an example; or D) experience bugs (such as this one).
And in my experience, most developers don't realize that the fundamental problem is, a JavaScript Date
does not represent a date! The DatePicker
API should help people do the right thing. Native <input type="date">
does.
It doesn't make sense to me that users must read a section about timezone management when they're picking a date. Most users don't want timezones in their dates. Why force them to read the docs for something they don't want?
I wholeheartedly agree with @adamhooper here, and if I recall correctly, it is pretty much why I chose the image above to summarise my surprise at finding a timezone all up in my date picker...
If you want to decide to maintain whatever time setting the user may have somehow initialised the date picker with then that is up to you, I think it’s hard to justify with a user story though, but also certainly the current behaviour when that time setting DOES change (input of midnight becomes 2300), and the case where the wrong date entirely (the day before) ends up as the value, would seem to need to be resolved as bugs anyway.
If you want to know how exactly I found this issue (ie why would I have initialised the date to some other timezone) here is what I recall:
We are using the date picker to have the user enter their birthdate. We were getting reports of users being unable to set their birthdate. I narrowed it down to a few months of the year that did not work. IIRC we were currently in summer time, and the user’s birth date was in standard time. So when they on boarded we set the date picker to today, and they then selected their birthday in April (a different timezone) and hence the problem.
@dmtrKovalenko Actually, I start to wonder if we shouldn't change the tradeoff. For what I understand, you have explained the "why" behind the current tradeoff in
What if I need to build a datetime picker? Google approach is to have 2 separate controls – it will be painful if datepicker will always reset local time.
Because if we are changing time it is the behavior user cannot control. He can easily control annulation time manually (moreover we are requiring using date library, so the user will not need to do date.setHours(0).setMinutes(0).setSeconds(0)).
However, it seems that in its current form, the tradeoff leads to fairly frequent bugs? that are hard to identify: with a long feedback loop. The main point of concern is how easy the developers can recover from it? It seems to be time-consuming (e.g. finding the daylight saving issue).
I don't think the "current tradeoffs" even work properly actually. For instance, in the multi-picker case, the user has set the time to say 10am, but then changes the date to a different TZ and the time will become 9am or 11 am. So that is contrary to the stated design goal of "not changing the time"
I think either "do not change the time" or "reset the time to midnight" would work for the original case in this ticket, it does not seem that important which one is chosen, just that whichever one is chosen is implemented correctly and the time either ends up the same or equal to midnight.
Resetting to midnight seems like it prevents some uses of the component. Leaving the time alone seems like it works for all cases, as the user can set the time to midnight themselves if that is what they want - I assume that is why it looks like the code is trying to do that, it's just failing in the "different timezone" case. The only scenario where "leave the time alone" seems to cause problems is where people are assuming that the picker does not deal with time, and get confused when the picker's time is set to the current time - that one can be improved by documentation.
Leaving the time alone seems like it works for all cases
That's impossible: some times don't exist on some dates in some timezones.
Choosing between "do not change the time" or "reset the time to midnight" guarantees that developers will see undefined behavior, because no developer would consider or expect either piece of logic. (Plus, almost everywhere -- even in England during the summer -- there are two sensible definitions of "midnight" and two sensible definitions of "do not change the time".)
I urge this project to mimic <input type="date">
, for:
Ease of understanding: Everyone understands what <input type="date">
picks. A tiny, tiny fraction of @material-ui/pickers
users understand what <DatePicker>
picks. (I would argue that nobody really understands what <DatePicker>
picks -- hence this bug, other bugs, a whole section of documentation about timezones, etc.)
Ease of composition: The <DatePicker>
docs suggest using Moment.js or other libraries' "start-of-day" function to compose dates. With <input type="date">
dates (ISO8601 Strings), that's already done. So if a developers wants to generate a JavaScript Date
using a value the <DatePicker>
picked, that developer will have an easier time generating a Date
from a String
than from a Date
! Consider:
// If `<DatePicker>` returns an ISO8601 String:
function buildTimestampInLocalTime(dateString, timeString) {
const [hour, minute] = timeString.split(":").map(parseFloat)
return moment(dateString).hours(hour).minutes(minute)
}
// If `<DatePicker>` returns a Date:
function buildTimestampInLocalTime(dateObject, timeString) {
const [hour, minute] = timeString.split(":").map(parseFloat)
return moment(dateObject).hours(hour).minutes(minute)
.startOf("minute") // gotcha! Nobody expected to need this
}
(Also, for those of us who despise moment.js, the String
still leads to pleasant code):
// If `<DatePicker>` returns an ISO8601 String:
function buildTimestampInLocalTime(dateString, timeString) {
const [year, monthPlusOne, day] = dateString.split("-").map(parseFloat)
const [hour, minute] = timeString.split(":").map(parseFloat)
return new Date(year, monthPlusOne - 1, day, hour, minute)
}
// If `<DatePicker>` returns a Date:
function buildTimestampInLocalTime(dateObject, timeString) {
const [hour, minute] = timeString.split(":").map(parseFloat)
return new Date(
dateObject.getFullYear(),
dateObject.getMonth(),
dateObject.getDate(),
hour,
minute,
)
}
Ease of testing: If <DatePicker>
picks times-of-day, then callers should test what their user experiences are when the <DatePicker>
returns a time-of-day. With <input type="date">
, they don't need to.
[edit, adding one more]
Ease of drop-in replacement: If <DatePicker>
behaves like <input type="date">
, then a developer can easily switch between the two. So switching to @material-ui/pickers
becomes near-free ... and there's no lock-in.
You are quite right, about keeping the date, I hadn’t considered that fully. And the best thing to do would certainly be to use ISO strings (it’s how I’ve fixed our implementation to maintain the correct values). But I was trying to suggest something that was likely to be implemented that would solve most of the problems. I just don’t see anyone agreeing to change this control to having a different value type now, would be happy to be proved wrong though.
I’m not sure I see why going with returning a date set to start of day (which is what I meant with reset to midnight) for whatever date it picked would not work. I can’t think of that you mean by two sensible definitions of midnight. Are there two definitions of start of day? I mean, the function itself must return one value for any day, this lib would be saying “that is the value you get for the time”. It would be expected, because the docs could literally say that. I am pretty sure midnight always exists (dst changes happen between 1am and 3am I think everywhere?) and only exist once (leap seconds are added just before at 23:60 I think).
The problem of most people not knowing what the picker returns stems from the fact that most people don’t understand what a JavaScript date really is, and is probably out of scope for this project.
The problem of most people not knowing what the picker returns stems from the fact that most people don’t understand what a JavaScript date really is, and is probably out of scope for this project.
(Sorry, coming back here when documenting my code, and I can't help but reply!)
We agree a JavaScript date is really a timestamp, not a date.
We agree that most users of DatePicker
and TimePicker
don't understand this.
We agree that teaching the nuances of JavaScript Date
really is is outside the scope of this project.
But I disagree about what this means for this project. If this project doesn't mean to teach the nuances of JavaScript Date, it shouldn't accept and return JavaScript Dates in DatePicker
and TimePicker
. It should tell users, "Use ISO8601 Strings for DatePicker
and TimePicker
." Curious developers can learn about JavaScript Date (and why it's inappropriate) on their own time.
I've seen DatePicker
in three codebases by three authors. In all these places, Date
led to mad, buggy spaghetti code. These three developers didn't know they didn't want JavaScript Dates. They tried manipulating the dates, leading to buggy spaghetti code.
Surely the best practice is to mimic <input type="date">
or <input type="time">
? Then developers would have a wealth more resources on the Web to learn at their own pace.
Hey, I'm on team ISO 100%. If I were writing this I would have done it that way (mostly because I've tried and failed to do it with Date
many times over the years). But I also understand team "don't change the component interface" and could see why a large percentage of program managers would be hard to convince to make this change. That said, it IS pretty broken as is, but I think you and I know that, but others maybe are not quite convinced. Still, were there a poll for changing over to ISO strings it would have my +1 for sure (maybe it's not too late).
We are not using Date objects at all. We are using special date-io library that is working as abstraction over date objects, or Luxon for example.
If you wish you could write your own date-io adapter that will work through the pure string and use it.
I understand the root of issue and will try to think how we can fix it in pickers.
@dmtrKovalenko What would be the downside for value
and onChange
to only accept/return plain strings, using the same format at the native input?
to only accept/return plain strings
Nevermind, bad idea, react-day-picker, react-datepicker, react-dates.
@dmtrKovalenko Using a date-io library doesn't address the root problem. The date-io libraries are all designed to abstract JavaScript Dates.
I feel like there's lots of confusing information or misinformation in this issue's discussion -- and in #1526. Many developers seem to think DatePicker
should not ... uh ... modify the time-of-day. That's downright bizarre. DatePicker
should not be aware of the time-of-day.
We are not talking about to use Date
objects or not to use. It is a kind of standard in javascript and we cannot remove support of it. If you don't want to use date object you can use any library for free and lock timezone globally. We do not return Date
objects. We return the object created by date-io as an abstraction. And pickers don't know anything about inputs or outputs in the runtime.
The nature of Date object is that it is always in user local timezone. If you understand that – you should not have any problem. It is really weird that you are using date-fns and trying to fight with timezones. Because date-fns is bad decision for international projects. (probably we can mention it in docs)
I am going to lock this discussion.
Lock away, but I think you have not understood this discussion at all if you think it’s not something to be resolved. Adam is right. “You should not have any problem” is untrue, have you taken the time to look at the repro I provided right at the start? Tell me again how that is the correct behaviour, or one that can be avoided by “understanding” something.
Saying that you only have to worry about time zones for “international” projects is incorrect and the root of the issue. Have a look at the repro. The picker fails to work completely.
If you don’t like the discussion, lock it. Seems a bit adversarial to me but I guess I can go and use another library right? I just like this one and was trying to help make it better.
No, I am saying that date-fns is a bad choice and you need to try Luxon or moment-timezone.
Ok I will try to make a fix with Luxon
Because date-fns is bad decision for international projects. (probably we can mention it in docs)
@dmtrKovalenko From what I understand, the issue we are talking about isn't related to internationalization but daylight saving. Let's take the initial codesandbox and set the starting day to 2020-03-2T00:00:00.000Z
. I'm in France, I'm in the winter timezone. Now, let's select June 1st. It's in the summer timezone in France. Both moment and date-fns returns 2020-05-31T23:00:00.000Z
as the ISO string. Because the daylight saving switches the timezone, the ISO day is one day behind.
I think that, in the use case of the birthday, people decide to use ISO as most common format but they don't realize that this timezone shift can occur, moving the date one day behind. This makes ISO the wrong abstraction. We don't ask people when they are born in the ISO format, we ask a day & time relative to a city of birth.
There is no issue of this nature with the string returned format of the native input as the time is stripped:
<input type="date" />
I've read through this and am maybe facing a similar issue. We have physical locations (testing sites for COVID-19). We have staff that need to configure appointments for these. The staff can be anywhere. They need to select a date.
For example, they want to select "June 6th". That June 6th is June 6th in a known time zone (the testing site's) but whatever "start of day June 6th" is in the user's local timezone might not be "June 6th" when converted to UTC or the testing site's timezone.
Basically, they want to pick a date.
I can't seem to get this to work. Setting disablePast
causes (depending on the difference between the local timezone and the one passed in (the one of the testing site)) the date to be pre-filled as invalid.
I'm quite at a loss and would appreciate any advice
Yeah, sounds the same. The “quick fix” for us - which is probably wrong in some cases, but has worked so far - was to set to startofday then pick the date, then if the time was wrong, remove it and adjust the day returned. Your situation might be different also as we were handling date of birth selection.
I've read through this and am maybe facing a similar issue. We have physical locations (testing sites for COVID-19). We have staff that need to configure appointments for these. The staff can be anywhere. They need to select a date.
For example, they want to select "June 6th". That June 6th is June 6th in a known time zone (the testing site's) but whatever "start of day June 6th" is in the user's local timezone might not be "June 6th" when converted to UTC or the testing site's timezone.
Basically, they want to pick a date.
@tyre if they're setting up appointments, then don't they want to pick a datetime? Are you maybe trying to use <DatePicker>
and then a separate time picker?
(It looks like a useful feature would be for <DateTimePicker>
to support a timezone
prop. If that would solve your problem, you should file a separate bug report for that.)
Anyway, the most urgent info for you is an absolute-rule datetime primer: always, always, always store datetimes (e.g., scheduled appointment times) in the UTC timezone. If your staff is in India and selects the time, "June 3, 2020, at 3:00 p.m.", then store the time as 9:30 a.m. -- 2020-06-03T09:30:00.000Z
(1591176600000
in milliseconds) in your database. Your web server should accept and serve all times as UTC (in JSON, use ISO8601 format). The parts of your system that touch end-users directly (e.g., JavaScript-powered web page) must know the timezone the user expects, and only those parts of your system should convert into whatever timezone is appropriate.
Rephrased, for clarity:
<DateTimePicker>
assumes [and <DatePicker>
assumes, in my opinion incorrectly], because JavaScript Date
functions implicitly use the user's operating-system timezone. It sounds like in your case, you may want a different timezone.)... anyway. You have indeed stumbled upon a problem that is straightforward to solve with <input type="date">
and hard to solve with <DatePicker>
. You can contribute to this issue by describing exactly what you're trying to do and what you've tried so far.
In the meantime ... if you want to deal with more urgent matters than this bug, just use <input type="date">
and maybe <input type="time">
. If you're trying to schedule appointments when the user is in a different timezone than the appointment, use a library to help convert. (Avoid date-fns-tz
: it doesn't actually store "UTC-date-plus-timezone", so it has unfixable bugs.)
Here's an example using Luxon (untested):
import React from 'react'
import PropTypes from 'prop-types'
import DateTime from 'luxon/src/datetime.js'
const tz = 'Asia/Kolkata'
const isoTimeFormattingOptions = {
suppressMilliseconds: true,
suppressSeconds: true,
includeOffset: false,
}
function iso8601DatetimeToDateAndTimeStrings(str) {
// Luxon DateTime holds two pieces of information:
// 1. UTC timestamp (as passed in `str`)
// 2. Timezone (as passed in `tz`)
const dt = DateTime.fromISO(str).setZone(tz);
return {
dateStr: dt.toISODate(),
timeStr: dt.toISOTime(isoTimeFormattingOptions),
}
}
function dateAndTimeValuesToIso8601Datetime(dateStr, timeStr) {
// Assume the user gave these date and time strings in `tz` timezone
const dt = DateTime.fromISO(`${dateStr}T${timeStr}`, { zone: tz });
if (!dt.isValid) {
return null // e.g., the time doesn't exist on that date
}
return dt.toUTC().toISO();
}
/**
* Pick an ISO8601 time in India's timezone.
*
* The output time will always be in UTC -- that is, ending with "Z".
*/
export default function IndiaDateTimePicker({ iso8601Value, onChange }) {
const { dateStr, timeStr } = iso8601DatetimeToDateAndTimeStrings(iso8601Value)
const handleDateChange = (ev) => {
if (ev.target.value) {
const newValue = dateAndTimeValuesToIso8601Datetime(ev.target.value, timeStr)
if (newValue !== null) {
onChange(newValue)
}
}
}
const handleTimeChange = (ev) => {
if (ev.target.value) {
const newValue = dateAndTimeValuesToIso8601Datetime(dateStr, ev.target.value)
if (newValue !== null) {
onChange(newValue)
}
}
}
return (
<>
<input type="date" value={dateStr} onChange={handleDateChange} />
<input type="time" value={timeStr} onChange={handleTimeChange} />
</>
)
}
IndiaDateTimePicker.propTypes = {
/** ISO8601 String. Must be non-empty -- maybe use a default value? */
iso8601Value: PropTypes.string.isRequired,
onChange: PropTypes.func.isRequired, // func(iso8601String) => undefined
};
@adamhooper thank you!
we do want them to pick a date and a time, but DateTimePicker
is not intuitive. They don't know what they're looking at (plurality of users > 50 years old, in government jobs that are well behind technologically.) So it makes sense to them to pick a day and then pick the range of times (we do that with a simple drop down since they can only happen on half-hour marks.)
All of our dates are served and stored in UTC. The issue is at the time of choosing these dates, we need to use javascript dates. We need the picker to display a range of dates that are those still valid (today -> future) in the testing site's timezone. So we need to display those, which is the issue I'm having here. Because disablePast
doesn't seem to work when the user's local timezone differs from that of the testing site, even when the date passed in is helpfully converted to the start of the day in that timezone.
It does seem that timezones are not supported by this library and I will have to do something else instead.
The issue is at the time of choosing these dates, we need to use javascript dates. We need the picker to display a range of dates that are those still valid (today -> future) in the testing site's timezone.
Why do you "need" to use JavaScript Dates? (It seems to me they're the problem, not the solution.)
I'm closing for #1526, I think that we are discussing the same problem
why it is dirfferent, when using mui date-picker, it shows GMT-8, but chrome console shows GMT-9
@httol because April 2022 in your timezone is daylight savings time. Nov 22 isn't.
Browsers use timezones, not UTC offsets. The UTC offset of a timezone varies.
Take a step back: this all makes no sense. Use Strings, not Date objects. Check my code snippets in this thread.
@adamhooper got it, very appreciate you!
If the initial date and the selected date are not in the same timezone (e.g. summer time vs standard time) the value of the DatePicker will be incorrect. DatePicker value is either the day before the date selected at 23:00, or, the correct day but with time set to 01:00.
Environment
Steps to reproduce
Expected behavior
Actual behavior
Live example
codesandbox
Sorry, I've not had time to look and see if I can fix this myself. I will do that but I've spent all the time I can today making the repro sandbox and figuring out what was going on. I will look into it if no-one else does over the weekend. My guess would be that the picker change code is just setting the date part and leaving the time part that was set by initialising the picker to midnight UTC on the initial date. I would also guess that to fix it one could just set the date to midnight UTC on the selected date as well and that should account for whatever timezone that new date happens to be in. Thanks! :)