Skip to content

Commit

Permalink
fix(xy): negative bar highlight and click [25.0.x] (#1126)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nickofthyme
nickofthyme authored Apr 20, 2021
1 parent b2a69f2 commit 95ac3ea
Show file tree
Hide file tree
Showing 5 changed files with 198 additions and 181 deletions.
Binary file modified ...ll-stories-interactions-bar-clicks-and-hovers-visually-looks-correct-1-snap.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 9 additions & 11 deletions src/chart_types/xy_chart/rendering/bars.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function renderBars(
sharedSeriesStyle: BarSeriesStyle,
displayValueSettings?: DisplayValueSpec,
styleAccessor?: BarStyleAccessor,
minBarHeight?: number,
minBarHeight: number = 0,
stackMode?: StackMode,
chartRotation?: number,
): {
Expand All @@ -53,7 +53,6 @@ export function renderBars(
// default padding to 1 for now
const padding = 1;
const { fontSize, fontFamily } = sharedSeriesStyle.displayValue;
const absMinHeight = minBarHeight && Math.abs(minBarHeight);

dataSeries.data.forEach((datum) => {
const { y0, y1, initialY1, filled } = datum;
Expand All @@ -77,21 +76,17 @@ export function renderBars(
}
} else {
y = yScale.scale(y1);
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);
}
// use always zero as baseline if y0 is null
y0Scaled = y0 === null ? yScale.scale(0) : yScale.scale(y0);
}

if (y === null || y0Scaled === null) {
return;
}
let height = y0Scaled - y;

// handle minBarHeight adjustment
if (absMinHeight !== undefined && height !== 0 && Math.abs(height) < absMinHeight) {
const absMinHeight = Math.abs(minBarHeight);
let height = y0Scaled - y;
if (height !== 0 && Math.abs(height) < absMinHeight) {
const heightDelta = absMinHeight - Math.abs(height);
if (height < 0) {
height = -absMinHeight;
Expand All @@ -101,6 +96,9 @@ export function renderBars(
y -= heightDelta;
}
}
const isUpsideDown = height < 0;
height = Math.abs(height);
y = isUpsideDown ? y - height : y;

const xScaled = xScale.scale(datum.x);

Expand Down
291 changes: 133 additions & 158 deletions src/chart_types/xy_chart/rendering/rendering.bars.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,164 +209,139 @@ describe('Rendering bars', () => {
expect(geometries.bars[0].value[0].displayValue?.width).toBe(50);
});
});
// describe('Multi series bar chart - ordinal', () => {
// const spec1Id = 'bar1';
// const spec2Id = 'bar2';
// const barSeriesSpec1: BarSeriesSpec = {
// chartType: ChartTypes.XYAxis,
// specType: SpecTypes.Series,
// id: spec1Id,
// groupId: GROUP_ID,
// seriesType: SeriesTypes.Bar,
// data: [
// [0, 10],
// [1, 5],
// ],
// xAccessor: 0,
// yAccessors: [1],
// xScaleType: ScaleType.Ordinal,
// yScaleType: ScaleType.Linear,
// };
// const barSeriesSpec2: BarSeriesSpec = {
// chartType: ChartTypes.XYAxis,
// specType: SpecTypes.Series,
// id: spec2Id,
// groupId: GROUP_ID,
// seriesType: SeriesTypes.Bar,
// data: [
// [0, 20],
// [1, 10],
// ],
// xAccessor: 0,
// yAccessors: [1],
// xScaleType: ScaleType.Ordinal,
// yScaleType: ScaleType.Linear,
// };
// const barSeriesMap = [barSeriesSpec1, barSeriesSpec2];
// 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] });
// const getBarGeometry = MockBarGeometry.fromBaseline(
// {
// x: 0,
// y: 0,
// width: 50,
// height: 100,
// color: 'red',
// value: {
// accessor: 'y1',
// x: 0,
// y: 10,
// mark: null,
// },
// seriesIdentifier: {
// specId: spec1Id,
// key: 'spec{bar1}yAccessor{1}splitAccessors{}',
// yAccessor: 1,
// splitAccessors: new Map(),
// seriesKeys: [1],
// },
// },
// 'displayValue',
// );
//
// test('can render first spec bars', () => {
// const { barGeometries } = renderBars(
// 0,
// barSeriesDomains.formattedDataSeries.nonStacked[0].dataSeries[0],
// xScale,
// yScales.get(GROUP_ID)!,
// 'red',
// LIGHT_THEME.barSeriesStyle,
// );
// expect(barGeometries.length).toEqual(2);
// expect(barGeometries[0]).toEqual(
// getBarGeometry({
// x: 0,
// y: 50,
// width: 25,
// height: 50,
// value: {
// x: 0,
// y: 10,
// },
// }),
// );
// expect(barGeometries[1]).toEqual(
// getBarGeometry({
// x: 50,
// y: 75,
// width: 25,
// height: 25,
// value: {
// x: 1,
// y: 5,
// },
// }),
// );
// });
// test('can render second spec bars', () => {
// const { barGeometries } = renderBars(
// 1,
// barSeriesDomains.formattedDataSeries.nonStacked[0].dataSeries[1],
// xScale,
// yScales.get(GROUP_ID)!,
// 'blue',
// LIGHT_THEME.barSeriesStyle,
// );
// const getBarGeometry = MockBarGeometry.fromBaseline(
// {
// x: 0,
// y: 0,
// width: 50,
// height: 100,
// color: 'blue',
// value: {
// accessor: 'y1',
// x: 0,
// y: 10,
// },
// seriesIdentifier: {
// specId: spec2Id,
// key: 'spec{bar2}yAccessor{1}splitAccessors{}',
// yAccessor: 1,
// splitAccessors: new Map(),
// seriesKeys: [1],
// },
// },
// 'displayValue',
// );
// expect(barGeometries.length).toEqual(2);
// expect(barGeometries[0]).toEqual(
// getBarGeometry({
// x: 25,
// y: 0,
// width: 25,
// height: 100,
// value: {
// x: 0,
// y: 20,
// },
// }),
// );
// expect(barGeometries[1]).toEqual(
// getBarGeometry({
// x: 75,
// y: 50,
// width: 25,
// height: 50,
// value: {
// x: 1,
// y: 10,
// },
// }),
// );
// });
// });
describe('Multi series bar chart - ordinal', () => {
const spec1Id = 'bar1';
const spec2Id = 'bar2';
const barSeriesSpec1 = MockSeriesSpec.bar({
id: spec1Id,
groupId: GROUP_ID,
data: [
[0, 10],
[1, 5],
],
xAccessor: 0,
yAccessors: [1],
xScaleType: ScaleType.Ordinal,
yScaleType: ScaleType.Linear,
});
const barSeriesSpec2 = MockSeriesSpec.bar({
id: spec2Id,
groupId: GROUP_ID,
data: [
[0, 20],
[1, 10],
],
xAccessor: 0,
yAccessors: [1],
xScaleType: ScaleType.Ordinal,
yScaleType: ScaleType.Linear,
});
const store = MockStore.default({ width: 100, height: 100, top: 0, left: 0 });
MockStore.addSpecs(
[
barSeriesSpec1,
barSeriesSpec2,
MockGlobalSpec.settingsNoMargins({ theme: { colors: { vizColors: ['red', 'blue'] } } }),
],
store,
);

const getBarGeometry = MockBarGeometry.fromBaseline(
{
x: 0,
y: 0,
width: 50,
height: 100,
color: 'red',
value: {
accessor: 'y1',
x: 0,
y: 10,
mark: null,
},
seriesIdentifier: MockSeriesIdentifier.fromSpec(barSeriesSpec1),
},
'displayValue',
);
const {
geometries: { bars },
} = computeSeriesGeometriesSelector(store.getState());

test('can render first spec bars', () => {
expect(bars[0].value.length).toEqual(2);
expect(bars[0].value[0]).toEqual(
getBarGeometry({
x: 0,
y: 50,
width: 25,
height: 50,
value: {
x: 0,
y: 10,
datum: [0, 10],
},
}),
);
expect(bars[0].value[1]).toEqual(
getBarGeometry({
x: 50,
y: 75,
width: 25,
height: 25,
value: {
x: 1,
y: 5,
datum: [1, 5],
},
}),
);
});
test('can render second spec bars', () => {
const getBarGeometry = MockBarGeometry.fromBaseline(
{
x: 0,
y: 0,
width: 50,
height: 100,
color: 'blue',
value: {
accessor: 'y1',
x: 0,
y: 10,
},
seriesIdentifier: MockSeriesIdentifier.fromSpec(barSeriesSpec2),
},
'displayValue',
);
expect(bars[1].value.length).toEqual(2);
expect(bars[1].value[0]).toEqual(
getBarGeometry({
x: 25,
y: 0,
width: 25,
height: 100,
value: {
x: 0,
y: 20,
datum: [0, 20],
},
}),
);
expect(bars[1].value[1]).toEqual(
getBarGeometry({
x: 75,
y: 50,
width: 25,
height: 50,
value: {
x: 1,
y: 10,
datum: [1, 10],
},
}),
);
});
});
// describe('Single series bar chart - linear', () => {
// const barSeriesSpec: BarSeriesSpec = {
// chartType: ChartTypes.XYAxis,
Expand Down
Loading

0 comments on commit 95ac3ea

Please sign in to comment.