Skip to content

Commit 9d7b159

Browse files
markov00markov00
authored
fix(rendering): fix rendering values <= 0 on log scale (#114)
Instead of throwing errors or warnings, if the user is using a log scale with a dataset with 0 values, we will hide them on the rendering of line and area charts. This fix also the defined area of the chart fix #112, fix #63
1 parent 3a42db4 commit 9d7b159

File tree

8 files changed

+2015
-42
lines changed

8 files changed

+2015
-42
lines changed

‎src/lib/series/rendering.areas.test.ts‎

Lines changed: 764 additions & 0 deletions
Large diffs are not rendered by default.

‎src/lib/series/rendering.bars.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const SPEC_ID = getSpecId('spec_1');
88
const GROUP_ID = getGroupId('group_1');
99

1010
describe('Rendering bars', () => {
11-
describe('Single series barchart - ordinal', () => {
11+
describe('Single series bar chart - ordinal', () => {
1212
const barSeriesSpec: BarSeriesSpec = {
1313
id: SPEC_ID,
1414
groupId: GROUP_ID,
@@ -70,7 +70,7 @@ describe('Rendering bars', () => {
7070
});
7171
});
7272
});
73-
describe('Multi series barchart - ordinal', () => {
73+
describe('Multi series bar chart - ordinal', () => {
7474
const spec1Id = getSpecId('bar1');
7575
const spec2Id = getSpecId('bar2');
7676
const barSeriesSpec1: BarSeriesSpec = {
@@ -191,7 +191,7 @@ describe('Rendering bars', () => {
191191
});
192192
});
193193
});
194-
describe('Single series barchart - lineaar', () => {
194+
describe('Single series bar chart - lineaar', () => {
195195
const barSeriesSpec: BarSeriesSpec = {
196196
id: SPEC_ID,
197197
groupId: GROUP_ID,
@@ -253,7 +253,7 @@ describe('Rendering bars', () => {
253253
});
254254
});
255255
});
256-
describe('Multi series barchart - linear', () => {
256+
describe('Multi series bar chart - linear', () => {
257257
const spec1Id = getSpecId('bar1');
258258
const spec2Id = getSpecId('bar2');
259259
const barSeriesSpec1: BarSeriesSpec = {
@@ -374,7 +374,7 @@ describe('Rendering bars', () => {
374374
});
375375
});
376376
});
377-
describe('Multi series barchart - time', () => {
377+
describe('Multi series bar chart - time', () => {
378378
const spec1Id = getSpecId('bar1');
379379
const spec2Id = getSpecId('bar2');
380380
const barSeriesSpec1: BarSeriesSpec = {

src/lib/series/rendering.points.test.ts renamed to src/lib/series/rendering.lines.test.ts

Lines changed: 77 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const SPEC_ID = getSpecId('spec_1');
99
const GROUP_ID = getGroupId('group_1');
1010

1111
describe('Rendering points - line', () => {
12-
describe('Single series point chart - ordinal', () => {
12+
describe('Single series line chart - ordinal', () => {
1313
const pointSeriesSpec: LineSeriesSpec = {
1414
id: SPEC_ID,
1515
groupId: GROUP_ID,
@@ -88,7 +88,7 @@ describe('Rendering points - line', () => {
8888
expect(indexedGeometries.size).toEqual(points.length);
8989
});
9090
});
91-
describe('Multi series pointchart - ordinal', () => {
91+
describe('Multi series line chart - ordinal', () => {
9292
const spec1Id = getSpecId('point1');
9393
const spec2Id = getSpecId('point2');
9494
const pointSeriesSpec1: LineSeriesSpec = {
@@ -238,7 +238,7 @@ describe('Rendering points - line', () => {
238238
expect(indexedGeometries.size).toEqual(points.length);
239239
});
240240
});
241-
describe('Single series pointchart - linear', () => {
241+
describe('Single series line chart - linear', () => {
242242
const pointSeriesSpec: LineSeriesSpec = {
243243
id: SPEC_ID,
244244
groupId: GROUP_ID,
@@ -316,7 +316,7 @@ describe('Rendering points - line', () => {
316316
expect(indexedGeometries.size).toEqual(points.length);
317317
});
318318
});
319-
describe('Multi series pointchart - linear', () => {
319+
describe('Multi series line chart - linear', () => {
320320
const spec1Id = getSpecId('point1');
321321
const spec2Id = getSpecId('point2');
322322
const pointSeriesSpec1: LineSeriesSpec = {
@@ -465,7 +465,7 @@ describe('Rendering points - line', () => {
465465
expect(indexedGeometries.size).toEqual(points.length);
466466
});
467467
});
468-
describe('Single series pointchart - time', () => {
468+
describe('Single series line chart - time', () => {
469469
const pointSeriesSpec: LineSeriesSpec = {
470470
id: SPEC_ID,
471471
groupId: GROUP_ID,
@@ -543,7 +543,7 @@ describe('Rendering points - line', () => {
543543
expect(indexedGeometries.size).toEqual(points.length);
544544
});
545545
});
546-
describe('Multi series pointchart - time', () => {
546+
describe('Multi series line chart - time', () => {
547547
const spec1Id = getSpecId('point1');
548548
const spec2Id = getSpecId('point2');
549549
const pointSeriesSpec1: LineSeriesSpec = {
@@ -679,4 +679,75 @@ describe('Rendering points - line', () => {
679679
expect(indexedGeometries.size).toEqual(points.length);
680680
});
681681
});
682+
describe('Single series line chart - y log', () => {
683+
const pointSeriesSpec: LineSeriesSpec = {
684+
id: SPEC_ID,
685+
groupId: GROUP_ID,
686+
seriesType: 'line',
687+
yScaleToDataExtent: false,
688+
data: [[0, 10], [1, 5], [2, null], [3, 5], [4, 5], [5, 0], [6, 10], [7, 10], [8, 10]],
689+
xAccessor: 0,
690+
yAccessors: [1],
691+
xScaleType: ScaleType.Linear,
692+
yScaleType: ScaleType.Log,
693+
};
694+
const pointSeriesMap = new Map();
695+
pointSeriesMap.set(SPEC_ID, pointSeriesSpec);
696+
const pointSeriesDomains = computeSeriesDomains(pointSeriesMap, new Map());
697+
const xScale = computeXScale(pointSeriesDomains.xDomain, pointSeriesMap.size, 0, 90);
698+
const yScales = computeYScales(pointSeriesDomains.yDomain, 100, 0);
699+
700+
let renderedLine: {
701+
lineGeometry: LineGeometry;
702+
indexedGeometries: Map<any, IndexedGeometry[]>;
703+
};
704+
705+
beforeEach(() => {
706+
renderedLine = renderLine(
707+
0, // not applied any shift, renderGeometries applies it only with mixed charts
708+
pointSeriesDomains.formattedDataSeries.nonStacked[0].dataSeries[0].data,
709+
xScale,
710+
yScales.get(GROUP_ID)!,
711+
'red',
712+
CurveType.LINEAR,
713+
SPEC_ID,
714+
[],
715+
);
716+
});
717+
test('Can render a splitted line', () => {
718+
// expect(renderedLine.lineGeometry.line).toBe('ss');
719+
expect(renderedLine.lineGeometry.line.split('M').length - 1).toBe(3);
720+
expect(renderedLine.lineGeometry.color).toBe('red');
721+
expect(renderedLine.lineGeometry.geometryId.seriesKey).toEqual([]);
722+
expect(renderedLine.lineGeometry.geometryId.specId).toEqual(SPEC_ID);
723+
expect(renderedLine.lineGeometry.transform).toEqual({ x: 0, y: 0 });
724+
});
725+
test('Can render points', () => {
726+
const {
727+
lineGeometry: { points },
728+
indexedGeometries,
729+
} = renderedLine;
730+
// all the points minus the undefined ones on a log scale
731+
expect(points.length).toBe(7);
732+
// all the points
733+
expect(indexedGeometries.size).toEqual(9);
734+
const nullIndexdGeometry = indexedGeometries.get(2);
735+
expect(nullIndexdGeometry).toBeDefined();
736+
expect(nullIndexdGeometry!.length).toBe(1);
737+
// moved to the bottom of the chart
738+
expect(nullIndexdGeometry![0].geom.y).toBe(100);
739+
// 0 radius point
740+
expect(nullIndexdGeometry![0].geom.width).toBe(0);
741+
expect(nullIndexdGeometry![0].geom.height).toBe(0);
742+
743+
const zeroValueIndexdGeometry = indexedGeometries.get(5);
744+
expect(zeroValueIndexdGeometry).toBeDefined();
745+
expect(zeroValueIndexdGeometry!.length).toBe(1);
746+
// moved to the bottom of the chart
747+
expect(zeroValueIndexdGeometry![0].geom.y).toBe(100);
748+
// 0 radius point
749+
expect(zeroValueIndexdGeometry![0].geom.width).toBe(0);
750+
expect(zeroValueIndexdGeometry![0].geom.height).toBe(0);
751+
});
752+
});
682753
});

‎src/lib/series/rendering.ts‎

Lines changed: 59 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { area, line } from 'd3-shape';
22
import { mutableIndexedGeometryMapUpsert } from '../../state/utils';
33
import { SharedGeometryStyle } from '../themes/theme';
44
import { SpecId } from '../utils/ids';
5+
import { isLogarithmicScale } from '../utils/scales/scale_continuous';
56
import { Scale, ScaleType } from '../utils/scales/scales';
67
import { CurveType, getCurveFactory } from './curves';
78
import { LegendItem } from './legend';
@@ -87,39 +88,56 @@ export function renderPoints(
8788
indexedGeometries: Map<any, IndexedGeometry[]>;
8889
} {
8990
const indexedGeometries: Map<any, IndexedGeometry[]> = new Map();
91+
const isLogScale = isLogarithmicScale(yScale);
9092

91-
const pointGeometries = dataset.map((datum) => {
92-
const x = xScale.scale(datum.x);
93-
const y = yScale.scale(datum.y1);
94-
const indexedGeometry: IndexedGeometry = {
95-
specId,
96-
datum: datum.datum,
97-
color,
98-
seriesKey,
99-
geom: {
100-
x: x + shift,
101-
y,
102-
width: 10,
103-
height: 10,
104-
isPoint: true,
105-
},
106-
};
107-
mutableIndexedGeometryMapUpsert(indexedGeometries, datum.x, indexedGeometry);
108-
return {
109-
x,
110-
y,
111-
color,
112-
value: {
93+
const pointGeometries = dataset.reduce(
94+
(acc, datum) => {
95+
const x = xScale.scale(datum.x);
96+
let y;
97+
let radius = 10;
98+
const isHidden = datum.y1 === null || (isLogScale && datum.y1 <= 0);
99+
// we fix 0 and negative values at y = 0
100+
if (isHidden) {
101+
y = yScale.range[0];
102+
radius = 0;
103+
} else {
104+
y = yScale.scale(datum.y1);
105+
}
106+
const indexedGeometry: IndexedGeometry = {
113107
specId,
114108
datum: datum.datum,
109+
color,
115110
seriesKey,
116-
},
117-
transform: {
118-
x: shift,
119-
y: 0,
120-
},
121-
};
122-
});
111+
geom: {
112+
x: x + shift,
113+
y,
114+
width: radius,
115+
height: radius,
116+
isPoint: true,
117+
},
118+
};
119+
mutableIndexedGeometryMapUpsert(indexedGeometries, datum.x, indexedGeometry);
120+
const pointGeometry: PointGeometry = {
121+
x,
122+
y,
123+
color,
124+
value: {
125+
specId,
126+
datum: datum.datum,
127+
seriesKey,
128+
},
129+
transform: {
130+
x: shift,
131+
y: 0,
132+
},
133+
};
134+
if (isHidden) {
135+
return acc;
136+
}
137+
return [...acc, pointGeometry];
138+
},
139+
[] as PointGeometry[],
140+
);
123141
return {
124142
pointGeometries,
125143
indexedGeometries,
@@ -216,9 +234,12 @@ export function renderLine(
216234
lineGeometry: LineGeometry;
217235
indexedGeometries: Map<any, IndexedGeometry[]>;
218236
} {
237+
const isLogScale = isLogarithmicScale(yScale);
238+
219239
const pathGenerator = line<DataSeriesDatum>()
220240
.x((datum: DataSeriesDatum) => xScale.scale(datum.x))
221241
.y((datum: DataSeriesDatum) => yScale.scale(datum.y1))
242+
.defined((datum: DataSeriesDatum) => datum.y1 !== null && !(isLogScale && datum.y1 <= 0))
222243
.curve(getCurveFactory(curve));
223244
const y = 0;
224245
const x = shift;
@@ -263,10 +284,18 @@ export function renderArea(
263284
areaGeometry: AreaGeometry;
264285
indexedGeometries: Map<any, IndexedGeometry[]>;
265286
} {
287+
const isLogScale = isLogarithmicScale(yScale);
288+
266289
const pathGenerator = area<DataSeriesDatum>()
267290
.x((datum: DataSeriesDatum) => xScale.scale(datum.x))
268291
.y1((datum: DataSeriesDatum) => yScale.scale(datum.y1))
269-
.y0((datum: DataSeriesDatum) => yScale.scale(datum.y0))
292+
.y0((datum: DataSeriesDatum) => {
293+
if (isLogScale && datum.y0 <= 0) {
294+
return yScale.range[0];
295+
}
296+
return yScale.scale(datum.y0);
297+
})
298+
.defined((datum: DataSeriesDatum) => datum.y1 !== null && !(isLogScale && datum.y1 <= 0))
270299
.curve(getCurveFactory(curve));
271300
const { lineGeometry, indexedGeometries } = renderLine(
272301
shift,

0 commit comments

Comments
 (0)