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

bug build x axis using getTimezoneOffset #4951

Closed
hugomrdias opened this Issue Jan 22, 2016 · 6 comments

Comments

Projects
None yet
4 participants
@hugomrdias

hugomrdias commented Jan 22, 2016

When building the x axis, getTimezoneOffset method is called with a strange timestamp, but when building datalabels or tooltips everything seems ok.

example for DST change in 'Europe/Lisbon':
http://jsbin.com/seralet/14/edit?js,console

problem:
x axis has day 25 twice and tooltips and datalabels show the correct datetime

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Jan 25, 2016

Collaborator

What happens is that the tick for the 26th is actually placed one hour too early (24h after the last tick), so the date becomes wrong. You see that clearly from this screenshot. Also note that both the tick placement and the point placement is correct, the only problem is that we would expect the tick to be at midnight.

skjermbilde 2016-01-25 kl 12 09 22

Collaborator

TorsteinHonsi commented Jan 25, 2016

What happens is that the tick for the 26th is actually placed one hour too early (24h after the last tick), so the date becomes wrong. You see that clearly from this screenshot. Also note that both the tick placement and the point placement is correct, the only problem is that we would expect the tick to be at midnight.

skjermbilde 2016-01-25 kl 12 09 22

@VilmaRamos

This comment has been minimized.

Show comment
Hide comment
@VilmaRamos

VilmaRamos Jan 25, 2016

Now the result is as I was expecting but trying to achieve the same result using only the pointStart and pointInterval properties didn't work. The following example illustrates the problem: http://jsbin.com/tojakel/2/edit?html,js,output .
Is it possible to define the point intervals using an irregular time interval? Maybe something like:

series: {
    pointStart: 1445641200000,
    pointInterval: 1,
    pointIntervalUnit: 'day'
}

VilmaRamos commented Jan 25, 2016

Now the result is as I was expecting but trying to achieve the same result using only the pointStart and pointInterval properties didn't work. The following example illustrates the problem: http://jsbin.com/tojakel/2/edit?html,js,output .
Is it possible to define the point intervals using an irregular time interval? Maybe something like:

series: {
    pointStart: 1445641200000,
    pointInterval: 1,
    pointIntervalUnit: 'day'
}
@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Jan 26, 2016

Collaborator

Good point! The current defination of pointIntervalUnit only covers month and year, but in order to cover DST crossover it should also cover day.

Collaborator

TorsteinHonsi commented Jan 26, 2016

Good point! The current defination of pointIntervalUnit only covers month and year, but in order to cover DST crossover it should also cover day.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Jan 26, 2016

Collaborator

Since it is unrelated to the axis label placement, I added an issue at #4958.

Collaborator

TorsteinHonsi commented Jan 26, 2016

Since it is unrelated to the axis label placement, I added an issue at #4958.

@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Oct 14, 2016

Contributor

It's still a little buggy: http://jsfiddle.net/1mtv71bx/

On the xAxis we can see 6. Nov twice: once at 00:00 and later at 23:00. All next daily higherRanks are translated by 1h to the left.

Contributor

pawelfus commented Oct 14, 2016

It's still a little buggy: http://jsfiddle.net/1mtv71bx/

On the xAxis we can see 6. Nov twice: once at 00:00 and later at 23:00. All next daily higherRanks are translated by 1h to the left.

@pawelfus pawelfus reopened this Oct 14, 2016

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Oct 17, 2016

Collaborator

Internal note
The problem lies in these lines that don't take into consideration that the axis can cross over DST even if the interval is smaller than days and weeks.

We need a separate clause below the day/week clause, that applies to hours and less. If the minDate and max lies in different time zones, we need to compute the time using makeTime like higher order times. Pseudo code, in real implementation we need to replace hours, possibly day, and minutes to support half-hour time zones:

var crossingDST = getTZOffset(minDate) !== getTZOffset(max);
[...]
            // if we're using global time, the interval is not fixed as it jumps
            // one hour at the DST crossover
            } else if (variableDayLength && (interval === timeUnits.day || interval === timeUnits.week)) {
                time = makeTime(minYear, minMonth, minDateDate +
                    i * count * (interval === timeUnits.day ? 1 : 7));

            } else if (variableDayLength && crossingDST) {
                time = makeTime(minYear, minMonth, day, hour, minute, second);
            }
Collaborator

TorsteinHonsi commented Oct 17, 2016

Internal note
The problem lies in these lines that don't take into consideration that the axis can cross over DST even if the interval is smaller than days and weeks.

We need a separate clause below the day/week clause, that applies to hours and less. If the minDate and max lies in different time zones, we need to compute the time using makeTime like higher order times. Pseudo code, in real implementation we need to replace hours, possibly day, and minutes to support half-hour time zones:

var crossingDST = getTZOffset(minDate) !== getTZOffset(max);
[...]
            // if we're using global time, the interval is not fixed as it jumps
            // one hour at the DST crossover
            } else if (variableDayLength && (interval === timeUnits.day || interval === timeUnits.week)) {
                time = makeTime(minYear, minMonth, minDateDate +
                    i * count * (interval === timeUnits.day ? 1 : 7));

            } else if (variableDayLength && crossingDST) {
                time = makeTime(minYear, minMonth, day, hour, minute, second);
            }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment