diff --git a/.playground/playgroud.tsx b/.playground/playgroud.tsx index 01f029a243..fddbc56f1c 100644 --- a/.playground/playgroud.tsx +++ b/.playground/playgroud.tsx @@ -1,5 +1,5 @@ import React, { Fragment } from 'react'; -import { Axis, Chart, getAxisId, getSpecId, Position, ScaleType, Settings, DataGenerator, BarSeries } from '../src'; +import { Axis, Chart, getAxisId, getSpecId, Position, ScaleType, BarSeries, DataGenerator } from '../src'; const dg = new DataGenerator(); export class Playground extends React.Component { @@ -16,44 +16,30 @@ export class Playground extends React.Component { }); }; render() { + const data = [{ x: 0, y: -4 }, { x: 1, y: -3 }, { x: 2, y: 2 }, { x: 3, y: 1 }]; return (
- - + Number(d).toFixed(2)} /> - {/* */} +
diff --git a/CHANGELOG.md b/CHANGELOG.md index eb68b69e2a..a487ee9674 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## [13.5.2](https://github.com/elastic/elastic-charts/compare/v13.5.1...v13.5.2) (2019-10-10) + + +### Bug Fixes + +* handle null y0 values on y log scale rendering ([#413](https://github.com/elastic/elastic-charts/issues/413)) ([5731c10](https://github.com/elastic/elastic-charts/commit/5731c10)) + ## [13.5.1](https://github.com/elastic/elastic-charts/compare/v13.5.0...v13.5.1) (2019-10-09) diff --git a/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-scale-to-extent-visually-looks-correct-1-snap.png b/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-scale-to-extent-visually-looks-correct-1-snap.png index 8e8bb7a5c1..f7674b251e 100644 Binary files a/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-scale-to-extent-visually-looks-correct-1-snap.png and b/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-scale-to-extent-visually-looks-correct-1-snap.png differ diff --git a/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-with-stacked-log-y-axis-visually-looks-correct-1-snap.png b/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-with-stacked-log-y-axis-visually-looks-correct-1-snap.png index 554fda9282..19a22560a0 100644 Binary files a/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-with-stacked-log-y-axis-visually-looks-correct-1-snap.png and b/integration/tests/__image_snapshots__/all-test-ts-baseline-visual-tests-for-all-stories-bar-chart-with-stacked-log-y-axis-visually-looks-correct-1-snap.png differ diff --git a/package.json b/package.json index a9115bc679..1ac6483492 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@elastic/charts", "description": "Elastic-Charts data visualization library", - "version": "13.5.1", + "version": "13.5.2", "author": "Marco Vettorello ", "license": "Apache-2.0", "main": "dist/index.js", diff --git a/src/chart_types/xy_chart/renderer/canvas/reactive_chart.tsx b/src/chart_types/xy_chart/renderer/canvas/reactive_chart.tsx index be1dded513..f18ae256b1 100644 --- a/src/chart_types/xy_chart/renderer/canvas/reactive_chart.tsx +++ b/src/chart_types/xy_chart/renderer/canvas/reactive_chart.tsx @@ -148,36 +148,6 @@ class Chart extends React.Component { ]; }; - // getAxes = (): AxisProps[] => { - // const { axesVisibleTicks, axesSpecs, axesTicksDimensions, axesPositions } = this.props.chartStore!; - // const ids = [...axesVisibleTicks.keys()]; - - // return ids - // .map((id) => ({ - // key: `axis-${id}`, - // ticks: axesVisibleTicks.get(id), - // axisSpec: axesSpecs.get(id), - // axisTicksDimensions: axesTicksDimensions.get(id), - // axisPosition: axesPositions.get(id), - // })) - // .filter( - // (config: Partial): config is AxisProps => { - // const { ticks, axisSpec, axisTicksDimensions, axisPosition } = config; - - // return Boolean(ticks && axisSpec && axisTicksDimensions && axisPosition); - // }, - // ); - // }; - - // renderAxes = (): JSX.Element[] => { - // const { chartTheme, debug, chartDimensions } = this.props.chartStore!; - // const axes = this.getAxes(); - - // return axes.map(({ key, ...axisProps }) => ( - // - // )); - // }; - renderAnnotations = (): ReactiveChartElementIndex[] => { const { annotationDimensions, annotationSpecs } = this.props; const annotationElements: ReactiveChartElementIndex[] = []; @@ -223,10 +193,10 @@ class Chart extends React.Component { sortAndRenderElements() { const { chartDimensions, chartRotation } = this.props; const clippings = { - clipX: -1, - clipY: -1, - clipWidth: ([90, -90].includes(chartRotation) ? chartDimensions.height : chartDimensions.width) + 1, - clipHeight: ([90, -90].includes(chartRotation) ? chartDimensions.width : chartDimensions.height) + 1, + clipX: 0, + clipY: 0, + clipWidth: [90, -90].includes(chartRotation) ? chartDimensions.height : chartDimensions.width, + clipHeight: [90, -90].includes(chartRotation) ? chartDimensions.width : chartDimensions.height, }; const bars = this.renderBarSeries(clippings); diff --git a/src/chart_types/xy_chart/rendering/rendering.bars.test.ts b/src/chart_types/xy_chart/rendering/rendering.bars.test.ts index 1cac9895d5..114e8b81ec 100644 --- a/src/chart_types/xy_chart/rendering/rendering.bars.test.ts +++ b/src/chart_types/xy_chart/rendering/rendering.bars.test.ts @@ -494,6 +494,47 @@ describe('Rendering bars', () => { }); }); }); + describe('Single series bar chart - log', () => { + const barSeriesSpec: BarSeriesSpec = { + chartType: 'xy_axis', + specType: 'series', + id: SPEC_ID, + groupId: GROUP_ID, + seriesType: 'bar', + yScaleToDataExtent: false, + data: [[1, 0], [2, 1], [3, 2], [4, 3], [5, 4], [6, 5]], + xAccessor: 0, + yAccessors: [1], + xScaleType: ScaleType.Linear, + yScaleType: ScaleType.Log, + }; + const barSeriesMap = [barSeriesSpec]; + const barSeriesDomains = computeSeriesDomains(barSeriesMap, new Map()); + const xScale = computeXScale({ + xDomain: barSeriesDomains.xDomain, + totalBarsInCluster: barSeriesMap.length, + range: [0, 100], + }); + const yScales = computeYScales({ yDomains: barSeriesDomains.yDomain, range: [100, 0] }); + + test('Can render correct bar height', () => { + const { barGeometries } = renderBars( + 0, + barSeriesDomains.formattedDataSeries.nonStacked[0].dataSeries[0].data, + xScale, + yScales.get(GROUP_ID)!, + 'red', + SPEC_ID, + [], + LIGHT_THEME.barSeriesStyle, + ); + expect(barGeometries.length).toBe(6); + expect(barGeometries[0].height).toBe(0); + expect(barGeometries[1].height).toBe(0); + expect(barGeometries[2].height).toBeGreaterThan(0); + expect(barGeometries[3].height).toBeGreaterThan(0); + }); + }); describe('Multi series bar chart - linear', () => { const spec1Id = 'bar1'; const spec2Id = 'bar2'; diff --git a/src/chart_types/xy_chart/rendering/rendering.ts b/src/chart_types/xy_chart/rendering/rendering.ts index 9b26b0d064..ff7713fdf1 100644 --- a/src/chart_types/xy_chart/rendering/rendering.ts +++ b/src/chart_types/xy_chart/rendering/rendering.ts @@ -124,9 +124,8 @@ export function renderPoints( } let y; let radius = 10; - const isHidden = yDatum === null || (isLogScale && yDatum <= 0); // we fix 0 and negative values at y = 0 - if (isHidden) { + if (yDatum === null || (isLogScale && yDatum <= 0)) { y = yScale.range[0]; radius = 0; } else { @@ -157,6 +156,7 @@ export function renderPoints( }; mutableIndexedGeometryMapUpsert(indexedGeometries, xValue, pointGeometry); // use the geometry only if the yDatum in contained in the current yScale domain + const isHidden = yDatum === null || (isLogScale && yDatum <= 0); if (!isHidden && yScale.isValueInDomain(yDatum)) { points.push(pointGeometry); } @@ -199,7 +199,7 @@ export function renderBars( dataset.forEach((datum) => { const { y0, y1, initialY1, filled } = datum; // don't create a bar if the initialY1 value is null. - if (initialY1 === null || (filled && filled.y1 !== undefined)) { + if (y1 === null || initialY1 === null || (filled && filled.y1 !== undefined)) { return; } // don't create a bar if not within the xScale domain @@ -207,21 +207,25 @@ export function renderBars( return; } - let height = 0; let y = 0; + let y0Scaled; if (yScale.type === ScaleType.Log) { - y = y1 === 0 ? yScale.range[0] : yScale.scale(y1); - let y0Scaled; + y = y1 === 0 || y1 === null ? yScale.range[0] : yScale.scale(y1); if (yScale.isInverted) { - y0Scaled = y0 === 0 ? yScale.range[1] : yScale.scale(y0); + y0Scaled = y0 === 0 || y0 === null ? yScale.range[1] : yScale.scale(y0); } else { - y0Scaled = y0 === 0 ? yScale.range[0] : yScale.scale(y0); + y0Scaled = y0 === 0 || y0 === null ? yScale.range[0] : yScale.scale(y0); } - height = y0Scaled - y; } else { y = yScale.scale(y1); - height = yScale.scale(y0) - y; + if (yScale.isInverted) { + // use always zero as baseline if y0 is null + y0Scaled = y0 === null ? yScale.scale(0) : yScale.scale(y0); + } else { + y0Scaled = y0 === null ? yScale.scale(0) : yScale.scale(y0); + } } + const height = y0Scaled - y; const x = xScale.scale(datum.x) + xScale.bandwidth * orderIndex; const width = xScale.bandwidth; @@ -316,7 +320,13 @@ export function renderLine( const pathGenerator = line() .x(({ x }) => xScale.scale(x) - xScaleOffset) - .y(({ y1 }) => yScale.scale(y1)) + .y(({ y1 }) => { + if (y1 !== null) { + return yScale.scale(y1); + } + // this should never happen thanks to the defined function + return yScale.isInverted ? yScale.range[1] : yScale.range[0]; + }) .defined(({ x, y1 }) => { return y1 !== null && !(isLogScale && y1 <= 0) && xScale.isValueInDomain(x); }) @@ -369,7 +379,7 @@ export function renderArea( seriesKey: any[], xScaleOffset: number, seriesStyle: AreaSeriesStyle, - isStacked: boolean = false, + isStacked = false, pointStyleAccessor?: PointStyleAccessor, ): { areaGeometry: AreaGeometry; @@ -379,7 +389,13 @@ export function renderArea( const pathGenerator = area() .x(({ x }) => xScale.scale(x) - xScaleOffset) - .y1(({ y1 }) => yScale.scale(y1)) + .y1(({ y1 }) => { + if (y1 !== null) { + return yScale.scale(y1); + } + // this should never happen thanks to the defined function + return yScale.isInverted ? yScale.range[1] : yScale.range[0]; + }) .y0(({ y0 }) => { if (y0 === null || (isLogScale && y0 <= 0)) { return yScale.range[0]; diff --git a/src/utils/scales/scales.ts b/src/utils/scales/scales.ts index bc4aafcf6a..0a7ccd3119 100644 --- a/src/utils/scales/scales.ts +++ b/src/utils/scales/scales.ts @@ -2,7 +2,7 @@ export interface Scale { domain: any[]; range: number[]; ticks: () => any[]; - scale: (value: any) => number; + scale: (value: string | number) => number; pureScale: (value: any) => number; invert: (value: number) => any; invertWithStep: ( diff --git a/stories/bar_chart.tsx b/stories/bar_chart.tsx index d953932ede..b9db92d4a0 100644 --- a/stories/bar_chart.tsx +++ b/stories/bar_chart.tsx @@ -1064,16 +1064,8 @@ storiesOf('Bar Chart', module) ); }) .add('scale to extent', () => { - const yScaleToDataExtent = boolean('yScaleDataToExtent', false); - const mixed = [ - { x: 3, y: 1 }, - { x: 0, y: -4 }, - { x: 2, y: 2 }, - { x: 1, y: -3 }, - { x: 2, y: 2 }, - { x: 1, y: -3 }, - { x: 3, y: 1 }, - ]; + const yScaleToDataExtent = boolean('yScaleDataToExtent', true); + const mixed = [{ x: 0, y: -4 }, { x: 1, y: -3 }, { x: 2, y: 2 }, { x: 3, y: 1 }]; const allPositive = mixed.map((datum) => ({ x: datum.x, y: Math.abs(datum.y) })); const allNegative = mixed.map((datum) => ({ x: datum.x, y: Math.abs(datum.y) * -1 })); @@ -1085,7 +1077,7 @@ storiesOf('Bar Chart', module) allPositive: 'all positive', allNegative: 'all negative', }, - 'mixed', + 'all negative', ); let data = mixed;