Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle null y0 values on y log scale rendering #413

Merged
merged 6 commits into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 8 additions & 46 deletions .playground/playgroud.tsx
Original file line number Diff line number Diff line change
@@ -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>
Expand Down
42 changes: 29 additions & 13 deletions src/chart_types/xy_chart/rendering/rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
markov00 marked this conversation as resolved.
Show resolved Hide resolved
return yScale.isInverted ? yScale.range[1] : yScale.range[0];
})
.defined(({ x, y1 }) => {
return y1 !== null && !(isLogScale && y1 <= 0) && xScale.isValueInDomain(x);
})
Expand Down Expand Up @@ -458,7 +468,7 @@ export function renderArea(
seriesKey: any[],
xScaleOffset: number,
seriesStyle: AreaSeriesStyle,
isStacked: boolean = false,
isStacked = false,
pointStyleAccessor?: PointStyleAccessor,
): {
areaGeometry: AreaGeometry;
Expand All @@ -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];
Expand Down
8 changes: 4 additions & 4 deletions src/components/react_canvas/reactive_chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/utils/scales/scales.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: (
Expand Down
14 changes: 3 additions & 11 deletions stories/bar_chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
Expand All @@ -1085,7 +1077,7 @@ storiesOf('Bar Chart', module)
allPositive: 'all positive',
allNegative: 'all negative',
},
'mixed',
'all negative',
);

let data = mixed;
Expand Down