diff --git a/packages/osd-charts/.playground/playgroud.tsx b/packages/osd-charts/.playground/playgroud.tsx index dc99fde61d10..f993a990b479 100644 --- a/packages/osd-charts/.playground/playgroud.tsx +++ b/packages/osd-charts/.playground/playgroud.tsx @@ -1,69 +1,31 @@ import React, { Fragment } from 'react'; -import { Axis, Chart, getAxisId, getSpecId, Position, ScaleType, Settings, AreaSeries } from '../src'; +import { Axis, Chart, getAxisId, getSpecId, Position, ScaleType, BarSeries } from '../src'; 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/packages/osd-charts/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/packages/osd-charts/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 8e8bb7a5c1d6..f7674b251e01 100644 Binary files a/packages/osd-charts/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/packages/osd-charts/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/packages/osd-charts/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/packages/osd-charts/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 554fda9282dc..19a22560a0f6 100644 Binary files a/packages/osd-charts/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/packages/osd-charts/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/packages/osd-charts/src/chart_types/xy_chart/rendering/rendering.bars.test.ts b/packages/osd-charts/src/chart_types/xy_chart/rendering/rendering.bars.test.ts index 3a152f5b1b0c..ebf886a9bb66 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/rendering/rendering.bars.test.ts +++ b/packages/osd-charts/src/chart_types/xy_chart/rendering/rendering.bars.test.ts @@ -490,6 +490,46 @@ describe('Rendering bars', () => { }); }); }); + describe('Single series bar chart - log', () => { + const barSeriesSpec: BarSeriesSpec = { + 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 = new Map(); + barSeriesMap.set(SPEC_ID, barSeriesSpec); + const barSeriesDomains = computeSeriesDomains(barSeriesMap, new Map()); + const xScale = computeXScale({ + xDomain: barSeriesDomains.xDomain, + totalBarsInCluster: barSeriesMap.size, + 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 = getSpecId('bar1'); const spec2Id = getSpecId('bar2'); diff --git a/packages/osd-charts/src/chart_types/xy_chart/rendering/rendering.ts b/packages/osd-charts/src/chart_types/xy_chart/rendering/rendering.ts index 362d62511f9b..6e349ffa9c16 100644 --- a/packages/osd-charts/src/chart_types/xy_chart/rendering/rendering.ts +++ b/packages/osd-charts/src/chart_types/xy_chart/rendering/rendering.ts @@ -213,9 +213,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 { @@ -246,6 +245,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); } @@ -288,7 +288,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 @@ -296,21 +296,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; @@ -405,7 +409,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); }) @@ -458,7 +468,7 @@ export function renderArea( seriesKey: any[], xScaleOffset: number, seriesStyle: AreaSeriesStyle, - isStacked: boolean = false, + isStacked = false, pointStyleAccessor?: PointStyleAccessor, ): { areaGeometry: AreaGeometry; @@ -468,7 +478,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/packages/osd-charts/src/components/react_canvas/reactive_chart.tsx b/packages/osd-charts/src/components/react_canvas/reactive_chart.tsx index edb57b1d7ae4..cac0c8260acb 100644 --- a/packages/osd-charts/src/components/react_canvas/reactive_chart.tsx +++ b/packages/osd-charts/src/components/react_canvas/reactive_chart.tsx @@ -356,10 +356,10 @@ class Chart extends React.Component { sortAndRenderElements() { const { chartRotation, chartDimensions } = this.props.chartStore!; 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/packages/osd-charts/src/utils/scales/scales.ts b/packages/osd-charts/src/utils/scales/scales.ts index bc4aafcf6a2c..0a7ccd3119f1 100644 --- a/packages/osd-charts/src/utils/scales/scales.ts +++ b/packages/osd-charts/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/packages/osd-charts/stories/bar_chart.tsx b/packages/osd-charts/stories/bar_chart.tsx index fa16c6f5a5e2..a96a2b48d6bb 100644 --- a/packages/osd-charts/stories/bar_chart.tsx +++ b/packages/osd-charts/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;