Skip to content

Commit

Permalink
fix(area_charts): correctly represent baseline with negative data poi…
Browse files Browse the repository at this point in the history
…nts (opensearch-project#896)

This commit fixes the rendering issue when using negative values in area charts. The correct approach
is to draw the area with a zero baseline (or dummy 1 baseline for log y scale).
  • Loading branch information
markov00 authored Nov 11, 2020
1 parent 3b88e1c commit b622fda
Show file tree
Hide file tree
Showing 34 changed files with 1,400 additions and 948 deletions.
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.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
28 changes: 28 additions & 0 deletions packages/osd-charts/integration/tests/area_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,32 @@ describe('Area series stories', () => {
);
});
});
describe('Negative log Areas', () => {
it('snows negative values with log scale', async () => {
await common.expectChartAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/area-chart--with-negative-values&knob-Y scale=log',
);
});
it('snows only positive domain mixed polarity domain', async () => {
await common.expectChartAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/area-chart--with-negative-and-positive&knob-Y scale=log',
);
});

it('snows only positive values when hiding negative one', async () => {
const action = async () => await page.click('.echLegendItem:nth-child(2) .echLegendItem__label');
await common.expectChartAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/area-chart--with-negative-and-positive&knob-Y scale=log',
{ action },
);
});

it('snows only negative values when hiding positive one', async () => {
const action = async () => await page.click('.echLegendItem:nth-child(1) .echLegendItem__label');
await common.expectChartAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/area-chart--with-negative-and-positive&knob-Y scale=log',
{ action },
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { Rotation } from '../../../../utils/commons';
import { Dimensions } from '../../../../utils/dimensions';
import { AreaGeometry, PerPanel } from '../../../../utils/geometry';
import { SharedGeometryStateStyle } from '../../../../utils/themes/theme';
import { getGeometryStateStyle } from '../../rendering/rendering';
import { getGeometryStateStyle } from '../../rendering/utils';
import { renderPoints } from './points';
import { renderLinePaths, renderAreaPath } from './primitives/path';
import { buildAreaStyles } from './styles/area';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { Rotation } from '../../../../utils/commons';
import { Dimensions } from '../../../../utils/dimensions';
import { BarGeometry, PerPanel } from '../../../../utils/geometry';
import { SharedGeometryStateStyle } from '../../../../utils/themes/theme';
import { getGeometryStateStyle } from '../../rendering/rendering';
import { getGeometryStateStyle } from '../../rendering/utils';
import { renderRect } from './primitives/rect';
import { buildBarStyles } from './styles/bar';
import { withPanelTransform } from './utils/panel_transform';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { Rotation } from '../../../../utils/commons';
import { Dimensions } from '../../../../utils/dimensions';
import { BubbleGeometry, PerPanel, PointGeometry } from '../../../../utils/geometry';
import { SharedGeometryStateStyle, GeometryStateStyle, PointStyle } from '../../../../utils/themes/theme';
import { getGeometryStateStyle } from '../../rendering/rendering';
import { getGeometryStateStyle } from '../../rendering/utils';
import { renderPointGroup } from './points';

interface BubbleGeometriesDataProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { Rotation } from '../../../../utils/commons';
import { Dimensions } from '../../../../utils/dimensions';
import { LineGeometry, PerPanel } from '../../../../utils/geometry';
import { SharedGeometryStateStyle } from '../../../../utils/themes/theme';
import { getGeometryStateStyle } from '../../rendering/rendering';
import { getGeometryStateStyle } from '../../rendering/utils';
import { renderPoints } from './points';
import { renderLinePaths } from './primitives/path';
import { buildLineStyles } from './styles/line';
Expand Down
152 changes: 152 additions & 0 deletions packages/osd-charts/src/chart_types/xy_chart/rendering/area.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { area } from 'd3-shape';

import { Scale } from '../../../scales';
import { Color } from '../../../utils/commons';
import { CurveType, getCurveFactory } from '../../../utils/curves';
import { Dimensions } from '../../../utils/dimensions';
import { AreaGeometry } from '../../../utils/geometry';
import { AreaSeriesStyle } from '../../../utils/themes/theme';
import { IndexedGeometryMap } from '../utils/indexed_geometry_map';
import { DataSeries, DataSeriesDatum } from '../utils/series';
import { PointStyleAccessor } from '../utils/specs';
import { renderPoints } from './points';
import {
getClippedRanges,
getY0ScaledValueOrThrow,
getY1ScaledValueOrThrow,
isYValueDefined,
MarkSizeOptions,
} from './utils';

/** @internal */
export function renderArea(
shift: number,
dataSeries: DataSeries,
xScale: Scale,
yScale: Scale,
panel: Dimensions,
color: Color,
curve: CurveType,
hasY0Accessors: boolean,
xScaleOffset: number,
seriesStyle: AreaSeriesStyle,
markSizeOptions: MarkSizeOptions,
isStacked = false,
pointStyleAccessor?: PointStyleAccessor,
hasFit?: boolean,
): {
areaGeometry: AreaGeometry;
indexedGeometryMap: IndexedGeometryMap;
} {
const y1Fn = getY1ScaledValueOrThrow(yScale);
const y0Fn = getY0ScaledValueOrThrow(yScale);
const definedFn = isYValueDefined(yScale, xScale);
const pathGenerator = area<DataSeriesDatum>()
.x(({ x }) => xScale.scaleOrThrow(x) - xScaleOffset)
.y1(y1Fn)
.y0(y0Fn)
.defined((datum) => {
return definedFn(datum) && (hasY0Accessors ? definedFn(datum, 'y0') : true);
})
.curve(getCurveFactory(curve));

const clippedRanges = getClippedRanges(dataSeries.data, xScale, xScaleOffset);

let y1Line: string | null;

try {
y1Line = pathGenerator.lineY1()(dataSeries.data);
} catch {
// When values are not scalable
y1Line = null;
}

const lines: string[] = [];
if (y1Line) {
lines.push(y1Line);
}
if (hasY0Accessors) {
let y0Line: string | null;

try {
y0Line = pathGenerator.lineY0()(dataSeries.data);
} catch {
// When values are not scalable
y0Line = null;
}
if (y0Line) {
lines.push(y0Line);
}
}

const { pointGeometries, indexedGeometryMap } = renderPoints(
shift - xScaleOffset,
dataSeries,
xScale,
yScale,
panel,
color,
seriesStyle.line,
hasY0Accessors,
markSizeOptions,
pointStyleAccessor,
false,
);

let areaPath: string;

try {
areaPath = pathGenerator(dataSeries.data) || '';
} catch {
// When values are not scalable
areaPath = '';
}

const areaGeometry: AreaGeometry = {
area: areaPath,
lines,
points: pointGeometries,
color,
transform: {
y: 0,
x: shift,
},
seriesIdentifier: {
key: dataSeries.key,
specId: dataSeries.specId,
yAccessor: dataSeries.yAccessor,
splitAccessors: dataSeries.splitAccessors,
seriesKeys: dataSeries.seriesKeys,
smHorizontalAccessorValue: dataSeries.smHorizontalAccessorValue,
smVerticalAccessorValue: dataSeries.smVerticalAccessorValue,
},
seriesAreaStyle: seriesStyle.area,
seriesAreaLineStyle: seriesStyle.line,
seriesPointStyle: seriesStyle.point,
isStacked,
clippedRanges,
hideClippedRanges: !hasFit,
};
return {
areaGeometry,
indexedGeometryMap,
};
}
Loading

0 comments on commit b622fda

Please sign in to comment.