getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

Date Field: Highlighted "today" day seems to always be tomorrow #3089

Closed neildaniels closed 2 years ago

neildaniels commented 3 years ago

Describe the bug
On a date field, the indicated "today" day (in orange) does not reflect the local day. My PHP server and locale machine are both in the Pacific time zone (America/Los_Angeles). My computer's clock shows that it's Thursday, January 14. However, the date popover highlights Friday, January 15. (In UTC time, it is January 15.)

Possibly a continuation of #3005, or a result of #3075.

Kirby 3.4.5 highlights the expected day of "Thursday, January 14."

          created:
            label: Published Date
            type: date
            translate: false
            time:
              step: 1
              notation: 12
            width: 1/3

To Reproduce
Steps to reproduce the behavior:

  1. Use above date field
  2. Set your server's time zone to something that would cause the day to wrap to something else (probably relative to UTC)
  3. Click into a blank date field

Expected behavior
"Today" date should be reflective of the server's time zone, not UTC.

Screenshots

Screen Shot 2021-01-14 at 5 10 51 PM

Kirby Version
3.5.1-rc.1

Desktop (please complete the following information):

distantnative commented 3 years ago

What timezone is your browser in? Cause that one should be the defining one, not your servers.

neildaniels commented 3 years ago

@distantnative My browser/computer is in the America Pacific time zone ("Los Angeles, California").

To be clear though, Kirby 3.4.x exclusively used the server time zone for all Panel date reasoning. I could be wrong on specifically what "day" is "today," but whatever date and/or time was set/displayed in the field was relative to the server time, not the browser's time.

Kirby 3.4.x Example: My server is set to American Pacific Standard Time (PST) (-8 hours from UTC).

If an editor in American Eastern Standard Time (EST) (-5 hours from UTC) uses the Panel, all dates and times will still be relative to PST.


Taking into account the browser's time zone into a date field is a significant departure from 3.4.x behavior. There are some cases where that is super useful, but I think that should be a non-default configuration option.

The stored value in the content file should definitely remain in the server's time though (or, at least record what the time zone offset is).

distantnative commented 3 years ago

You are wrong about 3.4 - we've always used dayjs to also get the current time (we didn't call the server when you click today to ask what today is for the server). What changed is that we're using a lot UTC in dayjs now because there were a lot of other timeline problems in pre 3.5 at saving a value.

So it seems that there are still issues :D that much I agree.

But regarding browser time, you need to consider sites with editors across multiple time zones. I think rarely an editor means "get the date of where my server is" when hitting today, they most likely mean "get the date I am located in currently". And to my understanding, browser timezone is much more likely to match that than server timezone .g. if I'm sitting in Thailand but working for a site with a server in Germany.

distantnative commented 3 years ago

BTW the stored value in the field is completely agnostic to a timezone. It has none. But this is where it becomes tricky when relating it to something like "today"/"now" which clearly is related to a person's timezone.

neildaniels commented 3 years ago

Yes, I agree that a user clicking a “Today” button would expect it to match their system’s current day, not the servers.

I was more concerned that Kirby 3.5.x was trying to be overly smart and make adjustments to the displayed date/time to be “correct” for the user’s time zone. It sounds like that’s not the case. Sorry for the confusion there.

However, in regards to my originally reported issue (and to re-answer your question): considering that computer/browser are in PST, it should have showed Jan 14 as the current day, not Jan 15.

neildaniels commented 3 years ago

This actually doesn't seem to be timezone related at all.

It's now Friday, January 15, 2021 9:16 am PST (-8 hours from UTC) and the Kirby date picker shows the "current" day as Saturday, January 16. The current UTC time is actually Friday, January 15, 2021 5:16 pm UTC.

Screen Shot 2021-01-15 at 9 19 47 AM

This might just be an off-by-one bug, not timezone related.

Interestingly, actually clicking the "Today" button correctly inserts January 15.

afbora commented 3 years ago

@neildaniels Do you think new use of local time is better than UTC? Or should we just revert the local time improvement for now? 🤔

neildaniels commented 3 years ago

@afbora Based on my most recent comment, I don't think this has anything to do with local/UTC time. In that case it wasn't January 16 "anywhere" (except for totally no relevant locales like New Zealand).

neildaniels commented 3 years ago

But, to be honest, if I could revert to the 3.4.x date-time field, I would at this point.

It wasn't "perfect" but it didn't seem broken, at least for my team's purposes.

The only outward-facing change in the 3.5.x date-time field seems to be a customizable display formatting of the date time, but we found the the default styling sufficient in 3.5.x. (My editors also preferred the drop down style interaction.)

Is there an example use-case I'm missing? Where the changes are materially different?

That being said, this issue seems to be one of the last with the 3.5.x date-time for me, so if fixing this (seemingly cosmetic issue), then I'm fine sticking with the 3.5.x version.

distantnative commented 3 years ago

@neildaniels is the main difference between 3.4 and 3.5 or are you also seeing a difference from 3.5.0 to 3.5.1?

neildaniels commented 3 years ago

@distantnative I'm mostly comparing 3.4.x and 3.5.1 at this point.

Time handling was so broken in 3.5.0 for us, that I basically punted on that entire release.

distantnative commented 2 years ago

We will remove all UTC references in 3.6.2 and handle timezones differently. I expect that this will solve the mentioned issue (I couldn't reproduce it before, so I cannot test it right now). Please get in touch if after the release of 3.6.2 this issue remains. Thank you (especially for the patience)!