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.

Sign up for GitHub

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(axis): limit chart dimensions to avoid axis labels overflow #314

Merged
merged 2 commits into from
Aug 16, 2019
Merged

fix(axis): limit chart dimensions to avoid axis labels overflow #314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
31a3df3
fix(axis): limit chart dimensions to avoid axis labels overflow
markov00 Aug 15, 2019
4b0a54a
Merge branch 'master' into fix_overflow_axis_labels
markov00 Aug 16, 2019
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
7 changes: 4 additions & 3 deletions src/chart_types/xy_chart/store/chart_state.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,15 +913,16 @@ export class ChartStore {
});
bboxCalculator.destroy();

// compute chart dimensions
this.chartDimensions = computeChartDimensions(
// // compute chart dimensions
const computedChartDims = computeChartDimensions(
this.parentDimensions,
this.chartTheme,
this.axesTicksDimensions,
this.axesSpecs,
this.showLegend.get() && !this.legendCollapsed.get(),
this.legendPosition,
);
this.chartDimensions = computedChartDims.chartDimensions;

this.chartTransform = computeChartTransform(this.chartDimensions, this.chartRotation);
this.brushExtent = computeBrushExtent(this.chartDimensions, this.chartRotation, this.chartTransform);
Expand Down Expand Up @@ -954,7 +955,7 @@ export class ChartStore {

// compute visible ticks and their positions
const axisTicksPositions = getAxisTicksPositions(
this.chartDimensions,
computedChartDims,
this.chartTheme,
this.chartRotation,
this.showLegend.get() && !this.legendCollapsed.get(),
Expand Down
16 changes: 8 additions & 8 deletions src/chart_types/xy_chart/utils/__snapshots__/dimensions.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,35 @@ Object {
exports[`Computed chart dimensions should be padded by a bottom axis 1`] = `
Object {
"height": 30,
"left": 20,
"left": 25,
"top": 20,
"width": 60,
"width": 50,
}
`;

exports[`Computed chart dimensions should be padded by a left axis 1`] = `
Object {
"height": 60,
"height": 50,
"left": 50,
"top": 20,
"top": 25,
"width": 30,
}
`;

exports[`Computed chart dimensions should be padded by a right axis 1`] = `
Object {
"height": 60,
"height": 50,
"left": 20,
"top": 20,
"top": 25,
"width": 30,
}
`;

exports[`Computed chart dimensions should be padded by a top axis 1`] = `
Object {
"height": 30,
"left": 20,
"left": 25,
"top": 50,
"width": 60,
"width": 50,
}
`;
101 changes: 65 additions & 36 deletions src/chart_types/xy_chart/utils/axis_utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,18 +565,24 @@ describe('Axis computational utils', () => {
const tickSize = 10;
const tickPadding = 5;
const tickPosition = 0;

const axisPosition = {
top: 0,
left: 0,
width: 100,
height: 100,
};
const unrotatedLabelProps = getTickLabelProps(
tickLabelRotation,
tickSize,
tickPadding,
tickPosition,
Position.Left,
axisPosition,
axis1Dims,
);

expect(unrotatedLabelProps).toEqual({
x: -10,
x: 75,
y: -5,
align: 'right',
verticalAlign: 'middle',
Expand All @@ -589,11 +595,12 @@ describe('Axis computational utils', () => {
tickPadding,
tickPosition,
Position.Left,
axisPosition,
axis1Dims,
);

expect(rotatedLabelProps).toEqual({
x: -10,
x: 75,
y: -5,
align: 'center',
verticalAlign: 'middle',
Expand All @@ -605,6 +612,7 @@ describe('Axis computational utils', () => {
tickPadding,
tickPosition,
Position.Right,
axisPosition,
axis1Dims,
);

Expand All @@ -622,6 +630,7 @@ describe('Axis computational utils', () => {
tickPadding,
tickPosition,
Position.Right,
axisPosition,
axis1Dims,
);

Expand All @@ -638,19 +647,26 @@ describe('Axis computational utils', () => {
const tickSize = 10;
const tickPadding = 5;
const tickPosition = 0;
const axisPosition = {
top: 0,
left: 0,
width: 100,
height: 100,
};

const unrotatedLabelProps = getTickLabelProps(
tickLabelRotation,
tickSize,
tickPadding,
tickPosition,
Position.Top,
axisPosition,
axis1Dims,
);

expect(unrotatedLabelProps).toEqual({
x: -5,
y: 0,
y: 75,
align: 'center',
verticalAlign: 'bottom',
});
Expand All @@ -662,12 +678,13 @@ describe('Axis computational utils', () => {
tickPadding,
tickPosition,
Position.Top,
axisPosition,
axis1Dims,
);

expect(rotatedLabelProps).toEqual({
x: -5,
y: 0,
y: 75,
align: 'center',
verticalAlign: 'middle',
});
Expand All @@ -678,6 +695,7 @@ describe('Axis computational utils', () => {
tickPadding,
tickPosition,
Position.Bottom,
axisPosition,
axis1Dims,
);

Expand All @@ -695,6 +713,7 @@ describe('Axis computational utils', () => {
tickPadding,
tickPosition,
Position.Bottom,
axisPosition,
axis1Dims,
);

Expand All @@ -710,11 +729,11 @@ describe('Axis computational utils', () => {
const tickPadding = 5;
const tickSize = 10;
const tickPosition = 10;
const maxLabelBboxHeight = 20;
const axisHeight = 20;

const leftAxisTickLinePositions = getVerticalAxisTickLineProps(Position.Left, tickPadding, tickSize, tickPosition);

expect(leftAxisTickLinePositions).toEqual([5, 10, 15, 10]);
expect(leftAxisTickLinePositions).toEqual([5, 10, -5, 10]);

const rightAxisTickLinePositions = getVerticalAxisTickLineProps(
Position.Right,
Expand All @@ -725,22 +744,15 @@ describe('Axis computational utils', () => {

expect(rightAxisTickLinePositions).toEqual([0, 10, 10, 10]);

const topAxisTickLinePositions = getHorizontalAxisTickLineProps(
Position.Top,
tickPadding,
tickSize,
tickPosition,
maxLabelBboxHeight,
);
const topAxisTickLinePositions = getHorizontalAxisTickLineProps(Position.Top, axisHeight, tickSize, tickPosition);

expect(topAxisTickLinePositions).toEqual([10, 25, 10, 35]);
expect(topAxisTickLinePositions).toEqual([10, 10, 10, 20]);

const bottomAxisTickLinePositions = getHorizontalAxisTickLineProps(
Position.Bottom,
tickPadding,
axisHeight,
tickSize,
tickPosition,
maxLabelBboxHeight,
);

expect(bottomAxisTickLinePositions).toEqual([10, 0, 10, 10]);
Expand Down Expand Up @@ -774,7 +786,10 @@ describe('Axis computational utils', () => {
axisDims.set(verticalAxisSpecWTitle.id, axis1Dims);

let axisTicksPosition = getAxisTicksPositions(
chartDim,
{
chartDimensions: chartDim,
leftMargin: 0,
},
LIGHT_THEME,
chartRotation,
showLegend,
Expand All @@ -786,11 +801,10 @@ describe('Axis computational utils', () => {
false,
);

let left = 12 + 5 + 10 + 10; // font size + title padding + chart margin left + label width
expect(axisTicksPosition.axisPositions.get(verticalAxisSpecWTitle.id)).toEqual({
top: 0,
left,
width: 10,
left: 10,
width: 12 + 5 + 10 + 10 + 10,
height: 100,
});

Expand All @@ -799,7 +813,10 @@ describe('Axis computational utils', () => {
axisDims.set(verticalAxisSpec.id, axis1Dims);

axisTicksPosition = getAxisTicksPositions(
chartDim,
{
chartDimensions: chartDim,
leftMargin: 0,
},
LIGHT_THEME,
chartRotation,
showLegend,
Expand All @@ -811,11 +828,10 @@ describe('Axis computational utils', () => {
false,
);

left = 0 + 10 + 10; // no title + chart margin left + label width
expect(axisTicksPosition.axisPositions.get(verticalAxisSpecWTitle.id)).toEqual({
top: 0,
left: 20,
width: 10,
left: 10,
width: 30,
height: 100,
});
});
Expand All @@ -842,8 +858,8 @@ describe('Axis computational utils', () => {
const expectedLeftAxisPosition = {
dimensions: {
height: 100,
width: 10,
left: 40,
width: 40,
left: 20,
top: 0,
},
topIncrement: 0,
Expand Down Expand Up @@ -878,7 +894,7 @@ describe('Axis computational utils', () => {
const expectedRightAxisPosition = {
dimensions: {
height: 100,
width: 10,
width: 40,
left: 110,
top: 0,
},
Expand Down Expand Up @@ -913,10 +929,11 @@ describe('Axis computational utils', () => {

const expectedTopAxisPosition = {
dimensions: {
height: 10,
height:
axis1Dims.maxLabelBboxHeight + axisTitleHeight + horizontalAxisSpec.tickSize + horizontalAxisSpec.tickPadding,
width: 100,
left: 0,
top: 30,
top: cumTopSum + LIGHT_THEME.chartMargins.top,
},
topIncrement: 50,
bottomIncrement: 0,
Expand Down Expand Up @@ -949,7 +966,7 @@ describe('Axis computational utils', () => {

const expectedBottomAxisPosition = {
dimensions: {
height: 10,
height: 40,
width: 100,
left: 0,
top: 110,
Expand All @@ -975,7 +992,10 @@ describe('Axis computational utils', () => {
axisDims.set(getAxisId('not_a_mapped_one'), axis1Dims);

const axisTicksPosition = getAxisTicksPositions(
chartDim,
{
chartDimensions: chartDim,
leftMargin: 0,
},
LIGHT_THEME,
chartRotation,
showLegend,
Expand Down Expand Up @@ -1006,7 +1026,10 @@ describe('Axis computational utils', () => {
axisDims.set(verticalAxisSpec.id, axis1Dims);

const axisTicksPosition = getAxisTicksPositions(
chartDim,
{
chartDimensions: chartDim,
leftMargin: 0,
},
LIGHT_THEME,
chartRotation,
showLegend,
Expand Down Expand Up @@ -1036,7 +1059,10 @@ describe('Axis computational utils', () => {
expect(axisTicksPosition.axisGridLinesPositions.get(verticalAxisSpec.id)).toEqual(expectedVerticalAxisGridLines);

const axisTicksPositionWithTopLegend = getAxisTicksPositions(
chartDim,
{
chartDimensions: chartDim,
leftMargin: 0,
},
LIGHT_THEME,
chartRotation,
showLegend,
Expand All @@ -1051,7 +1077,7 @@ describe('Axis computational utils', () => {

const expectedPositionWithTopLegend = {
height: 100,
width: 10,
width: 30,
left: 100,
top: 0,
};
Expand All @@ -1063,7 +1089,10 @@ describe('Axis computational utils', () => {
invalidSpecs.set(verticalAxisSpec.id, ungroupedAxisSpec);
const computeScalelessSpec = () => {
getAxisTicksPositions(
chartDim,
{
chartDimensions: chartDim,
leftMargin: 0,
},
LIGHT_THEME,
chartRotation,
showLegend,
Expand Down
Loading