mariusmuntean / ChartJs.Blazor

Brings Chart.js charts to Blazor
https://www.iheartblazor.com/
MIT License
691 stars 152 forks source link

Inconsistent timezones for client-size Blazor #104

Closed dfederm closed 3 years ago

dfederm commented 4 years ago

Describe the bug

Due to a bug in client-side Blazor (https://github.com/dotnet/aspnetcore/issues/17017), the current timezone is always in UTC. However, the underlying date handler Chart JS uses (moment) handles the local timezone just fine. This causes inconsistency and the need to manually adjust times. However, even beyond this bug, the times given to Moment seem to need adjusting when using UTC times.

Which Blazor project type is your bug related to?

Which charts does this bug apply to?

Time charts

To Reproduce

Use a time chart :)

For example, add a data point with DateTime.Today or DateTime.UtcNow.Date, which should be midnight of the current day. On the graph I end up seeing the data point at yesterday 5pm (my local time is UTC-7).

Expected behavior

UTC times should be handled correctly

Screenshots

image

The console output is simply a Console.WriteLine of the dates as I add them to the datasets.

You can see that the time labels show 5pm. The tooltips seem correct in the screenshot (no TooltipFormat was provided), but if I provide "MMM D" the day is off by 1 due to the conversion to local time.

Additional context / logging

A potential solution would be for the the Moment ctor to look at the provided DateTime's DateTimeKind and adjust the time if it's UTC.

Code example

The time samples should be sufficient. They use random times though so the bug probably just wasn't noticed.

Joelius300 commented 4 years ago

Thanks for the report!

I'm a bit confused about this being a bug. From what I understand, this is the expected behaviour. Also I'm not sure what you mean by inconsistent. As far as I know, the times should be consistent for both client-side and server-side blazor (but not across both; that's because the time zones are handled differently).

We convert our datetimes (or moments) to ISO 8601 using the Round-trip ("O", "o") Format Specifier. These are then parsed by moment.js. This means the time zone information is preserved accross the C# <-> JavaScript barrier.
Since on client-side blazor, every datetime is represented as UTC time, it will also be a UTC time for chart.js. Chart.js automatically converts UTC time to local time when displaying time points (source). That's why you see them displayed at 5pm because that's what the local time would be according to your browser.

Do you understand what I mean? I'm not able to do any tests right now. Are you able to provide some more examples with datetimes of different DateTimeKinds?

dfederm commented 4 years ago

Looks like in the latest version of Blazor they implemented timezones so the bug is a bit more clear now.

Basically, the bug on the ChartJS.Blazor side is that it doesn't seem to look at the DateTimeKind. If I give it a UTC time vs the same exact time (semantically) but with DateTimeKind.Local, it treats them differently. For example, DateTime.Now and DateTime.UtcNow end up not being treated as the same date when crossing the boundary to JS. It seems like the DateTime given is always assumed to be local, so I have to ToLocalTime() all of my UTC dates when providing them.

Probably the fix is as easy as something like this on your end:

// Adjust the time to be a local time before passing off to JS
if (dateTime.Kind == DateTimeKind.Utc)
{
  dateTime = dateTime.ToLocalTime();
}
Joelius300 commented 4 years ago

I don't think our library treats them differently, that might be moment.js that does it. As already mentioned we use the o format specifier. However the docs of moment.js state that they first try to parse the time from a known ISO 8601 format (which we provide) and they also state that they support time parts:

If a time part is included, an offset from UTC can also be included as +-HH:mm, +-HHmm, +-HH or Z.

Here's some sample output in client-side blazor 3.1 pre4 in Firefox.

grafik

Here's the correspondig code:

<h3>DateTime.Now</h3>
<p>@now</p>
<h3>DateTime.UtcNow</h3>
<p>@nowUtc</p>
<h3>DateTime.Now.ToUniversalTime()</h3>
<p>@nowToUtc</p>
<h3>DateTime.UtcNow.ToLocalTime()</h3>
<p>@nowUtcToLocal</p>

@code
{
    private string now;
    private string nowUtc;
    private string nowToUtc;
    private string nowUtcToLocal;

    protected override void OnInitialized()
    {
        var dtNow = DateTime.Now;
        var dtNowUtc = DateTime.UtcNow;
        now = ToDisplayString(dtNow);
        nowUtc = ToDisplayString(dtNowUtc);
        nowToUtc = ToDisplayString(dtNow.ToUniversalTime());
        nowUtcToLocal = ToDisplayString(dtNowUtc.ToLocalTime());
    }

    private string ToDisplayString(DateTime dt)
    {
        return dt.ToString("o") + "  |  " + dt.Kind;
    }
}

Do you get different results? I still don't quite understand what the issue is. Maybe it would be of use to have a complete repro project.

Joelius300 commented 4 years ago

I'm still a bit lost on this. Have you tried my snippet? Do your results differ from mine?
Maybe we don't have to search in C# or Blazor.. Does moment.js or Chart.js handle dates differently depending on whether the timezone is denoted by a Z or an UTC offset?

Joelius300 commented 3 years ago

Closing this due to inactivity. Please respond if this issue should be reopened.

dfederm commented 3 years ago

This is no longer an issue as Blazor supports timezones at this point.

Joelius300 commented 3 years ago

Great 😁