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

[YearPicker] 1 is extracted from year when timezone is +00:00 #6616

Open
2 tasks done
Oktay28 opened this issue Oct 24, 2022 · 22 comments
Open
2 tasks done

[YearPicker] 1 is extracted from year when timezone is +00:00 #6616

Oktay28 opened this issue Oct 24, 2022 · 22 comments
Labels
component: pickers This is the name of the generic UI component, not the React module! l10n localization

Comments

@Oktay28
Copy link

Oktay28 commented Oct 24, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

When "2017" or new Date("2017") is passed as value to DatePicker with views ['year'], the displayed value in the input is 2016. This happens when the timezone is +00:00

timezone +03:00 : displayed value is 2017 when the given value is "2017". Displayed value is same as given value

timezone +00:00 : displayed value is 2016 while the given value is "2017"

Expected behavior 🤔

Displayed value to be exactly the same as the given value

Steps to reproduce 🕹

Pass value="2017" and views={['year']} to MobileDatePicker. Timezone should be +00:00
Adapter: AdapterDayjs or AdapterDateFns

Context 🔦

I receive integer from server to use it as value. I convert it to string, because integer as value is for milliseconds. The truth is "2017", but it is displayed as 2016

Your environment 🌎

npx @mui/envinfo
System:
    OS: Windows 10 10.0.19044
  Binaries:
    Node: 17.4.0 - C:\Program Files\nodejs\node.EXE
    Yarn: Not Found
    npm: 8.3.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: Not Found
    Edge: Spartan (44.19041.1266.0), Chromium (106.0.1370.52)
  npmPackages:
    @emotion/react: ^11.4.1 => 11.9.3
    @emotion/styled: ^11.3.0 => 11.9.3
    @mui/base:  5.0.0-alpha.89
    @mui/icons-material: ^5.6.0 => 5.8.4
    @mui/lab: ^5.0.0-alpha.76 => 5.0.0-alpha.90
    @mui/material: ^5.6.0 => 5.9.0
    @mui/private-theming:  5.9.0
    @mui/styled-engine:  5.8.7
    @mui/styles: ^5.6.0 => 5.9.0
    @mui/system:  5.9.0
    @mui/types:  7.1.4
    @mui/utils:  5.10.9
    @mui/x-data-grid: ^5.6.1 => 5.13.1
    @mui/x-date-pickers: ^5.0.4 => 5.0.4
    @types/react: ^17.0.39 => 17.0.47
    react: ^17.0.2 => 17.0.2
    react-dom: ^17.0.2 => 17.0.2
    typescript: ^4.5.5 => 4.7.4
@Oktay28 Oktay28 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 24, 2022
@zannager zannager added the component: pickers This is the name of the generic UI component, not the React module! label Oct 24, 2022
@yaredtsy
Copy link
Contributor

hi @Oktay28, I could not recreate this issue. check this demo https://codesandbox.io/s/dawn-sea-k1uprr?file=/demo.tsx

@alexfauquette alexfauquette added status: waiting for author Issue with insufficient information l10n localization and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 24, 2022
@alexfauquette
Copy link
Member

@Oktay28 there are multiple ways to manage timezone, so we would need a codesandbox to see how you do it. The one proposed by @yaredtsy is a good starting point.

There is another issue where you could find some information about timezone management #6545

@Oktay28
Copy link
Author

Oktay28 commented Oct 24, 2022

@yaredtsy is your timezone +00:00 ? format(new Date(), 'z') (format from 'date-fns') should return 'GMT+0' . Only then there is a difference. This behaviour occurs on that timezone

@alexfauquette I do not manage it, I just pass "2017" as value and because of my timezone it is displayed as 2016

I use this for localication: <LocalizationProvider dateAdapter={AdapterDateFns}>...</..>

@alexfauquette
Copy link
Member

The problem might be in the string parsing. timezone management of Date() when parsing string is buggy (behavior depends on the browser). What if you pass value={new Date(2017)} ?

@Oktay28
Copy link
Author

Oktay28 commented Oct 24, 2022

Both cases work the same way. 2017 is not integer in both cases, because integer is for milliseconds. "2017" or new Date("2017") as value result to same displayed value "2016".

new Date("2017").getFullYear()  returns 2016
new Date("2017-02-02").getFullYear() returns 2017
new Date("2017").getUTCFullYear() returns 2017

I think the issue comes from here. If I pass "2017-02-02" I get displayed value 2017. But when it is only the year "2017", the displayed value is 2016

@alexfauquette
Copy link
Member

Ok, so setting a full date and not only a year solves your problem?

@Oktay28
Copy link
Author

Oktay28 commented Oct 24, 2022

I only explained what I think is causing the problem. I do not want to format the value like this. The server sends me the truth. The truth is 2017. I do not want to make string or date manipulations to hack that behaviour. Does it look normal to you when "2017" is passed the result to be 2016? is this the expected behaviour from the datepickers?

@alexfauquette
Copy link
Member

Is this the expected behavior from the datepickers?

The expected behavior is to should the full year provided.

The ability to pass a string is just sugar syntax. Behind the scene, all we do is parse the string you provided to value with the date library you chose. If parse(value).getFullYear() return 2016 yes the expected behavior is to show 2016

Since the parsing is done behind the scene lot of people hope it would work magically. That's why in v6 we will remove the string support.


I do not want to make string or date manipulations to hack that behavior.

Since date-fns is using Date object, you should respect the specified format to make it work.

Form Date.parse doc

Only the ISO 8601 format (YYYY-MM-DDTHH:mm:ss.sssZ) is explicitly specified to be supported

@Oktay28
Copy link
Author

Oktay28 commented Oct 24, 2022

Passing value as string or Date does not matter for me. The problem is that I do not want to convert it to "2017-02-01" or date.setMonth(1) // (Feb) before passing it as value.

When new Date("2017") is provided as value to DateTimePicker the displayed value is "12/31/2016 11:00 pm" with timezone +00:00
Is this the expected output?

I may do some tricks to add month to the "2017" as "2017-02-01" to make it think it is Feb so I can get the right year.

But how I am supposed to store that value "2017-01-01T00:00:00Z" and use it on DateTimePicker

What solution you recommend for passing value as "2017" or new Date("2017") and get the same displayed value as the value prop?

@alexfauquette
Copy link
Member

For timezone issues with date-fns, better to ask directly on date-fns. Here you have some StackOverflow question related to your problem of parsing a string with timezone.

@LukasTy
Copy link
Member

LukasTy commented Oct 24, 2022

I tend to agree—timezones and it's management is generally tricky and always has some edge-cases.
I've changed my computer timezone to Azores Summer Time (UTC+0) and was able to reproduce the mentioned issue using only AdapterDateFns. Using AdapterDayjs does not produce the same issue.

Consider the examples below. They clearly illustrate that the date is not initialized to the value you would expect—unless time is also provided.

new Date('2017'); 
// Sat Dec 31 2016 23:00:00 GMT-0100 (Azores Standard Time)

new Date('2017-01-01'); 
// Sat Dec 31 2016 23:00:00 GMT-0100 (Azores Standard Time)

new Date('2017-01-01T00:00');
// Sun Jan 01 2017 00:00:00 GMT-0100 (Azores Standard Time)

@Oktay28 Could you please consider initializing the date with the expected format (as @alexfauquette has mentioned in the spec—JS Date expects a date in ISO format, and it's minimal form is YYYY-MM-DDTHH:mm) or use a more elaborate date management library (such as dayjs) to avoid such cases if you want simplicity?

@Oktay28
Copy link
Author

Oktay28 commented Oct 24, 2022

I changed my timezone via extension for Chrome. Neither that format YYYY-MM-DDTHH:mm or dayjs helped me. It was still displayed as 2016.

When I tested changing it via Chrome dev tools both solutions worked.

I am not sure in real situation what people gonna get as displayed value. It seems like dayjs or format YYYY-MM-DDTHH:mm are not always working

@LukasTy
Copy link
Member

LukasTy commented Oct 24, 2022

I am not sure in real situation what people gonna get as displayed value.

I can confirm that changing timezone using extension and timezone settings on Mac did behave differently.
This is what I actually mentioned when saying that timezone management is not a trivial topic. 😞

What do you think about opting to use UTC time explicitly by suffixing Z to the date string?

new Date('2017-01-01T00:00:00Z');
// Sun Jan 01 2017 00:00:00 GMT-0100 (Azores Standard Time)

This consistently produces same result in both computer changed time zone as well as time zone changed by an extension.

@Oktay28
Copy link
Author

Oktay28 commented Oct 24, 2022

I have already tried that.

new Date("2017-01-01T00:00:00Z").getFullYear() returns 2016

@yaredtsy
Copy link
Contributor

when you change the timezone, the time also changes. if your local time is 2017-01-01T00:00:00 it won't be the same everywhere. you are getting 2016-31-12 11:00 because your timezone offset is 1 hour.

@LukasTy
Copy link
Member

LukasTy commented Oct 25, 2022

I have already tried that.

Okay, I've done more testing and reading around the Date parsing docs.
By the looks of it—something along these lines should be the most fool-proof solution.
Not using string parsing makes sure the date is not time offset.

P.S. Do not use browser extension to test this change as we are not sure what it is actually doing—its also producing strange results for me.
Using the actual device timezone changing is the most correct approach to test/replicate other users in different timezones.

@oliviertassinari oliviertassinari removed the status: waiting for author Issue with insufficient information label Oct 29, 2022
@famoser
Copy link

famoser commented Feb 1, 2023

IMO the core issue is here that there are two different ways how dates can be understood:

  • As point in time (e.g. date when something was edited), hence a full timestamp including time. This is what mui currently implements.
  • As date (e.g. birthdate). Only the date is relevant, the time is not. Notably, this is different to a point in time, as the date is the same independent of timezones: My birthday begins at midnight and ends at midnight on the same day, for people in both Mexico City (GMT-6) as well as Kiev (GMT+2).

The latter use-case is hard to implement with mui at the moment. To do this properly:

  • the date has to be converted to local time when passing to mui (else different timezones see different dates).
  • and back to UTC time when getting it from mui (else different timezones store different dates).

Example

Sandbox: https://codesandbox.io/s/relaxed-mirzakhani-6tsyym?file=/demo.tsx

The upper is the date picker used as is, the lower is a modified version with the fixes detailed above. Then, I edit the date in both textfields (with the picker the timezone is not set, hence this issue does not occur). Now, the sandbox shows in UTC+1 wintertime (so UTC+2):
image

Note that this sandbox only shows the weird behavior around storing the date. A similar problem occurs when reading the date; e.g. in GMT-6 the date picker would show the date before the expected date (as midnight UTC is in the previous day in GMT-6).

Proposed fix

A clean solution would be to explicitly separate the behaviors where a timestamp is needed, and where it is not. In the latter, the DatePicker should not expose nor work with timestamps, but only on the date (holds all equivalent components, e.g. TimePicker). Possibly the component needs to even exist twice, in a form where it works on a timestamp and in a form where it does not. This is obviously a heavily breaking change, and maybe there is a more elegant solution to this (what do other libraries do?).

The documentation could however at least be extended in that way. There is already an issue for this: #7769

@LukasTy
Copy link
Member

LukasTy commented Feb 2, 2023

@famoser If I'm not mistaken, you are talking about the need to have UTC time (without time zone offset).
We have added a section to our v6 (next) documentation with an example of how to use UTC time with dayjs library. If you are not tied to using date-fns—you might have a go at it.

@famoser
Copy link

famoser commented Feb 2, 2023

@LukasTy Thank you for the link! Although it still requires wrapping the DatePicker, it would indeed be a more elegant solution compared to the example given in the Sandbox (if dayjs can be used).

However, this would still be a timestamp, and therefore not fully capture the second use-case I've mentioned above (e.g. birthdate). A concrete example here would be a user choosing their birthday, resulting in the timestamp 1990-01-01T00:00:00.000Z. But this includes the time, which is not relevant for birthdays (and was not picked by the user).

Instead, the DatePicker should just expose 1990-01-01 (ISO date). This prevents confusion later; e.g. rendering the timestamp relative to local time, which would lead to 12/31/1989 shown in Mexico (GMT-6).

@LukasTy
Copy link
Member

LukasTy commented Feb 2, 2023

However, this would still be a timestamp, and therefore not fully capture the second use-case I've mentioned above (e.g. birthdate). A concrete example here would be a user choosing their birthday, resulting in the timestamp 1990-01-01T00:00:00.000Z. But this includes the time, which is not relevant for birthdays (and was not picked by the user).

JavaScript and most date manipulation libraries for it operate on the notion that Date is a timestamp, rather than having a distinction between Date, Time and/or DateTime, like you could have with other languages (i.e. Java).
How would you suggest managing this kind of date? Passing/exposing only ISO string version of it?
If this management is strictly necessary, I'd assume that writing a custom adapter to manage the date in this desired way should be possible.

Instead, the DatePicker should just expose 1990-01-01 (ISO date). This prevents confusion later; e.g. rendering the timestamp relative to local time, which would lead to 12/31/1989 shown in Mexico (GMT-6).

If your code assumes that you are operating with UTC dates in every step along the data chain—you shouldn't end up in the mentioned case.
I.e. You assume that BE returns data in ISO date string, then you can safely use UTC date in FE to render the same date string formatted in the desired way. 🤔

@famoser
Copy link

famoser commented Feb 2, 2023

Thank you @LukasTy, those are some good points.

I think in my ideal world I'd like to work on the ISO string directly, because it prevents the kind of confusion I certainly had, but which also seems to be core issue behind this whole thread. But on the other hand, a good point is that

JavaScript (...) operate[s] on the notion that Date is a timestamp

so not sure if Mui doing something different is such a good idea.


You assume that BE returns data in ISO date string, then you can safely use UTC date in FE to render the same date string formatted in the desired way.

This is what we are going for now; but this behavior is subtle. Everyone has to "know" this is a UTC date and not a real timestamp, so use different formatting functions.


So to summarize:

  • There is a rather straightforward way to enforce UTC dates being picked (using dayjs), which is already documented (at least for the next version of mui).
  • The behavior of mui fits into how JavaScript handles dates usually.

Thank you for your help.

@kkirsche
Copy link

I also ran into this with date-fns on the DatePicker and found the current behavior on v5 unintuitive. I'll review the v6 docs, but I wanted to note for the MUI team (e.g., for prioritization purposes) that I ended up here as well because of this type of issue and couldn't find a section in the docs that seemed to address it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! l10n localization
Projects
None yet
Development

No branches or pull requests

8 participants