Skip to content

Commit

Permalink
fix: shift bars independently from the specs order (#302)
Browse files Browse the repository at this point in the history
Each bar series needs to be shifted depending on how many bars are clustered. We don't have to shift a bar if we previously have computed a set of lines or area series.
  • Loading branch information
markov00 authored Aug 12, 2019
1 parent 0d10751 commit 1cd934d
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 2 deletions.
58 changes: 58 additions & 0 deletions src/chart_types/xy_chart/store/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,64 @@ describe('Chart State utils', () => {
expect(geometries.geometriesCounts.lines).toBe(0);
expect(geometries.geometriesCounts.areas).toBe(0);
});
test('can compute the bar offset in mixed charts', () => {
const line1: LineSeriesSpec = {
id: getSpecId('line1'),
groupId: getGroupId('group2'),
seriesType: 'line',
yScaleType: ScaleType.Log,
xScaleType: ScaleType.Linear,
xAccessor: 'x',
yAccessors: ['y'],
splitSeriesAccessors: ['g'],
yScaleToDataExtent: false,
data: BARCHART_1Y1G,
};

const bar1: BarSeriesSpec = {
id: getSpecId('line3'),
groupId: getGroupId('group2'),
seriesType: 'bar',
yScaleType: ScaleType.Log,
xScaleType: ScaleType.Linear,
xAccessor: 'x',
yAccessors: ['y'],
splitSeriesAccessors: ['g'],
yScaleToDataExtent: false,
data: BARCHART_1Y1G,
};
const seriesSpecs = new Map<SpecId, BasicSeriesSpec>([[line1.id, line1], [bar1.id, bar1]]);
const axesSpecs = new Map<AxisId, AxisSpec>();
const chartRotation = 0;
const chartDimensions = { width: 100, height: 100, top: 0, left: 0 };
const chartColors = {
vizColors: ['violet', 'green', 'blue'],
defaultVizColor: 'red',
};
const chartTheme = {
...LIGHT_THEME,
scales: {
barsPadding: 0,
histogramPadding: 0,
},
};
const domainsByGroupId = mergeYCustomDomainsByGroupId(axesSpecs, chartRotation);
const seriesDomains = computeSeriesDomains(seriesSpecs, domainsByGroupId);
const seriesColorMap = getSeriesColorMap(seriesDomains.seriesColors, chartColors, new Map());
const geometries = computeSeriesGeometries(
seriesSpecs,
seriesDomains.xDomain,
seriesDomains.yDomain,
seriesDomains.formattedDataSeries,
seriesColorMap,
chartTheme,
chartDimensions,
chartRotation,
axesSpecs,
false,
);
expect(geometries.geometries.bars[0].x).toBe(0);
});
});
test('can merge geometry indexes', () => {
const map1 = new Map<string, IndexedGeometry[]>();
Expand Down
5 changes: 3 additions & 2 deletions src/chart_types/xy_chart/store/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ export function renderGeometries(
lines: 0,
linePoints: 0,
};
let barIndexOffset = 0;
for (i = 0; i < len; i++) {
const ds = dataSeries[i];
const spec = getSpecById(seriesSpecs, ds.specId);
Expand All @@ -425,8 +426,7 @@ export function renderGeometries(
const color = seriesColorsMap.get(ds.seriesColorKey) || defaultColor;

if (isBarSeriesSpec(spec)) {
const shift = isStacked ? indexOffset : indexOffset + i;

const shift = isStacked ? indexOffset : indexOffset + barIndexOffset;
const barSeriesStyle = mergePartial(chartTheme.barSeriesStyle, spec.barSeriesStyle, {
mergeOptionalPartialValues: true,
});
Expand Down Expand Up @@ -456,6 +456,7 @@ export function renderGeometries(
barGeometriesIndex = mergeGeometriesIndexes(barGeometriesIndex, renderedBars.indexedGeometries);
bars.push(...renderedBars.barGeometries);
geometriesCounts.bars += renderedBars.barGeometries.length;
barIndexOffset += 1;
} else if (isLineSeriesSpec(spec)) {
const lineShift = clusteredCount > 0 ? clusteredCount : 1;
const lineSeriesStyle = spec.lineSeriesStyle
Expand Down

0 comments on commit 1cd934d

Please sign in to comment.