fix(timescale): consider timezone on axis ticks #151

markov00 · 2019-04-05T10:19:18Z

Summary

The d3-scaleTime and d3-scaleUtc functions, used to compute axis ticks, compute a discrete and nice rounded number of ticks on a time scale. This rounding (the start of a day, an hour, a month, a year depending on the data interval) is applied to UTC or local timezones, meaning that we cannot display a nicely rounded tick if we want to display data in a timezone different from the local or utc one.

This PR includes a new optional prop to each series timeZone that can be used to configure this behaviour (default to utc).
To enable a nice rounded ticks on d3-scaleUtc on any timezone we followed this approach:

your current status:

my local timezone is UTC+1
Kibana, or what else, is configured to use UTC-6 as timezone.
I want to visualize my data on that timezone, that means: if my dataset is composed by 3 days from 00:00:00 of the 1st january to 00:00:00 of 3rd of january in UTC-6 I want visualize 3 data points, one on the left edge, one in the middle and the last in the right edge of my chart.
I also want to visualize the relative ticks on the bottom axis whenever possible, on the same timezone of my data, means I want to visualize at least 3 ticks each one at the midnight of the 3 days.
my data looks like the following:

[
  ['2014-01-01T00:00:00.000-06:00', 1388556000000, 6206],
  ['2015-01-01T00:00:00.000-06:00', 1420092000000, 5674],
  ['2016-01-01T00:00:00.000-06:00', 1451628000000, 4148],
  ['2017-01-01T00:00:00.000-06:00', 1483250400000, 6206],
  ['2018-01-01T00:00:00.000-06:00', 1514786400000, 3698],
]

the d3 scaleUtc scale is implemented to display nicely rounded axis ticks, rounded to hours, days, months days, depending on the domain extent.
This means that if my domain is [1388556000000,1514786400000] scaleUtc will find the nice time based on UTC timezone, that means:

t0 => 2014-01-01T00:00:00.000-06:00 in milliseconds is 1388556000000
1388556000000 in UTC is 2014-01-01T06:00:00.000Z
scaleUtc will compute the ticks and will round that nice value to 2014-04-01T00:00:00.000Z
this means that rounded time is - 6 hours from the starting point t0 1388556000000 and graphically it will represented as the following

            t0
             |
--------------------------
  |
t0-6

no matter what type of formatting you are applying to the text label, the positioning will be always shifted by -6 hours

To fix this, the idea is to apply a shift only on the axis scale. We want to shift the nice rounded value to the right place:

shift the domain back by the same utc offset:
from [1388556000000,1514786400000] to [1388556000000 ,1514786400000]

WIP

fix #130

Display data with UTC-6 using UTC-6 as display timezone (the screenshot local timezone is UTC+1):

Display UTC timezoned data using UTC as display timezone

Display UTC timezoned data using local timezone (UTC+1) as display timezone (data is shifted to +1 but the ticks are nicely kept to UTC+1 rounded days

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~[ ] This was checked for cross-browser compatibility, including a check against IE11~~
Proper documentation or storybook story was added for features that require explanation or tutorials
Unit tests were updated or added to match the most common scenarios
Each commit follows the convention

The d3-scale function, used to compute axis ticks, compute a discrete and nice rounded number of ticks on a time scale. By the way, this rounding is applied using UTC or local timezone, meaning that we cannot display a nicely rounded tick if we want to display data in a timezone different from the local or utc one. This commit includes a new optional prop to each series `timeZone` that can be used to configure this behaviour (default to utc). fix elastic#130

codecov-io · 2019-04-05T10:23:52Z

Codecov Report

Merging #151 into master will increase coverage by 0.13%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
+ Coverage   94.03%   94.16%   +0.13%     
==========================================
  Files          34       34              
  Lines        1744     1766      +22     
  Branches      219      224       +5     
==========================================
+ Hits         1640     1663      +23     
  Misses         90       90              
+ Partials       14       13       -1

Impacted Files	Coverage Δ
src/lib/series/specs.ts	`100% <ø> (ø)`	⬆️
src/lib/axes/axis_utils.ts	`100% <100%> (ø)`	⬆️
src/lib/series/domains/x_domain.ts	`100% <100%> (ø)`	⬆️
src/lib/utils/scales/scale_continuous.ts	`90.12% <100%> (+1.08%)`	⬆️
src/lib/series/scales.ts	`100% <100%> (ø)`	⬆️
src/lib/utils/scales/scales.ts	`100% <100%> (ø)`	⬆️
src/lib/utils/domain.ts	`100% <0%> (ø)`	⬆️
src/state/chart_state.ts	`96.18% <0%> (+0.34%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77a352c...6cf8fbf. Read the comment docs.

emmacunningham

one minor naming suggestion, otherwise code LGTM (the tests are super for helping to understand the changes!)

emmacunningham · 2019-04-05T20:15:56Z

src/lib/utils/scales/scale_continuous.ts

@@ -179,10 +200,10 @@ export function linearStepAfter(invertedValue: number, minInterval: number): num
 * @param invertedValue the inverted value
 * @param minInterval the data minimum interval grether than 0
 */
-export function linearStep(invertedValue: number, minInterval: number): number {
-  const diff = invertedValue / minInterval;
+export function linearStep(minDomain: number, invertedValue: number, minInterval: number): number {


if I'm following correctly, minDomain represents the domain's minimum value instead of the minimum possible domain. so I wonder if it this parameter could be domainMin as opposed to minDomain (which, especially since another parameter is called minInterval, makes it seem like this is the minimum domain amount instead of the domain mininum). I guess the point I'm trying to get at is changing the name to reflect that in minInterval we are describing the distance between numbers, while the other parameter currently called minDomain is representing the minimum bound of a range of numbers.

maybe also we could add this param to the @param docs on the function to clarify what this value is (and that it comes from the computed domain).

emmacunningham · 2019-04-05T20:17:39Z

src/lib/utils/scales/scale_continuous.ts

@@ -147,7 +163,12 @@ export function isLogarithmicScale(scale: Scale) {
  return scale.type === ScaleType.Log;
 }

-function invertValue(invertedValue: number, minInterval: number, stepType?: StepType) {
+function invertValue(
+  minDomain: number,


similar to the comment on minDomain parameter in linearStep, a suggestion to comment what this parameter represents (and possibly change the name).

## [3.7.2](v3.7.1...v3.7.2) (2019-04-08) ### Bug Fixes * **timescale:** consider timezone on axis ticks ([#151](#151)) ([d860c97](d860c97)), closes [#130](#130)

markov00 · 2019-04-08T12:09:25Z

🎉 This PR is included in version 3.7.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

## [3.7.2](elastic/elastic-charts@v3.7.1...v3.7.2) (2019-04-08) ### Bug Fixes * **timescale:** consider timezone on axis ticks ([opensearch-project#151](elastic/elastic-charts#151)) ([e4ee93e](elastic/elastic-charts@e4ee93e)), closes [opensearch-project#130](elastic/elastic-charts#130)

markov00 added bug Something isn't working :axis Axis related issue :data Data/series/scales related issue :specs Chart specifications related issue labels Apr 5, 2019

markov00 requested a review from emmacunningham April 5, 2019 10:19

markov00 force-pushed the fix-timescales branch from cdc9206 to 7bccb24 Compare April 5, 2019 10:20

chore: remove unnecessary comments

4e2e998

emmacunningham approved these changes Apr 5, 2019

View reviewed changes

refactor: rename minDomain to domainMin

6cf8fbf

markov00 merged commit d860c97 into elastic:master Apr 8, 2019

markov00 pushed a commit that referenced this pull request Apr 8, 2019

chore(release): 3.7.2 [skip ci]

22659de

## [3.7.2](v3.7.1...v3.7.2) (2019-04-08) ### Bug Fixes * **timescale:** consider timezone on axis ticks ([#151](#151)) ([d860c97](d860c97)), closes [#130](#130)

markov00 added the released Issue released publicly label Apr 8, 2019

markov00 deleted the fix-timescales branch April 16, 2019 12:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(timescale): consider timezone on axis ticks #151

fix(timescale): consider timezone on axis ticks #151

markov00 commented Apr 5, 2019 •

edited

Loading

codecov-io commented Apr 5, 2019 •

edited

Loading

emmacunningham left a comment

emmacunningham Apr 5, 2019

emmacunningham Apr 5, 2019

markov00 commented Apr 8, 2019

fix(timescale): consider timezone on axis ticks #151

fix(timescale): consider timezone on axis ticks #151

Conversation

markov00 commented Apr 5, 2019 • edited Loading

Summary

Checklist

codecov-io commented Apr 5, 2019 • edited Loading

Codecov Report

emmacunningham left a comment

Choose a reason for hiding this comment

emmacunningham Apr 5, 2019

Choose a reason for hiding this comment

emmacunningham Apr 5, 2019

Choose a reason for hiding this comment

markov00 commented Apr 8, 2019

markov00 commented Apr 5, 2019 •

edited

Loading

codecov-io commented Apr 5, 2019 •

edited

Loading