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

fix(xy): switch default timezone to local #1544

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions integration/tests/timezone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,24 @@ describe('Time zone', () => {
'http://localhost:9001/?path=/story/scales--timezone-configuration&globals=theme:light&knob-dataset=utc-8&knob-tooltip=utc',
);
});
// skip until puppeteer is updated to > 4.x
it.skip('use default local timezone America/New_York', async () => {
// await page.emulateTimezone('America/New_York');
// time should start at 06:00 (UTC time is 11:00)
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/bar-chart--with-time-x-axis',
{ left: 80, top: 100 },
{ screenshotSelector: 'body' },
);
});
// skipped until puppeteer is updated to > 4.x
it.skip('use default local timezone Europe/Rome', async () => {
// await page.emulateTimezone('Europe/Rome');
// time should start at 12:00 (UTC time is 11:00)
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/bar-chart--with-time-x-axis',
{ left: 80, top: 100 },
{ screenshotSelector: 'body' },
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('X Domain', () => {
type: getXScaleTypeFromSpec(ScaleType.Linear),
nice: getXNiceFromSpec(),
isBandScale: true,
timeZone: 'utc',
timeZone: 'local',
});
});

Expand All @@ -58,7 +58,7 @@ describe('X Domain', () => {
type: getXScaleTypeFromSpec(ScaleType.Ordinal),
nice: getXNiceFromSpec(),
isBandScale: true,
timeZone: 'utc',
timeZone: 'local',
});
});

Expand All @@ -74,7 +74,7 @@ describe('X Domain', () => {
type: getXScaleTypeFromSpec(ScaleType.Linear),
nice: getXNiceFromSpec(),
isBandScale: false,
timeZone: 'utc',
timeZone: 'local',
});
});
test('Should return correct scale type with single line (time)', () => {
Expand Down Expand Up @@ -132,7 +132,7 @@ describe('X Domain', () => {
type: getXScaleTypeFromSpec(ScaleType.Time),
nice: getXNiceFromSpec(),
isBandScale: false,
timeZone: 'utc',
timeZone: 'local',
});
});

Expand All @@ -152,7 +152,7 @@ describe('X Domain', () => {
type: getXScaleTypeFromSpec(ScaleType.Ordinal),
nice: getXNiceFromSpec(),
isBandScale: false,
timeZone: 'utc',
timeZone: 'local',
});
});
test('Should return correct scale type with multi bar, area with different scale types (linear, ordinal)', () => {
Expand All @@ -171,7 +171,7 @@ describe('X Domain', () => {
type: getXScaleTypeFromSpec(ScaleType.Ordinal),
nice: getXNiceFromSpec(),
isBandScale: true,
timeZone: 'utc',
timeZone: 'local',
});
});
test('Should return correct scale type with multi bar, area with same scale types (linear, linear)', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/charts/src/chart_types/xy_chart/domains/x_domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export function findMinInterval(xValues: number[]): number {
* Convert the scale types of a set of specification to a generic one.
* If there are at least one `ordinal` scale type, the resulting scale is coerced to ordinal.
* If there are only `continuous` scale types, the resulting scale is coerced to linear.
* If there are only `time` scales, we coerce the timeZone to `utc` only if we have multiple
* If there are only `time` scales, we coerce the timeZone to `local` only if we have multiple
* different timezones.
* @returns the coerced scale type, the timezone and a parameter that describe if its a bandScale or not
* @internal
Expand All @@ -167,6 +167,6 @@ export function convertXScaleTypes(
: ScaleType.Linear; // if Ordinal is not present, coerce to Linear, whether it's present or not
const nice = !niceDomains.includes(false);
const isBandScale = seriesTypes.has(SeriesType.Bar);
const timeZone = timeZones.size === 1 ? timeZones.values().next().value : 'utc';
const timeZone = timeZones.size === 1 ? timeZones.values().next().value : 'local';
return { type, nice, isBandScale, timeZone };
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ describe('Chart State utils', () => {
isBandScale: false,
minInterval: 1,
logBase: 10,
timeZone: 'utc',
// the default timezone (local) is resolved in computeSeriesDomains fn with Intl functions
timeZone: Intl.DateTimeFormat().resolvedOptions().timeZone,
}),
);
expect(domains.yDomains).toEqual([
Expand Down Expand Up @@ -126,7 +127,8 @@ describe('Chart State utils', () => {
isBandScale: false,
minInterval: 1,
logBase: 10,
timeZone: 'utc',
// the default timezone (local) is resolved in computeSeriesDomains fn with Intl functions
timeZone: Intl.DateTimeFormat().resolvedOptions().timeZone,
}),
);
expect(domains.yDomains).toEqual([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1421,6 +1421,7 @@ describe('Axis computational utils', () => {
isBandScale: false,
domain: [1547190000000, 1547622000000],
minInterval: 86400000,
timeZone: 'utc',
});
const scale: Scale<number | string> = computeXScale({
xDomain: xDomainTime,
Expand Down Expand Up @@ -1575,6 +1576,7 @@ describe('Axis computational utils', () => {
isBandScale: false,
domain: [1547190000000, 1547622000000],
minInterval: 86400000,
timeZone: 'utc',
});
const scale = computeXScale({ xDomain: xDomainTime, totalBarsInCluster: 0, range: [0, 603.5] });
const offset = 0;
Expand Down
3 changes: 2 additions & 1 deletion packages/charts/src/chart_types/xy_chart/utils/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,7 @@ export interface SeriesScales {
* compute a nice set of xScale ticks. Can be any IANA zone supported by
* the host environment, or a fixed-offset name of the form 'utc+3',
* or the strings 'local' or 'utc'.
* @defaultValue `local`
*/
timeZone?: string;
/**
Expand Down Expand Up @@ -727,7 +728,7 @@ export interface AxisSpec extends Spec {
groupId: GroupId;
/** Hide this axis */
hide: boolean;
/** shows all ticks and gridlines, including those belonging to labels that got culled due to overlapping with other labels*/
/** shows all ticks and gridlines, including those belonging to labels that got culled due to overlapping with other labels */
showOverlappingTicks: boolean;
/** Shows all labels, also the overlapping ones */
showOverlappingLabels: boolean;
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/src/mocks/xy/domains.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class MockXDomain {
...X_SCALE_DEFAULT,
isBandScale: X_SCALE_DEFAULT.type !== ScaleType.Ordinal,
minInterval: 0,
timeZone: 'UTC',
timeZone: 'local',
domain: [0, 1],
};

Expand Down
4 changes: 2 additions & 2 deletions packages/charts/src/scales/scale_continuous.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const SCALES = {
const defaultScaleOptions: ScaleOptions = {
bandwidth: 0,
minInterval: 0,
timeZone: 'utc',
timeZone: 'local',
totalBarsInCluster: 1,
barsPadding: 0,
constrainDomainPadding: true,
Expand Down Expand Up @@ -349,7 +349,7 @@ type ScaleOptions = Required<LogScaleOptions, 'logBase'> & {
/**
* A time zone identifier. Can be any IANA zone supported by he host environment,
* or a fixed-offset name of the form 'utc+3', or the strings 'local' or 'utc'.
* @defaultValue `utc`
* @defaultValue `local`
*/
timeZone: string;
/**
Expand Down