Merge branch 'master' into nct_api

elastic · Oct 15, 2019 · aaa1416 · aaa1416
2 parents e927275 + a1725e4
commit aaa1416
Show file tree

Hide file tree

Showing 10 changed files with 94 additions and 82 deletions.
diff --git 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 (
       <Fragment>
         <div className="chart">
           <button onClick={this.switchLegend}>Switch legend</button>
           <Chart>
-            <Settings debug showLegend legendPosition={this.state.legendPosition} />
-            <Axis
-              id={getAxisId('bottom')}
-              position={Position.Bottom}
-              title={this.state.legendPosition}
-              showOverlappingTicks={true}
-            />
+            <Axis id={getAxisId('top')} position={Position.Bottom} title={'Top axis'} />
             <Axis
               id={getAxisId('left2')}
               title={'Left axis'}
               position={Position.Left}
               tickFormat={(d: any) => Number(d).toFixed(2)}
             />
-            {/* <BarSeries
-          id={getSpecId('bars')}
-          xScaleType={ScaleType.Linear}
-          yScaleType={ScaleType.Linear}
-          xAccessor="x"
-          yAccessors={['y']}
-          splitSeriesAccessors={['g']}
-          stackAccessors={['x']}
-          data={data}
-          yScaleToDataExtent={false}
-        /> */}
+
             <BarSeries
-              id={getSpecId('bars2')}
+              id={getSpecId('bars')}
               xScaleType={ScaleType.Linear}
               yScaleType={ScaleType.Linear}
               xAccessor="x"
               yAccessors={['y']}
-              stackAccessors={['x']}
               splitSeriesAccessors={['g']}
-              data={this.state.data}
+              stackAccessors={['x']}
+              data={data}
+              yScaleToDataExtent={true}
             />
           </Chart>
         </div>

diff --git 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/...sts-for-all-stories-bar-chart-scale-to-extent-visually-looks-correct-1-snap.png b/...sts-for-all-stories-bar-chart-scale-to-extent-visually-looks-correct-1-snap.png
diff --git a/...all-stories-bar-chart-with-stacked-log-y-axis-visually-looks-correct-1-snap.png b/...all-stories-bar-chart-with-stacked-log-y-axis-visually-looks-correct-1-snap.png
diff --git 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 <[email protected]>",
   "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
@@ -148,36 +148,6 @@ class Chart extends React.Component<Props> {
     ];
   };
 
-  // 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<AxisProps>): 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 }) => (
-  //     <Axis {...axisProps} key={key} chartTheme={chartTheme} debug={debug} chartDimensions={chartDimensions} />
-  //   ));
-  // };
-
   renderAnnotations = (): ReactiveChartElementIndex[] => {
     const { annotationDimensions, annotationSpecs } = this.props;
     const annotationElements: ReactiveChartElementIndex[] = [];
@@ -223,10 +193,10 @@ class Chart extends React.Component<Props> {
   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
@@ -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
@@ -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,29 +199,33 @@ 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
     if (!xScale.isValueInDomain(datum.x)) {
       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<DataSeriesDatum>()
     .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<DataSeriesDatum>()
     .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
@@ -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
@@ -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;