Skip to content

Commit

Permalink
fix: remove series with undefined splitSeriesAccessor values (#627)
Browse files Browse the repository at this point in the history
This commit removes every series generated from a partial datum where all the values accessed by the splitSeriesAccessors array are null or undefined.

Co-authored-by: nickofthyme <[email protected]>
  • Loading branch information
markov00 and nickofthyme committed Apr 14, 2020
1 parent 5669178 commit 59f0f6e
Show file tree
Hide file tree
Showing 20 changed files with 162 additions and 30 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 10 additions & 1 deletion integration/tests/interactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import { common } from '../page_objects';

describe.only('Tooltips', () => {
describe('Tooltips', () => {
describe('rotation 0', () => {
it('shows tooltip on first x value - top', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
Expand Down Expand Up @@ -104,4 +104,13 @@ describe.only('Tooltips', () => {
);
});
});
it('should render corrent tooltip for split and y accessors', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/iframe.html?id=bar-chart--bar-chart-2-y-2-g',
{
x: 330,
y: 40,
},
);
});
});
23 changes: 17 additions & 6 deletions src/chart_types/xy_chart/state/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,8 @@ describe('Chart State utils', () => {

describe('series collection is not empty', () => {
it('it should return an empty map if no color', () => {
const barSpec1 = MockSeriesSpec.bar({ id: specId1, data });
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data });
const barSpec1 = MockSeriesSpec.bar({ id: specId1, data, splitSeriesAccessors: ['g'] });
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data, splitSeriesAccessors: ['g'] });
const barSeriesSpecs = MockSeriesSpecs.fromSpecs([barSpec1, barSpec2]);
const barSeriesCollection = MockSeriesCollection.fromSpecs(barSeriesSpecs);
const actual = getCustomSeriesColors(barSeriesSpecs, barSeriesCollection);
Expand All @@ -384,8 +384,13 @@ describe('Chart State utils', () => {

describe('with customSeriesColors array', () => {
const customSeriesColors = ['red', 'blue', 'green'];
const barSpec1 = MockSeriesSpec.bar({ id: specId1, data, color: customSeriesColors });
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data });
const barSpec1 = MockSeriesSpec.bar({
id: specId1,
data,
color: customSeriesColors,
splitSeriesAccessors: ['g'],
});
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data, splitSeriesAccessors: ['g'] });
const barSeriesSpecs = MockSeriesSpecs.fromSpecs([barSpec1, barSpec2]);
const barSeriesCollection = MockSeriesCollection.fromSpecs(barSeriesSpecs);

Expand All @@ -412,8 +417,14 @@ describe('Chart State utils', () => {

return null;
};
const barSpec1 = MockSeriesSpec.bar({ id: specId1, yAccessors: ['y'], data, color });
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data });
const barSpec1 = MockSeriesSpec.bar({
id: specId1,
yAccessors: ['y'],
data,
color,
splitSeriesAccessors: ['g'],
});
const barSpec2 = MockSeriesSpec.bar({ id: specId2, data, splitSeriesAccessors: ['g'] });
const barSeriesSpecs = MockSeriesSpecs.fromSpecs([barSpec1, barSpec2]);
const barSeriesCollection = MockSeriesCollection.fromSpecs(barSeriesSpecs);

Expand Down
93 changes: 93 additions & 0 deletions src/chart_types/xy_chart/utils/__snapshots__/series.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -19144,6 +19144,99 @@ Array [
]
`;

exports[`Series functional accessors Shall ignore undefined values on splitSeriesAccessors 1`] = `
Array [
Object {
"data": Array [
Object {
"datum": Array [
0,
1,
"a",
],
"x": 0,
"y0": null,
"y1": 1,
},
Object {
"datum": Array [
1,
1,
"a",
],
"x": 1,
"y0": null,
"y1": 1,
},
Object {
"datum": Array [
2,
1,
"a",
],
"x": 2,
"y0": null,
"y1": 1,
},
],
"key": "spec{spec1}yAccessor{1}splitAccessors{2-a}",
"seriesKeys": Array [
"a",
1,
],
"specId": "spec1",
"splitAccessors": Map {
2 => "a",
},
"yAccessor": 1,
},
Object {
"data": Array [
Object {
"datum": Array [
0,
1,
"b",
],
"x": 0,
"y0": null,
"y1": 1,
},
Object {
"datum": Array [
1,
1,
"b",
],
"x": 1,
"y0": null,
"y1": 1,
},
Object {
"datum": Array [
2,
1,
"b",
],
"x": 2,
"y0": null,
"y1": 1,
},
],
"key": "spec{spec1}yAccessor{1}splitAccessors{2-b}",
"seriesKeys": Array [
"b",
1,
],
"specId": "spec1",
"splitAccessors": Map {
2 => "b",
},
"yAccessor": 1,
},
]
`;

exports[`Series should compute data series for stacked specs 1`] = `
Array [
Object {
Expand Down
36 changes: 36 additions & 0 deletions src/chart_types/xy_chart/utils/series.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -947,5 +947,41 @@ describe('Series', () => {
expect(splittedSeries.rawDataSeries.length).toBe(1);
expect(splittedSeries.rawDataSeries).toMatchSnapshot();
});

test('Shall ignore undefined values on splitSeriesAccessors', () => {
const spec = MockSeriesSpec.bar({
data: [
[0, 1, 'a'],
[1, 1, 'a'],
[2, 1, 'a'],
[0, 1, 'b'],
[1, 1, 'b'],
[2, 1, 'b'],
[0, 1],
[1, 1],
[2, 1],
],
xAccessor: 0,
yAccessors: [1],
splitSeriesAccessors: [2],
});
const splittedSeries = splitSeries(spec);
expect(splittedSeries.rawDataSeries.length).toBe(2);
expect(splittedSeries.rawDataSeries).toMatchSnapshot();
});
test('Should ignore series if splitSeriesAccessors are defined but not contained in any datum', () => {
const spec = MockSeriesSpec.bar({
data: [
[0, 1],
[1, 1],
[2, 1],
],
xAccessor: 0,
yAccessors: [1],
splitSeriesAccessors: [2],
});
const splittedSeries = splitSeries(spec);
expect(splittedSeries.rawDataSeries.length).toBe(0);
});
});
});
5 changes: 5 additions & 0 deletions src/chart_types/xy_chart/utils/series.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ export function splitSeries({

data.forEach((datum) => {
const splitAccessors = getSplitAccessors(datum, splitSeriesAccessors);
// if splitSeriesAccessors are defined we should have at least one split value to include datum
if (splitSeriesAccessors.length > 0 && splitAccessors.size < 1) {
return;
}

if (isMultipleY) {
yAccessors.forEach((accessor, index) => {
const cleanedDatum = cleanDatum(datum, xAccessor, accessor, y0Accessors && y0Accessors[index]);
Expand Down
1 change: 0 additions & 1 deletion src/mocks/specs/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ export class MockSeriesSpec {
yScaleType: ScaleType.Linear,
xAccessor: 'x',
yAccessors: ['y'],
splitSeriesAccessors: ['g'],
yScaleToDataExtent: false,
hideInLegend: false,
enableHistogramMode: false,
Expand Down
1 change: 0 additions & 1 deletion stories/axes/5_multi_axis_bar_lines.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export const example = () => {
xAccessor="x"
yAccessors={['y']}
stackAccessors={['x']}
splitSeriesAccessors={['g']}
data={[
{ x: 0, y: 3 },
{ x: 1, y: 2 },
Expand Down
2 changes: 0 additions & 2 deletions stories/axes/6_different_tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export const example = () => {
xAccessor="x"
yAccessors={['y']}
stackAccessors={['x']}
splitSeriesAccessors={['g']}
data={[
{ x: 0, y: 5 },
{ x: 1, y: 4 },
Expand All @@ -64,7 +63,6 @@ export const example = () => {
xAccessor="x"
yAccessors={['y']}
stackAccessors={['x']}
splitSeriesAccessors={['g']}
data={[
{ x: 0, y: 2 },
{ x: 1, y: 3 },
Expand Down
1 change: 0 additions & 1 deletion stories/axes/9_custom_mixed_domain.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ export const example = () => {
xAccessor="x"
yAccessors={['y']}
stackAccessors={['x']}
splitSeriesAccessors={['g']}
data={[
{ x: 0, y: 3 },
{ x: 1, y: 2 },
Expand Down
9 changes: 1 addition & 8 deletions stories/bar/23_bar_chart_2y2g.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,11 @@

import React from 'react';

import { Axis, BarSeries, Chart, Position, ScaleType, Settings, FilterPredicate } from '../../src';
import { Axis, BarSeries, Chart, Position, ScaleType, Settings } from '../../src';
import * as TestDatasets from '../../src/utils/data_samples/test_dataset';
import { SB_SOURCE_PANEL } from '../utils/storybook';

export const example = () => {
const isVisibleFunction: FilterPredicate = (series) => {
return series.splitAccessors.size > 0
? series.specId === 'bars1' && series.yAccessor === 'y1' && series.splitAccessors.get('g1') === 'cloudflare.com'
: series.specId === 'bars1' && series.yAccessor === 'y1';
};

return (
<Chart className="story-chart">
<Settings showLegend showLegendExtra legendPosition={Position.Right} />
Expand All @@ -43,7 +37,6 @@ export const example = () => {
yAccessors={['y1', 'y2']}
splitSeriesAccessors={['g1', 'g2', 'g3']}
data={TestDatasets.BARCHART_2Y2G}
filterSeriesInTooltip={isVisibleFunction}
/>
</Chart>
);
Expand Down
1 change: 0 additions & 1 deletion stories/bar/26_single_data_linear.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export const example = () => {
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
splitSeriesAccessors={['g']}
data={[{ x: 1, y: 10 }]}
/>
</Chart>
Expand Down
1 change: 0 additions & 1 deletion stories/bar/31_negative_and_positive_x_values.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export const example = () => {
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
splitSeriesAccessors={['g']}
stackAccessors={['x']}
data={[
{ x: -3, y: 1 },
Expand Down
1 change: 0 additions & 1 deletion stories/bar/32_scale_to_extent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ export const example = () => {
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
splitSeriesAccessors={['g']}
stackAccessors={['x']}
data={data}
yScaleToDataExtent={yScaleToDataExtent}
Expand Down
2 changes: 0 additions & 2 deletions stories/bar/33_band_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ export const example = () => {
yAccessors={['max']}
y0Accessors={['min']}
data={data}
// this is a temporary hack to display names for min and max values
splitSeriesAccessors={['fake']}
yScaleToDataExtent={scaleToDataExtent}
/>

Expand Down
1 change: 0 additions & 1 deletion stories/grids/2_multiple_axes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ export const example = () => {
xAccessor="x"
yAccessors={['y']}
stackAccessors={['x']}
splitSeriesAccessors={['g']}
data={[
{ x: 0, y: 3 },
{ x: 1, y: 2 },
Expand Down
1 change: 0 additions & 1 deletion stories/interactions/4_line_area_bar_clicks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export const example = () => {
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
splitSeriesAccessors={['g']}
data={[
{ x: 0, y: 2.3 },
{ x: 1, y: 2 },
Expand Down
1 change: 0 additions & 1 deletion stories/interactions/8_clicks_legend_items_mixed.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export const example = () => {
xAccessor="x"
yAccessors={['y']}
stackAccessors={['x']}
splitSeriesAccessors={['g']}
data={[
{ x: 0, y: 3 },
{ x: 1, y: 2 },
Expand Down
1 change: 0 additions & 1 deletion stories/legend/6_hide_legend.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export const example = () => {
xAccessor="x"
yAccessors={['y']}
stackAccessors={['x']}
splitSeriesAccessors={['g']}
data={[
{ x: 0, y: 3 },
{ x: 1, y: 2 },
Expand Down
1 change: 0 additions & 1 deletion stories/mixed/3_areas_and_bars.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export const example = () => {
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
splitSeriesAccessors={['g']}
stackAccessors={['x']}
data={[
{ x: 0, y: 2 },
Expand Down

0 comments on commit 59f0f6e

Please sign in to comment.