diff --git a/src/components/react_canvas/axis.tsx b/src/components/react_canvas/axis.tsx index 7cdf521760..67ba805a2e 100644 --- a/src/components/react_canvas/axis.tsx +++ b/src/components/react_canvas/axis.tsx @@ -29,7 +29,16 @@ export class Axis extends React.PureComponent { return this.renderAxis(); } renderTickLabel = (tick: AxisTick, i: number) => { - const { padding, ...labelStyle } = this.props.chartTheme.axes.tickLabelStyle; + /** + * padding is already computed through width + * and bbox_calculator using tickLabelPadding + * set padding to 0 to avoid conflict + */ + const labelStyle = { + ...this.props.chartTheme.axes.tickLabelStyle, + padding: 0, + }; + const { axisSpec: { tickSize, tickPadding, position }, axisTicksDimensions, @@ -48,6 +57,7 @@ export class Axis extends React.PureComponent { ); const { maxLabelTextWidth, maxLabelTextHeight } = axisTicksDimensions; + const centeredRectProps = centerRotationOrigin(axisTicksDimensions, { x: tickLabelProps.x, y: tickLabelProps.y, diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index 4ccf891372..9adc7bad6f 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -1,6 +1,6 @@ import { XDomain } from '../series/domains/x_domain'; import { YDomain } from '../series/domains/y_domain'; -import { AxisSpec, DomainRange, Position } from '../series/specs'; +import { AxisSpec, DomainRange, Position, AxisStyle } from '../series/specs'; import { LIGHT_THEME } from '../themes/light_theme'; import { AxisId, getAxisId, getGroupId, GroupId } from '../utils/ids'; import { ScaleType } from '../utils/scales/scales'; @@ -28,6 +28,7 @@ import { isVertical, isYDomain, mergeDomainsByGroupId, + getAxisTickLabelPadding, } from './axis_utils'; import { CanvasTextBBoxCalculator } from './canvas_text_bbox_calculator'; import { SvgTextBBoxCalculator } from './svg_text_bbox_calculator'; @@ -458,7 +459,7 @@ describe('Axis computational utils', () => { }); test('should get max bbox dimensions for a tick in comparison to previous values', () => { const bboxCalculator = new CanvasTextBBoxCalculator(); - const reducer = getMaxBboxDimensions(bboxCalculator, 16, 'Arial', 0); + const reducer = getMaxBboxDimensions(bboxCalculator, 16, 'Arial', 0, 1); const accWithGreaterValues = { maxLabelBboxWidth: 100, @@ -1272,4 +1273,24 @@ describe('Axis computational utils', () => { expect(isBounded(lowerBounded)).toBe(true); expect(isBounded(upperBounded)).toBe(true); }); + test('should not allow negative padding', () => { + const negativePadding = -2; + // value canvas_text_bbox_calculator changes negative values is 1 + const positivePadding = 1; + + const bboxCalculator = new CanvasTextBBoxCalculator(); + const negativeReducer = getMaxBboxDimensions(bboxCalculator, 16, 'Arial', 0, negativePadding); + const positiveReducer = getMaxBboxDimensions(bboxCalculator, 16, 'Arial', 0, positivePadding); + + expect(JSON.stringify(negativeReducer)).toEqual(JSON.stringify(positiveReducer)); + }); + test('should expect axisSpec.style.tickLabelPadding if specified', () => { + const axisSpecStyle: AxisStyle = { + tickLabelPadding: 2, + }; + + const axisConfigTickLabelPadding = 1; + + expect(getAxisTickLabelPadding(axisConfigTickLabelPadding, axisSpecStyle)).toEqual(2); + }); }); diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 842e6ad31f..114f3d86f4 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -10,6 +10,7 @@ import { Rotation, TickFormatter, UpperBoundedDomain, + AxisStyle, } from '../series/specs'; import { AxisConfig, Theme } from '../themes/theme'; import { Dimensions, Margins } from '../utils/dimensions'; @@ -44,7 +45,7 @@ export interface TickLabelProps { /** * Compute the ticks values and identify max width and height of the labels * so we can compute the max space occupied by the axis component. - * @param axisSpec tbe spec of the axis + * @param axisSpec the spec of the axis * @param xDomain the x domain associated * @param yDomain the y domain array * @param totalBarsInCluster the total number of grouped series @@ -69,11 +70,15 @@ export function computeAxisTicksDimensions( if (!scale) { throw new Error(`Cannot compute scale for axis spec ${axisSpec.id}`); } + + const tickLabelPadding = getAxisTickLabelPadding(axisConfig.tickLabelStyle.padding, axisSpec.style); + const dimensions = computeTickDimensions( scale, axisSpec.tickFormat, bboxCalculator, axisConfig, + tickLabelPadding, axisSpec.tickLabelRotation, ); @@ -82,6 +87,13 @@ export function computeAxisTicksDimensions( }; } +export function getAxisTickLabelPadding(axisConfigTickLabelPadding: number, axisSpecStyle?: AxisStyle): number { + if (axisSpecStyle && axisSpecStyle.tickLabelPadding !== undefined) { + return axisSpecStyle.tickLabelPadding; + } + return axisConfigTickLabelPadding; +} + export function isYDomain(position: Position, chartRotation: Rotation): boolean { const isStraightRotation = chartRotation === 0 || chartRotation === 180; if (isVertical(position)) { @@ -133,6 +145,7 @@ export const getMaxBboxDimensions = ( fontSize: number, fontFamily: string, tickLabelRotation: number, + tickLabelPadding: number, ) => ( acc: { [key: string]: number }, tickLabel: string, @@ -142,7 +155,7 @@ export const getMaxBboxDimensions = ( maxLabelTextWidth: number; maxLabelTextHeight: number; } => { - const bbox = bboxCalculator.compute(tickLabel, fontSize, fontFamily).getOrElse({ + const bbox = bboxCalculator.compute(tickLabel, tickLabelPadding, fontSize, fontFamily).getOrElse({ width: 0, height: 0, }); @@ -158,7 +171,6 @@ export const getMaxBboxDimensions = ( const prevHeight = acc.maxLabelBboxHeight; const prevLabelWidth = acc.maxLabelTextWidth; const prevLabelHeight = acc.maxLabelTextHeight; - return { maxLabelBboxWidth: prevWidth > width ? prevWidth : width, maxLabelBboxHeight: prevHeight > height ? prevHeight : height, @@ -172,6 +184,7 @@ function computeTickDimensions( tickFormat: TickFormatter, bboxCalculator: BBoxCalculator, axisConfig: AxisConfig, + tickLabelPadding: number, tickLabelRotation: number = 0, ) { const tickValues = scale.ticks(); @@ -182,7 +195,7 @@ function computeTickDimensions( } = axisConfig; const { maxLabelBboxWidth, maxLabelBboxHeight, maxLabelTextWidth, maxLabelTextHeight } = tickLabels.reduce( - getMaxBboxDimensions(bboxCalculator, fontSize, fontFamily, tickLabelRotation), + getMaxBboxDimensions(bboxCalculator, fontSize, fontFamily, tickLabelRotation, tickLabelPadding), { maxLabelBboxWidth: 0, maxLabelBboxHeight: 0, maxLabelTextWidth: 0, maxLabelTextHeight: 0 }, ); diff --git a/src/lib/axes/bbox_calculator.ts b/src/lib/axes/bbox_calculator.ts index 8d8bffe953..dd172cdeec 100644 --- a/src/lib/axes/bbox_calculator.ts +++ b/src/lib/axes/bbox_calculator.ts @@ -6,6 +6,6 @@ export interface BBox { } export interface BBoxCalculator { - compute(text: string, fontSize?: number, fontFamily?: string): Option; + compute(text: string, padding: number, fontSize?: number, fontFamily?: string): Option; destroy(): void; } diff --git a/src/lib/axes/canvas_text_bbox_calculator.test.ts b/src/lib/axes/canvas_text_bbox_calculator.test.ts index 12aa0c05cc..eb726dca78 100644 --- a/src/lib/axes/canvas_text_bbox_calculator.test.ts +++ b/src/lib/axes/canvas_text_bbox_calculator.test.ts @@ -4,7 +4,7 @@ import { CanvasTextBBoxCalculator } from './canvas_text_bbox_calculator'; describe('CanvasTextBBoxCalculator', () => { test('can create a canvas for computing text measurement values', () => { const canvasBboxCalculator = new CanvasTextBBoxCalculator(); - const bbox = canvasBboxCalculator.compute('foo').getOrElse({ + const bbox = canvasBboxCalculator.compute('foo', 0).getOrElse({ width: 0, height: 0, }); @@ -12,12 +12,12 @@ describe('CanvasTextBBoxCalculator', () => { expect(bbox.height).toBe(16); canvasBboxCalculator.context = null; - expect(canvasBboxCalculator.compute('foo')).toBe(none); + expect(canvasBboxCalculator.compute('foo', 0)).toBe(none); }); test('can compute near the same width for the same text independently of the scale factor', () => { let canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 5); - let bbox = canvasBboxCalculator.compute('foo').getOrElse({ + let bbox = canvasBboxCalculator.compute('foo', 0).getOrElse({ width: 0, height: 0, }); @@ -26,7 +26,7 @@ describe('CanvasTextBBoxCalculator', () => { canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 10); - bbox = canvasBboxCalculator.compute('foo').getOrElse({ + bbox = canvasBboxCalculator.compute('foo', 0).getOrElse({ width: 0, height: 0, }); @@ -35,7 +35,7 @@ describe('CanvasTextBBoxCalculator', () => { canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 50); - bbox = canvasBboxCalculator.compute('foo').getOrElse({ + bbox = canvasBboxCalculator.compute('foo', 0).getOrElse({ width: 0, height: 0, }); @@ -44,7 +44,7 @@ describe('CanvasTextBBoxCalculator', () => { canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 100); - bbox = canvasBboxCalculator.compute('foo').getOrElse({ + bbox = canvasBboxCalculator.compute('foo', 0).getOrElse({ width: 0, height: 0, }); @@ -53,7 +53,7 @@ describe('CanvasTextBBoxCalculator', () => { canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 1000); - bbox = canvasBboxCalculator.compute('foo').getOrElse({ + bbox = canvasBboxCalculator.compute('foo', 0).getOrElse({ width: 0, height: 0, }); diff --git a/src/lib/axes/canvas_text_bbox_calculator.ts b/src/lib/axes/canvas_text_bbox_calculator.ts index f3143cf9a8..82308cd654 100644 --- a/src/lib/axes/canvas_text_bbox_calculator.ts +++ b/src/lib/axes/canvas_text_bbox_calculator.ts @@ -17,11 +17,16 @@ export class CanvasTextBBoxCalculator implements BBoxCalculator { this.attachedRoot.appendChild(this.offscreenCanvas); this.scaledFontSize = scaledFontSize; } - compute(text: string, fontSize = 16, fontFamily = 'Arial', padding: number = 1): Option { + compute(text: string, padding: number, fontSize = 16, fontFamily = 'Arial'): Option { if (!this.context) { return none; } + // Padding should be at least one to avoid browser measureText inconsistencies + if (padding < 1) { + padding = 1; + } + // We scale the text up to get a more accurate computation of the width of the text // because `measureText` can vary a lot between browsers. const scalingFactor = this.scaledFontSize / fontSize; diff --git a/src/lib/series/rendering.ts b/src/lib/series/rendering.ts index c25e36ab50..132f5fead4 100644 --- a/src/lib/series/rendering.ts +++ b/src/lib/series/rendering.ts @@ -207,6 +207,8 @@ export function renderBars( const barGeometries: BarGeometry[] = []; const bboxCalculator = new CanvasTextBBoxCalculator(); + // default padding to 1 for now + const padding = 1; const fontSize = seriesStyle && seriesStyle.displayValue ? seriesStyle.displayValue.fontSize : undefined; const fontFamily = seriesStyle && seriesStyle.displayValue ? seriesStyle.displayValue.fontFamily : undefined; @@ -253,10 +255,12 @@ export function renderBars( : undefined : formattedDisplayValue; - const computedDisplayValueWidth = bboxCalculator.compute(displayValueText || '', fontSize, fontFamily).getOrElse({ - width: 0, - height: 0, - }).width; + const computedDisplayValueWidth = bboxCalculator + .compute(displayValueText || '', padding, fontSize, fontFamily) + .getOrElse({ + width: 0, + height: 0, + }).width; const displayValueWidth = displayValueSettings && displayValueSettings.isValueContainedInElement ? width : computedDisplayValueWidth; diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index be0f59b7b2..ab8599f20e 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -218,10 +218,17 @@ export interface AxisSpec { title?: string; /** If specified, it constrains the domain for these values */ domain?: DomainRange; + /** Object to hold custom styling */ + style?: AxisStyle; } export type TickFormatter = (value: any) => string; +export interface AxisStyle { + /** Specifies the amount of padding on the tick label bounding box */ + tickLabelPadding?: number; +} + /** * The position of the axis relative to the chart. * A left or right positioned axis is a vertical axis. diff --git a/src/lib/themes/dark_theme.ts b/src/lib/themes/dark_theme.ts index 0905a8eac3..def00d3c47 100644 --- a/src/lib/themes/dark_theme.ts +++ b/src/lib/themes/dark_theme.ts @@ -92,7 +92,7 @@ export const DARK_THEME: Theme = { fontFamily: 'sans-serif', fontStyle: 'normal', fill: 'white', - padding: 0, + padding: 1, }, tickLineStyle: { stroke: 'white', diff --git a/src/lib/themes/light_theme.ts b/src/lib/themes/light_theme.ts index cce01be178..b6bf5da46b 100644 --- a/src/lib/themes/light_theme.ts +++ b/src/lib/themes/light_theme.ts @@ -92,7 +92,7 @@ export const LIGHT_THEME: Theme = { fontFamily: 'sans-serif', fontStyle: 'normal', fill: 'gray', - padding: 0, + padding: 1, }, tickLineStyle: { stroke: 'gray', diff --git a/stories/axis.tsx b/stories/axis.tsx index 0d1f67a0eb..0286749640 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -54,15 +54,26 @@ function renderAxisWithOptions(position: Position, seriesGroup: string, show: bo storiesOf('Axis', module) .add('basic', () => { + const customStyle = { + tickLabelPadding: number('Tick Label Padding', 0), + }; + return ( - + Number(d).toFixed(2)} + style={customStyle} /> { + const customStyle = { + tickLabelPadding: number('Tick Label Padding', 0), + }; + return ( Number(d).toFixed(2)} + style={customStyle} /> Number(d).toFixed(2)} + style={customStyle} /> Number(d).toFixed(2)} + style={customStyle} /> { const dg = new DataGenerator(); const data = dg.generateSimpleSeries(31); + const customStyle = { + tickLabelPadding: number('Tick Label Padding', 0), + }; + return ( - + diff --git a/stories/styling.tsx b/stories/styling.tsx index b61a7849ff..b928b5188d 100644 --- a/stories/styling.tsx +++ b/stories/styling.tsx @@ -170,7 +170,7 @@ storiesOf('Stylings', module) fontSize: range('tickFontSize', 0, 40, 10, 'Tick Label'), fontFamily: `'Open Sans', Helvetica, Arial, sans-serif`, fontStyle: 'normal', - padding: 0, + padding: number('tickLabelPadding', 1, {}, 'Tick Label'), }, tickLineStyle: { stroke: color('tickLineColor', '#333', 'Tick Line'), @@ -672,4 +672,47 @@ storiesOf('Stylings', module) /> ); + }) + .add('tickLabelPadding both prop and theme', () => { + const theme: PartialTheme = { + axes: { + tickLabelStyle: { + fill: color('tickFill', '#333', 'Tick Label'), + fontSize: range('tickFontSize', 0, 40, 10, 'Tick Label'), + fontFamily: `'Open Sans', Helvetica, Arial, sans-serif`, + fontStyle: 'normal', + padding: number('Tick Label Padding Theme', 1, {}, 'Tick Label'), + }, + }, + }; + const customTheme = mergeWithDefaultTheme(theme, LIGHT_THEME); + const customStyle = { + tickLabelPadding: number('Tick Label Padding Axis Spec', 0), + }; + return ( + + + + Number(d).toFixed(2)} + /> + + + ); });