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: build issues and tooltip formatting issues #810

Merged
merged 3 commits into from
Sep 8, 2020
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
1 change: 1 addition & 0 deletions api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1543,6 +1543,7 @@ export type TooltipType = $Values<typeof TooltipType>;
// @public
export interface TooltipValue {
color: Color;
formattedValue: string;
isHighlighted: boolean;
isVisible: boolean;
label: string;
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
29 changes: 25 additions & 4 deletions integration/tests/interactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,28 +232,49 @@ describe('Interactions', () => {
describe('Tooltip formatting', () => {
it('should use all custom tick formatters', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show%20legend=true&knob-Disable%20Y%20Axis%20tickFormat=&knob-Y%20Axis%20value%20format=0[.]0&knob-Y%20Axis%20unit=pets&knob-Disable%20dog%20line%20tickFormat=&knob-Dog%20line%20unit=dogs&knob-Disable%20cat%20line%20tickFormat=&knob-Cat%20line%20unit=cats',
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show%20legend_Y%20axis=true&knob-Disable%20Axis%20tickFormat_Y%20axis=&knob-Axis%20value%20format_Y%20axis=0[.]0&knob-Axis%20unit_Y%20axis=pets&knob-Disable%20header%20tickFormat_X%20axis=&knob-Header%20unit_X%20axis=(header)&knob-Disable%20Axis%20tickFormat_X%20axis=&knob-Axis%20unit_X%20axis=(axis)&knob-Disable%20dog%20line%20tickFormat_Y%20axis=&knob-Dog%20line%20unit_Y%20axis=dogs&knob-Disable%20cat%20line%20tickFormat_Y%20axis=&knob-Cat%20line%20unit_Y%20axis=cats',
{ left: 280, top: 80 },
);
});

it('should use series tick formatter with no axis tick formatter', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show%20legend=true&knob-Disable%20Y%20Axis%20tickFormat=&knob-Y%20Axis%20value%20format=0[.]0&knob-Y%20Axis%20unit=pets&knob-Disable%20dog%20line%20tickFormat=&knob-Dog%20line%20unit=dogs&knob-Disable%20cat%20line%20tickFormat=&knob-Cat%20line%20unit=cats',
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show legend_Y axis=true&knob-Disable Axis tickFormat_Y axis=true&knob-Axis value format_Y axis=0[.]0&knob-Axis unit_Y axis=pets&knob-Disable header tickFormat_X axis=&knob-Header unit_X axis=(header)&knob-Disable Axis tickFormat_X axis=&knob-Axis unit_X axis=(axis)&knob-Disable dog line tickFormat_Y axis=&knob-Dog line unit_Y axis=dogs&knob-Disable cat line tickFormat_Y axis=&knob-Cat line unit_Y axis=cats',
{ left: 280, top: 80 },
);
});

it('should use series tick formatter with no axis tick formatter, missing series tick formatter', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show%20legend=true&knob-Disable%20Y%20Axis%20tickFormat=true&knob-Y%20Axis%20value%20format=0[.]0&knob-Y%20Axis%20unit=pets&knob-Disable%20dog%20line%20tickFormat=true&knob-Dog%20line%20unit=dogs&knob-Disable%20cat%20line%20tickFormat=&knob-Cat%20line%20unit=cats',
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show legend_Y axis=true&knob-Disable Axis tickFormat_Y axis=true&knob-Axis value format_Y axis=0[.]0&knob-Axis unit_Y axis=pets&knob-Disable header tickFormat_X axis=&knob-Header unit_X axis=(header)&knob-Disable Axis tickFormat_X axis=&knob-Axis unit_X axis=(axis)&knob-Disable dog line tickFormat_Y axis=true&knob-Dog line unit_Y axis=dogs&knob-Disable cat line tickFormat_Y axis=&knob-Cat line unit_Y axis=cats',
{ left: 280, top: 80 },
);
});

it('should use default tick formatter with no axis tick formatter, nor series tick formatter', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show%20legend=true&knob-Disable%20Y%20Axis%20tickFormat=true&knob-Y%20Axis%20value%20format=0[.]0&knob-Y%20Axis%20unit=pets&knob-Disable%20dog%20line%20tickFormat=true&knob-Dog%20line%20unit=dogs&knob-Disable%20cat%20line%20tickFormat=true&knob-Cat%20line%20unit=cats',
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show legend_Y axis=true&knob-Disable Axis tickFormat_Y axis=true&knob-Axis value format_Y axis=0[.]0&knob-Axis unit_Y axis=pets&knob-Disable header tickFormat_X axis=&knob-Header unit_X axis=(header)&knob-Disable Axis tickFormat_X axis=&knob-Axis unit_X axis=(axis)&knob-Disable dog line tickFormat_Y axis=true&knob-Dog line unit_Y axis=dogs&knob-Disable cat line tickFormat_Y axis=true&knob-Cat line unit_Y axis=cats',
{ left: 280, top: 80 },
);
});

it('should use headerFormatter for x axis', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show legend_Y axis=true&knob-Disable Axis tickFormat_Y axis=&knob-Axis value format_Y axis=0[.]0&knob-Axis unit_Y axis=pets&knob-Disable header tickFormat_X axis=&knob-Header unit_X axis=(header)&knob-Disable Axis tickFormat_X axis=&knob-Axis unit_X axis=(axis)&knob-Disable dog line tickFormat_Y axis=&knob-Dog line unit_Y axis=dogs&knob-Disable cat line tickFormat_Y axis=&knob-Cat line unit_Y axis=cats',
{ left: 280, top: 80 },
);
});

it('should use axis tick formatter with no headerFormatter', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show legend_Y axis=true&knob-Disable Axis tickFormat_Y axis=&knob-Axis value format_Y axis=0[.]0&knob-Axis unit_Y axis=pets&knob-Disable header tickFormat_X axis=true&knob-Header unit_X axis=(header)&knob-Disable Axis tickFormat_X axis=&knob-Axis unit_X axis=(axis)&knob-Disable dog line tickFormat_Y axis=&knob-Dog line unit_Y axis=dogs&knob-Disable cat line tickFormat_Y axis=&knob-Cat line unit_Y axis=cats',
{ left: 280, top: 80 },
);
});

it('should use default tick formatter with no axis tick formatter nor headerFormatter', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/axes--different-tooltip-formatter&knob-Show legend_Y axis=true&knob-Disable Axis tickFormat_Y axis=&knob-Axis value format_Y axis=0[.]0&knob-Axis unit_Y axis=pets&knob-Disable header tickFormat_X axis=true&knob-Header unit_X axis=(header)&knob-Disable Axis tickFormat_X axis=true&knob-Axis unit_X axis=(axis)&knob-Disable dog line tickFormat_Y axis=&knob-Dog line unit_Y axis=dogs&knob-Disable cat line tickFormat_Y axis=&knob-Cat line unit_Y axis=cats',
{ left: 280, top: 80 },
);
});
Expand Down
17 changes: 7 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"semantic-release": "semantic-release",
"start": "yarn storybook",
"start-docs": "start-storybook -p 8001 -c .storybook-docs --ci",
"storybook": "RNG_SEED='elastic-charts' start-storybook -p 9001 -c .storybook --ci",
"storybook": "RNG_SEED='elastic-charts' start-storybook -p 9001 -c .storybook --ci --no-version-updates",
"storybook:build": "rm -rf .out && build-storybook -c .storybook -o .out",
"test": "jest --verbose --config jest.config.js",
"test:tz": "yarn test:tz-utc && yarn test:tz-ny && yarn test:tz-jp",
Expand Down Expand Up @@ -95,14 +95,14 @@
"@semantic-release/github": "^6.0.1",
"@semantic-release/npm": "^6.0.0",
"@semantic-release/release-notes-generator": "^7.3.5",
"@storybook/addon-actions": "5.3.19",
"@storybook/addon-actions": "^5.3.19",
"@storybook/addon-docs": "^5.3.19",
"@storybook/addon-info": "^5.3.19",
"@storybook/addon-knobs": "5.3.19",
"@storybook/addon-knobs": "^5.3.19",
"@storybook/addon-links": "^5.3.19",
"@storybook/addon-storysource": "^5.3.19",
"@storybook/preset-typescript": "^3.0.0",
"@storybook/react": "5.3.19",
"@storybook/react": "^5.3.19",
"@storybook/source-loader": "^5.3.19",
"@storybook/theming": "^5.3.19",
"@types/chroma-js": "^2.0.0",
Expand All @@ -125,7 +125,6 @@
"@types/luxon": "^1.11.1",
"@types/moment-timezone": "^0.5.12",
"@types/puppeteer": "^1.19.1",
"@types/react": "^16.9.41",
"@types/react-dom": "^16.9.8",
"@types/react-redux": "^7.1.1",
"@types/seedrandom": "^2.4.28",
Expand All @@ -145,7 +144,7 @@
"core-js": "^3.6.4",
"cross-env": "^7.0.2",
"css-loader": "^3.6.0",
"cz-conventional-changelog": "^3.0.2",
"cz-conventional-changelog": "^3.3.0",
"enzyme": "^3.9.0",
"enzyme-adapter-react-16": "^1.10.0",
"eslint": "^7.1.0",
Expand Down Expand Up @@ -189,6 +188,8 @@
"react-dom": "^16.13.0",
"react-is": "^16.13.0",
"redux-devtools-extension": "^2.13.8",
"redux-immutable-state-invariant": "^2.1.0",
"redux-logger": "^3.0.6",
"sass-graph": "^3.0.5",
"sass-loader": "^9.0.2",
"seedrandom": "^3.0.5",
Expand All @@ -210,10 +211,6 @@
"react": "^16.12.0",
"react-dom": "^16.12.0"
},
"optionalDependencies": {
"redux-immutable-state-invariant": "^2.1.0",
"redux-logger": "^3.0.6"
},
"config": {
"commitizen": {
"path": "./node_modules/cz-conventional-changelog"
Expand Down
3 changes: 2 additions & 1 deletion src/chart_types/goal_chart/state/selectors/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ export const getTooltipInfoSelector = createCachedSelector(
specId: spec.id,
key: spec.id,
},
value: `${shape.actual}`,
value: shape.actual,
formattedValue: `${shape.actual}`,
});
});

Expand Down
6 changes: 3 additions & 3 deletions src/chart_types/partition_chart/state/selectors/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export const getTooltipInfoSelector = createCachedSelector(
pickedShapes.forEach((shape) => {
const labelFormatter = labelFormatters[shape.depth - 1];
const formatter = labelFormatter?.nodeLabel;
const value = primaryValueGetterFun(shape);

tooltipInfo.values.push({
label: formatter ? formatter(shape.dataName) : shape.dataName,
Expand All @@ -62,9 +63,8 @@ export const getTooltipInfoSelector = createCachedSelector(
specId: pieSpec.id,
key: pieSpec.id,
},
value: `${valueFormatter(primaryValueGetterFun(shape))} (${pieSpec.percentFormatter(
percentValueGetter(shape),
)})`,
value,
formattedValue: `${valueFormatter(value)} (${pieSpec.percentFormatter(percentValueGetter(shape))})`,
valueAccessor: shape.depth,
});
});
Expand Down
3 changes: 1 addition & 2 deletions src/chart_types/xy_chart/legend/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ export function computeLegend(
): LegendItem[] {
const legendItems: LegendItem[] = [];
const sortedCollection = getSortedDataSeriesColorsValuesMap(seriesCollection);
const fallbackTickFormatter = specs.find(({ tickFormat }) => tickFormat)?.tickFormat ?? defaultTickFormatter;

sortedCollection.forEach((series, key) => {
const { banded, lastValue, seriesIdentifier } = series;
Expand All @@ -85,7 +84,7 @@ export function computeLegend(

// Use this to get axis spec w/ tick formatter
const { yAxis } = getAxesSpecForSpecId(axesSpecs, spec.groupId);
const formatter = yAxis?.tickFormat ?? fallbackTickFormatter;
const formatter = spec.tickFormat ?? yAxis?.tickFormat ?? defaultTickFormatter;
const { hideInLegend } = spec;

legendItems.push({
Expand Down
12 changes: 8 additions & 4 deletions src/chart_types/xy_chart/state/chart_state.interactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -767,8 +767,10 @@ function mouseOverTestSuite(scaleType: ScaleType) {
MockStore.addSpecs([spec, leftAxis, bottomAxis, currentSettingSpec], store);
store.dispatch(onPointerMove({ x: chartLeft + 0, y: chartTop + 89 }, 0));
const tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState());
expect(tooltipInfo.tooltip.header?.value).toBe('bottom 0');
expect(tooltipInfo.tooltip.values[0].value).toBe('left 10');
expect(tooltipInfo.tooltip.header?.value).toBe(0);
expect(tooltipInfo.tooltip.header?.formattedValue).toBe('bottom 0');
expect(tooltipInfo.tooltip.values[0].value).toBe(10);
expect(tooltipInfo.tooltip.values[0].formattedValue).toBe('left 10');
});

test('chart 90 deg rotated', () => {
Expand All @@ -780,8 +782,10 @@ function mouseOverTestSuite(scaleType: ScaleType) {

store.dispatch(onPointerMove({ x: chartLeft + 0, y: chartTop + 89 }, 0));
const tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState());
expect(tooltipInfo.tooltip.header?.value).toBe('left 1');
expect(tooltipInfo.tooltip.values[0].value).toBe('bottom 5');
expect(tooltipInfo.tooltip.header?.value).toBe(1);
expect(tooltipInfo.tooltip.header?.formattedValue).toBe('left 1');
expect(tooltipInfo.tooltip.values[0].value).toBe(5);
expect(tooltipInfo.tooltip.values[0].formattedValue).toBe('bottom 5');
});
});
describe('brush', () => {
Expand Down
6 changes: 6 additions & 0 deletions src/chart_types/xy_chart/state/chart_state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ describe.skip('Chart Store', () => {
const tooltipValue: TooltipValue = {
label: 'a',
value: 'a',
formattedValue: 'a',
color: 'a',
isHighlighted: false,
seriesIdentifier: {
Expand Down Expand Up @@ -873,6 +874,7 @@ describe.skip('Chart Store', () => {
const tooltipValue: TooltipValue = {
label: 'a',
value: 'a',
formattedValue: 'a',
color: 'a',
isHighlighted: false,
seriesIdentifier: {
Expand Down Expand Up @@ -1019,6 +1021,7 @@ describe.skip('Chart Store', () => {
const highlightedTooltipValue: TooltipValue = {
label: 'foo',
value: 1,
formattedValue: '1',
color: 'color',
isHighlighted: true,
seriesIdentifier: {
Expand All @@ -1031,6 +1034,7 @@ describe.skip('Chart Store', () => {
const unhighlightedTooltipValue: TooltipValue = {
label: 'foo',
value: 1,
formattedValue: '1',
color: 'color',
isHighlighted: false,
seriesIdentifier: {
Expand Down Expand Up @@ -1063,6 +1067,7 @@ describe.skip('Chart Store', () => {
const headerValue: TooltipValue = {
label: 'header',
value: 'foo',
formattedValue: 'foo',
color: 'a',
isHighlighted: false,
seriesIdentifier: {
Expand All @@ -1079,6 +1084,7 @@ describe.skip('Chart Store', () => {
const tooltipValue: TooltipValue = {
label: 'a',
value: 123,
formattedValue: '123',
color: 'a',
isHighlighted: false,
seriesIdentifier: {
Expand Down
54 changes: 36 additions & 18 deletions src/chart_types/xy_chart/state/chart_state.timescales.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,18 +91,24 @@ describe('Render chart', () => {
store.dispatch(onPointerMove({ x: 15, y: 10 }, 0)); // check first valid tooltip
let tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(`${day1}`);
expect(tooltip.values[0].value).toBe(`${10}`);
expect(tooltip.header?.value).toBe(day1);
expect(tooltip.header?.formattedValue).toBe(`${day1}`);
expect(tooltip.values[0].value).toBe(10);
expect(tooltip.values[0].formattedValue).toBe(`${10}`);
store.dispatch(onPointerMove({ x: 35, y: 10 }, 1)); // check second valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(`${day2}`);
expect(tooltip.values[0].value).toBe(`${22}`);
expect(tooltip.header?.value).toBe(day2);
expect(tooltip.header?.formattedValue).toBe(`${day2}`);
expect(tooltip.values[0].value).toBe(22);
expect(tooltip.values[0].formattedValue).toBe(`${22}`);
store.dispatch(onPointerMove({ x: 76, y: 10 }, 2)); // check third valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(`${day3}`);
expect(tooltip.values[0].value).toBe(`${6}`);
expect(tooltip.header?.value).toBe(day3);
expect(tooltip.header?.formattedValue).toBe(`${day3}`);
expect(tooltip.values[0].value).toBe(6);
expect(tooltip.values[0].formattedValue).toBe(`${6}`);
});
});
describe('line, utc-time, 5m interval', () => {
Expand Down Expand Up @@ -159,18 +165,24 @@ describe('Render chart', () => {
store.dispatch(onPointerMove({ x: 15, y: 10 }, 0)); // check first valid tooltip
let tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(`${date1}`);
expect(tooltip.values[0].value).toBe(`${10}`);
expect(tooltip.header?.value).toBe(date1);
expect(tooltip.header?.formattedValue).toBe(`${date1}`);
expect(tooltip.values[0].value).toBe(10);
expect(tooltip.values[0].formattedValue).toBe(`${10}`);
store.dispatch(onPointerMove({ x: 35, y: 10 }, 1)); // check second valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(`${date2}`);
expect(tooltip.values[0].value).toBe(`${22}`);
expect(tooltip.header?.value).toBe(date2);
expect(tooltip.header?.formattedValue).toBe(`${date2}`);
expect(tooltip.values[0].value).toBe(22);
expect(tooltip.values[0].formattedValue).toBe(`${22}`);
store.dispatch(onPointerMove({ x: 76, y: 10 }, 2)); // check third valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(`${date3}`);
expect(tooltip.values[0].value).toBe(`${6}`);
expect(tooltip.header?.value).toBe(date3);
expect(tooltip.header?.formattedValue).toBe(`${date3}`);
expect(tooltip.values[0].value).toBe(6);
expect(tooltip.values[0].formattedValue).toBe(`${6}`);
});
});
describe('line, non utc-time, 5m + 1s interval', () => {
Expand Down Expand Up @@ -245,18 +257,24 @@ describe('Render chart', () => {
store.dispatch(onPointerMove({ x: 15, y: 10 }, 0)); // check first valid tooltip
let tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(`${date1}`);
expect(tooltip.values[0].value).toBe(`${10}`);
expect(tooltip.header?.value).toBe(date1);
expect(tooltip.header?.formattedValue).toBe(`${date1}`);
expect(tooltip.values[0].value).toBe(10);
expect(tooltip.values[0].formattedValue).toBe(`${10}`);
store.dispatch(onPointerMove({ x: 35, y: 10 }, 1)); // check second valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(`${date2}`);
expect(tooltip.values[0].value).toBe(`${22}`);
expect(tooltip.header?.value).toBe(date2);
expect(tooltip.header?.formattedValue).toBe(`${date2}`);
expect(tooltip.values[0].value).toBe(22);
expect(tooltip.values[0].formattedValue).toBe(`${22}`);
store.dispatch(onPointerMove({ x: 76, y: 10 }, 2)); // check third valid tooltip
tooltip = getTooltipInfoSelector(store.getState());
expect(tooltip.values.length).toBe(1);
expect(tooltip.header?.value).toBe(`${date3}`);
expect(tooltip.values[0].value).toBe(`${6}`);
expect(tooltip.header?.value).toBe(date3);
expect(tooltip.header?.formattedValue).toBe(`${date3}`);
expect(tooltip.values[0].value).toBe(6);
expect(tooltip.values[0].formattedValue).toBe(`${6}`);
});
});
});
Loading