From 74198dc585b9097e67b81406973bb9da7de8c5de Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Wed, 5 Jun 2019 14:14:06 +0200 Subject: [PATCH] fix(axis_title): remove whitespace with empty axis title (#226) The whitespace left from an undefined title is removed from the computation of the axis dimension, removing the unnecessary whitespace occupied by an empty title label fix #225 --- src/components/react_canvas/axis.tsx | 6 +- src/lib/axes/axis_utils.test.ts | 81 +++++++++++++++++++ src/lib/axes/axis_utils.ts | 3 +- .../__snapshots__/dimensions.test.ts.snap | 12 +-- src/lib/utils/dimensions.ts | 11 +-- stories/styling.tsx | 12 ++- 6 files changed, 108 insertions(+), 17 deletions(-) diff --git a/src/components/react_canvas/axis.tsx b/src/components/react_canvas/axis.tsx index c912b1d29e..00da725189 100644 --- a/src/components/react_canvas/axis.tsx +++ b/src/components/react_canvas/axis.tsx @@ -91,9 +91,13 @@ export class Axis extends React.PureComponent { return ; } private renderAxis = () => { - const { ticks, axisPosition } = this.props; + const { ticks, axisPosition, debug } = this.props; return ( + {debug && ( + + ) + } {this.renderAxisLine()} {ticks.map(this.renderTickLine)} diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index 38bb9c7220..abf65b87c9 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -103,6 +103,37 @@ describe('Axis computational utils', () => { }, }; + const verticalAxisSpecWTitle: AxisSpec = { + id: getAxisId('axis_1'), + groupId: getGroupId('group_1'), + title: 'v axis', + hide: false, + showOverlappingTicks: false, + showOverlappingLabels: false, + position: Position.Left, + tickSize: 10, + tickPadding: 10, + tickFormat: (value: any) => { + return `${value}`; + }, + showGridLines: true, + }; + + // const horizontalAxisSpecWTitle: AxisSpec = { + // id: getAxisId('axis_2'), + // groupId: getGroupId('group_1'), + // title: 'h axis', + // hide: false, + // showOverlappingTicks: false, + // showOverlappingLabels: false, + // position: Position.Top, + // tickSize: 10, + // tickPadding: 10, + // tickFormat: (value: any) => { + // return `${value}`; + // }, + // }; + const xDomain: XDomain = { type: 'xDomain', scaleType: ScaleType.Linear, @@ -689,6 +720,56 @@ describe('Axis computational utils', () => { expect(horizontalAxisGridLinePositions).toEqual([10, 0, 10, 200]); }); + test('should compute axis ticks positions with title', () => { + const chartRotation = 0; + const showLegend = false; + + // validate assumptions for test + expect(verticalAxisSpec.id).toEqual(verticalAxisSpecWTitle.id); + + const axisSpecs = new Map(); + axisSpecs.set(verticalAxisSpecWTitle.id, verticalAxisSpecWTitle); + + const axisDims = new Map(); + axisDims.set(verticalAxisSpecWTitle.id, axis1Dims); + + let axisTicksPosition = getAxisTicksPositions( + chartDim, + LIGHT_THEME, + chartRotation, + showLegend, + axisSpecs, + axisDims, + xDomain, + [yDomain], + 1, + ); + + 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, height: 100 }); + + axisSpecs.set(verticalAxisSpec.id, verticalAxisSpec); + + axisDims.set(verticalAxisSpec.id, axis1Dims); + + axisTicksPosition = getAxisTicksPositions( + chartDim, + LIGHT_THEME, + chartRotation, + showLegend, + axisSpecs, + axisDims, + xDomain, + [yDomain], + 1, + ); + + left = 0 + 10 + 10; // no title + chart margin left + label width + expect(axisTicksPosition.axisPositions.get(verticalAxisSpecWTitle.id)) + .toEqual({ top: 0, left: 20, width: 10, height: 100 }); + }); + test('should compute left axis position', () => { const axisTitleHeight = 10; const cumTopSum = 10; diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index a8485bf1aa..111244f260 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -581,7 +581,8 @@ export function getAxisTicksPositions( } const { fontSize, padding } = chartTheme.axes.axisTitleStyle; - const axisTitleHeight = fontSize + padding; + + const axisTitleHeight = axisSpec.title !== undefined ? fontSize + padding : 0; const axisPosition = getAxisPosition( chartDimensions, diff --git a/src/lib/utils/__snapshots__/dimensions.test.ts.snap b/src/lib/utils/__snapshots__/dimensions.test.ts.snap index d1128b1f94..a4ba19bd47 100644 --- a/src/lib/utils/__snapshots__/dimensions.test.ts.snap +++ b/src/lib/utils/__snapshots__/dimensions.test.ts.snap @@ -11,7 +11,7 @@ Object { exports[`Computed chart dimensions should be padded by a bottom axis 1`] = ` Object { - "height": 10, + "height": 30, "left": 20, "top": 20, "width": 60, @@ -21,9 +21,9 @@ Object { exports[`Computed chart dimensions should be padded by a left axis 1`] = ` Object { "height": 60, - "left": 70, + "left": 50, "top": 20, - "width": 10, + "width": 30, } `; @@ -32,15 +32,15 @@ Object { "height": 60, "left": 20, "top": 20, - "width": 10, + "width": 30, } `; exports[`Computed chart dimensions should be padded by a top axis 1`] = ` Object { - "height": 10, + "height": 30, "left": 20, - "top": 70, + "top": 50, "width": 60, } `; diff --git a/src/lib/utils/dimensions.ts b/src/lib/utils/dimensions.ts index 97301cf436..565e291763 100644 --- a/src/lib/utils/dimensions.ts +++ b/src/lib/utils/dimensions.ts @@ -51,23 +51,24 @@ export function computeChartDimensions( if (!axisSpec || axisSpec.hide) { return; } - const { position, tickSize, tickPadding } = axisSpec; + const { position, tickSize, tickPadding, title } = axisSpec; + const titleHeight = title !== undefined ? axisTitleHeight : 0; switch (position) { case Position.Top: hTopAxisSpecHeight += - maxLabelBboxHeight + tickSize + tickPadding + chartMargins.top + axisTitleHeight; + maxLabelBboxHeight + tickSize + tickPadding + chartMargins.top + titleHeight; break; case Position.Bottom: hBottomAxisSpecHeight += - maxLabelBboxHeight + tickSize + tickPadding + chartMargins.bottom + axisTitleHeight; + maxLabelBboxHeight + tickSize + tickPadding + chartMargins.bottom + titleHeight; break; case Position.Left: vLeftAxisSpecWidth += - maxLabelBboxWidth + tickSize + tickPadding + chartMargins.left + axisTitleHeight; + maxLabelBboxWidth + tickSize + tickPadding + chartMargins.left + titleHeight; break; case Position.Right: vRightAxisSpecWidth += - maxLabelBboxWidth + tickSize + tickPadding + chartMargins.right + axisTitleHeight; + maxLabelBboxWidth + tickSize + tickPadding + chartMargins.right + titleHeight; break; } }); diff --git a/stories/styling.tsx b/stories/styling.tsx index af528f9dc1..6965b80823 100644 --- a/stories/styling.tsx +++ b/stories/styling.tsx @@ -105,6 +105,10 @@ storiesOf('Stylings', module) barsPadding: range('bar padding', 0, 1, 0.1, undefined, 0.01), }, }; + const withLeftTitle = boolean('left axis with title', true); + const withBottomTitle = boolean('bottom axis with title', true); + const withRightTitle = boolean('right axis with title', true); + const withTopTitle = boolean('top axis with title', true); const customTheme = mergeWithDefaultTheme(theme, LIGHT_THEME); return ( @@ -117,13 +121,13 @@ storiesOf('Stylings', module) Number(d).toFixed(2)} showGridLines={boolean('show left axis grid lines', false)} @@ -131,13 +135,13 @@ storiesOf('Stylings', module) Number(d).toFixed(2)} showGridLines={boolean('show right axis grid lines', false)}