fix: handle null y0 values on y log scale rendering (opensearch-proje…

…ct#413)

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 (
       <Fragment>
         <div className="chart">
           <Chart>
-            <Settings showLegend theme={{ areaSeriesStyle: { point: { visible: true } } }} />
-            <Axis
-              id={getAxisId('bottom')}
-              position={Position.Bottom}
-              title={'Bottom axis'}
-              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)}
             />
-            <AreaSeries
-              id={getSpecId('bars1')}
+
+            <BarSeries
+              id={getSpecId('bars')}
               xScaleType={ScaleType.Linear}
               yScaleType={ScaleType.Linear}
               xAccessor="x"
               yAccessors={['y']}
-              stackAccessors={['x']}
               splitSeriesAccessors={['g']}
-              // curve={CurveType.CURVE_MONOTONE_X}
-              data={[
-                { x: 0, y: 2, g: 'a' },
-                { x: 1, y: 7, g: 'a' },
-                { x: 2, y: 3, g: 'a' },
-                { x: 3, y: 6, g: 'a' },
-                { x: 0, y: 4, g: 'b' },
-                { x: 1, y: 5, g: 'b' },
-                { x: 2, y: 8, g: 'b' },
-                { x: 3, y: 2, g: 'b' },
-                { x: 4, y: 6, g: 'b' },
-                { x: 5, y: 7, g: 'a' },
-                { x: 5, y: 7, g: 'b' },
-                { x: 6, y: 7, g: 'a' },
-                { x: 6, y: 7, g: 'b' },
-              ]}
-            />
-            <AreaSeries
-              id={getSpecId('area2')}
-              xScaleType={ScaleType.Linear}
-              yScaleType={ScaleType.Linear}
-              xAccessor="x"
-              yAccessors={['y']}
               stackAccessors={['x']}
-              splitSeriesAccessors={['g']}
-              // curve={CurveType.CURVE_MONOTONE_X}
-              data={[
-                { x: 1, y: 7, g: 'a' },
-                { x: 2, y: 3, g: 'a' },
-                { x: 3, y: 6, g: 'a' },
-                { x: 0, y: 4, g: 'b' },
-                { x: 1, y: 5, g: 'b' },
-                { x: 2, y: 8, g: 'b' },
-                { x: 3, y: 2, g: 'b' },
-                { x: 4, y: 6, g: 'b' },
-              ]}
+              data={data}
+              yScaleToDataExtent={true}
             />
           </Chart>
         </div>

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<SpecId, BarSeriesSpec>();
+    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');

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,29 +288,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;
@@ -405,7 +409,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);
     })
@@ -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<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];

packages/osd-charts/src/components/react_canvas/reactive_chart.tsx

@@ -356,10 +356,10 @@ class Chart extends React.Component<ReactiveChartProps, ReactiveChartState> {
   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);

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: (

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;