From f3032fed338504fc3f0f7cbd71c4506dff09555f Mon Sep 17 00:00:00 2001 From: rshen91 Date: Wed, 20 Mar 2019 12:53:43 -0700 Subject: [PATCH 01/18] feat(specs.ts stories/axis.tsx): included tickLabelPadding prop hardcoded in axis.tsx marked optional in specs.ts feat(axis_utils axis axis_utils.test): tickLabelPadding in axis_utils --- src/lib/axes/axis_utils.test.ts | 10 ++++++++++ src/lib/axes/axis_utils.ts | 14 ++++++++++++-- src/lib/series/specs.ts | 2 ++ stories/axis.tsx | 4 ++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index 38bb9c7220..cdbba0e6c0 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -482,6 +482,7 @@ describe('Axis computational utils', () => { const tickSize = 10; const tickPadding = 5; const tickPosition = 0; + const tickLabelPadding = 10; let axisPosition = Position.Left; const unrotatedLabelProps = getTickLabelProps( @@ -491,6 +492,7 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, + tickLabelPadding, ); expect(unrotatedLabelProps).toEqual({ @@ -508,6 +510,7 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, + tickLabelPadding, ); expect(rotatedLabelProps).toEqual({ @@ -525,6 +528,7 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, + tickLabelPadding, ); expect(rightRotatedLabelProps).toEqual({ @@ -542,6 +546,7 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, + tickLabelPadding, ); expect(rightUnrotatedLabelProps).toEqual({ @@ -558,6 +563,7 @@ describe('Axis computational utils', () => { const tickPadding = 5; const tickPosition = 0; let axisPosition = Position.Top; + const tickLabelPadding = 10; const unrotatedLabelProps = getTickLabelProps( tickLabelRotation, @@ -566,6 +572,7 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, + tickLabelPadding, ); expect(unrotatedLabelProps).toEqual({ @@ -583,6 +590,7 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, + tickLabelPadding, ); expect(rotatedLabelProps).toEqual({ @@ -600,6 +608,7 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, + tickLabelPadding, ); expect(bottomRotatedLabelProps).toEqual({ @@ -617,6 +626,7 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, + tickLabelPadding, ); expect(bottomUnrotatedLabelProps).toEqual({ diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index a8485bf1aa..c985a48e7b 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -35,6 +35,7 @@ export interface AxisTicksDimensions { maxLabelBboxHeight: number; maxLabelTextWidth: number; maxLabelTextHeight: number; + // tickLabelPadding?: number; } export interface TickLabelProps { @@ -47,7 +48,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 @@ -78,6 +79,7 @@ export function computeAxisTicksDimensions( 1, barsPadding, ); + if (!scale) { throw new Error(`Cannot compute scale for axis spec ${axisSpec.id}`); } @@ -262,10 +264,12 @@ export function getTickLabelProps( tickPosition: number, axisPosition: Position, axisTicksDimensions: AxisTicksDimensions, + tickLabelPadding?: number, ): TickLabelProps { const { maxLabelBboxWidth, maxLabelBboxHeight } = axisTicksDimensions; const isVerticalAxis = isVertical(axisPosition); const isRotated = tickLabelRotation !== 0; + let desiredTickPadding; let align = 'center'; let verticalAlign = 'middle'; @@ -275,9 +279,15 @@ export function getTickLabelProps( if (!isRotated) { align = isAxisLeft ? 'right' : 'left'; } + // console.log('$$$$$$$$$ tickPadding', tickPadding); + if (tickPadding !== tickLabelPadding) { + desiredTickPadding = tickLabelPadding; + } + desiredTickPadding = tickPadding; return { - x: isAxisLeft ? -maxLabelBboxWidth : tickSize + tickPadding, + x: isAxisLeft ? -maxLabelBboxWidth : tickSize + desiredTickPadding, + // x: isAxisLeft ? -maxLabelBboxWidth : tickSize + tickPadding, y: tickPosition - maxLabelBboxHeight / 2, align, verticalAlign, diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index 3c21626a31..3903483b69 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -176,6 +176,8 @@ export interface AxisSpec { title?: string; /** If specified, it constrains the domain for these values */ domain?: DomainRange; + /** Handles the user supplied padding for axis debug */ + tickLabelPadding?: number; } export type TickFormatter = (value: any) => string; diff --git a/stories/axis.tsx b/stories/axis.tsx index f1bed603cd..280bf38d3a 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -95,6 +95,7 @@ storiesOf('Axis', module) max: 90, step: 1, })} + tickLabelPadding={200} /> Number(d).toFixed(2)} + tickLabelPadding={200} /> Number(d).toFixed(2)} + tickLabelPadding ={200} /> Number(d).toFixed(2)} + tickLabelPadding={200} /> Date: Fri, 22 Mar 2019 13:52:54 -0700 Subject: [PATCH 02/18] feat(axis.tsx axis_utils.ts): tickLabelPadding extend width of Text Axis refactor(added testUserInput number in knob): wip broken build --- src/components/react_canvas/axis.tsx | 3 ++- src/lib/axes/axis_utils.ts | 21 ++++++--------------- src/lib/axes/canvas_text_bbox_calculator.ts | 5 +++++ src/specs/axis.tsx | 1 + src/specs/settings.tsx | 1 + src/state/chart_state.ts | 1 + stories/axis.tsx | 5 +++-- 7 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/components/react_canvas/axis.tsx b/src/components/react_canvas/axis.tsx index c912b1d29e..605cda13c9 100644 --- a/src/components/react_canvas/axis.tsx +++ b/src/components/react_canvas/axis.tsx @@ -48,6 +48,7 @@ export class Axis extends React.PureComponent { ); const { maxLabelTextWidth, maxLabelTextHeight } = axisTicksDimensions; + const centeredRectProps = centerRotationOrigin(axisTicksDimensions, { x: tickLabelProps.x, y: tickLabelProps.y, @@ -64,7 +65,7 @@ export class Axis extends React.PureComponent { return ( {debug && } - + ); } diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index c985a48e7b..23093d82de 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -35,7 +35,6 @@ export interface AxisTicksDimensions { maxLabelBboxHeight: number; maxLabelTextWidth: number; maxLabelTextHeight: number; - // tickLabelPadding?: number; } export interface TickLabelProps { @@ -174,14 +173,13 @@ 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, - maxLabelTextWidth: prevLabelWidth > labelWidth ? prevLabelWidth : labelWidth, - maxLabelTextHeight: prevLabelHeight > labelHeight ? prevLabelHeight : labelHeight, + maxLabelBboxWidth: prevWidth > width ? prevWidth : width, + maxLabelBboxHeight: prevHeight > height ? prevHeight : height, + maxLabelTextWidth: prevLabelWidth > labelWidth ? prevLabelWidth : labelWidth, + maxLabelTextHeight: prevLabelHeight > labelHeight ? prevLabelHeight : labelHeight, + }; }; -}; function computeTickDimensions( scale: Scale, @@ -269,7 +267,6 @@ export function getTickLabelProps( const { maxLabelBboxWidth, maxLabelBboxHeight } = axisTicksDimensions; const isVerticalAxis = isVertical(axisPosition); const isRotated = tickLabelRotation !== 0; - let desiredTickPadding; let align = 'center'; let verticalAlign = 'middle'; @@ -279,15 +276,9 @@ export function getTickLabelProps( if (!isRotated) { align = isAxisLeft ? 'right' : 'left'; } - // console.log('$$$$$$$$$ tickPadding', tickPadding); - if (tickPadding !== tickLabelPadding) { - desiredTickPadding = tickLabelPadding; - } - desiredTickPadding = tickPadding; return { - x: isAxisLeft ? -maxLabelBboxWidth : tickSize + desiredTickPadding, - // x: isAxisLeft ? -maxLabelBboxWidth : tickSize + tickPadding, + x: isAxisLeft ? -maxLabelBboxWidth : tickSize + tickPadding, y: tickPosition - maxLabelBboxHeight / 2, align, verticalAlign, diff --git a/src/lib/axes/canvas_text_bbox_calculator.ts b/src/lib/axes/canvas_text_bbox_calculator.ts index f3143cf9a8..a4651a8793 100644 --- a/src/lib/axes/canvas_text_bbox_calculator.ts +++ b/src/lib/axes/canvas_text_bbox_calculator.ts @@ -6,6 +6,7 @@ export class CanvasTextBBoxCalculator implements BBoxCalculator { private attachedRoot: HTMLElement; private offscreenCanvas: HTMLCanvasElement; private scaledFontSize: number; + private testUserInputPadding!: number; constructor(rootElement?: HTMLElement, scaledFontSize: number = 100) { this.offscreenCanvas = document.createElement('canvas'); @@ -28,6 +29,10 @@ export class CanvasTextBBoxCalculator implements BBoxCalculator { this.context.font = `${this.scaledFontSize}px ${fontFamily}`; const measure = this.context.measureText(text); + if (this.testUserInputPadding) { + padding = this.testUserInputPadding; + } + return some({ width: measure.width / scalingFactor + padding, height: fontSize, diff --git a/src/specs/axis.tsx b/src/specs/axis.tsx index e0aae05dc9..cad2fd172d 100644 --- a/src/specs/axis.tsx +++ b/src/specs/axis.tsx @@ -17,6 +17,7 @@ class AxisSpec extends PureComponent { tickPadding: 10, tickFormat: (tick: any) => `${tick}`, tickLabelRotation: 0, + tickLabelPadding: 10, }; componentDidMount() { const { chartStore, children, ...spec } = this.props; diff --git a/src/specs/settings.tsx b/src/specs/settings.tsx index 4ae7d06cc3..bbd3fd8a01 100644 --- a/src/specs/settings.tsx +++ b/src/specs/settings.tsx @@ -28,6 +28,7 @@ interface SettingSpecProps { /** Snap tooltip to grid */ tooltipSnap?: boolean; debug: boolean; + testUserInput: number; legendPosition?: Position; isLegendItemsSortDesc: boolean; showLegendDisplayValue: boolean; diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 4fc4dd97db..8ac7079b4e 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -162,6 +162,7 @@ export class ChartStore { customSeriesColors: Map = new Map(); seriesColorMap: Map = new Map(); totalBarsInCluster?: number; + testUserInput!: number; tooltipData = observable.array([], { deep: false }); tooltipType = observable.box(DEFAULT_TOOLTIP_TYPE); diff --git a/stories/axis.tsx b/stories/axis.tsx index 280bf38d3a..faa87faf61 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -55,8 +55,9 @@ function renderAxisWithOptions(position: Position, seriesGroup: string, show: bo storiesOf('Axis', module) .add('basic', () => { return ( - - + + Date: Wed, 17 Apr 2019 15:22:52 -0600 Subject: [PATCH 03/18] feat(axis): add tickLabelPadding prop docs(axis): add storybook knob to tickLabelPadding prop feat(axis): add bottom axis tick label padding prop feat(rotations): add tickLabelPadding prop feat(bar_chart): add tickLabelPadding prop --- src/lib/axes/axis_utils.test.ts | 2 +- src/lib/axes/axis_utils.ts | 8 ++++++-- src/lib/axes/bbox_calculator.ts | 2 +- src/lib/axes/canvas_text_bbox_calculator.ts | 5 ----- stories/axis.tsx | 11 +++++------ stories/bar_chart.tsx | 2 ++ stories/rotations.tsx | 4 +++- 7 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index cdbba0e6c0..bbaa070bdc 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -430,7 +430,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, diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 23093d82de..d90f1f8dd1 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -88,6 +88,7 @@ export function computeAxisTicksDimensions( bboxCalculator, axisConfig, axisSpec.tickLabelRotation, + axisSpec.tickLabelPadding, ); return { @@ -148,6 +149,7 @@ export const getMaxBboxDimensions = ( fontSize: number, fontFamily: string, tickLabelRotation: number, + tickLabelPadding: number, ) => ( acc: { [key: string]: number }, tickLabel: string, @@ -157,7 +159,8 @@ export const getMaxBboxDimensions = ( maxLabelTextWidth: number; maxLabelTextHeight: number; } => { - const bbox = bboxCalculator.compute(tickLabel, fontSize, fontFamily).getOrElse({ + + const bbox = bboxCalculator.compute(tickLabel, fontSize, fontFamily, tickLabelPadding).getOrElse({ width: 0, height: 0, }); @@ -187,6 +190,7 @@ function computeTickDimensions( bboxCalculator: BBoxCalculator, axisConfig: AxisConfig, tickLabelRotation: number = 0, + tickLabelPadding: number = 1, ) { const tickValues = scale.ticks(); const tickLabels = tickValues.map(tickFormat); @@ -201,7 +205,7 @@ function computeTickDimensions( 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..7dbe2c7992 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, fontSize?: number, fontFamily?: string, padding?: number ): Option; destroy(): void; } diff --git a/src/lib/axes/canvas_text_bbox_calculator.ts b/src/lib/axes/canvas_text_bbox_calculator.ts index a4651a8793..f3143cf9a8 100644 --- a/src/lib/axes/canvas_text_bbox_calculator.ts +++ b/src/lib/axes/canvas_text_bbox_calculator.ts @@ -6,7 +6,6 @@ export class CanvasTextBBoxCalculator implements BBoxCalculator { private attachedRoot: HTMLElement; private offscreenCanvas: HTMLCanvasElement; private scaledFontSize: number; - private testUserInputPadding!: number; constructor(rootElement?: HTMLElement, scaledFontSize: number = 100) { this.offscreenCanvas = document.createElement('canvas'); @@ -29,10 +28,6 @@ export class CanvasTextBBoxCalculator implements BBoxCalculator { this.context.font = `${this.scaledFontSize}px ${fontFamily}`; const measure = this.context.measureText(text); - if (this.testUserInputPadding) { - padding = this.testUserInputPadding; - } - return some({ width: measure.width / scalingFactor + padding, height: fontSize, diff --git a/stories/axis.tsx b/stories/axis.tsx index faa87faf61..578614357a 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -54,15 +54,16 @@ function renderAxisWithOptions(position: Position, seriesGroup: string, show: bo storiesOf('Axis', module) .add('basic', () => { + const tickLabelPadding = number('Tick Label Padding', 0); return ( - + { + const tickLabelPadding = number('Bottom Axis Tick Label Padding', 0); return ( Number(d).toFixed(2)} - tickLabelPadding={200} /> Number(d).toFixed(2)} - tickLabelPadding ={200} /> Number(d).toFixed(2)} - tickLabelPadding={200} /> { const formatter = timeFormatter(niceTimeFormatByDay(1)); + const tickLabelPadding = number('Tick Label Padding', 0); return ( @@ -312,6 +313,7 @@ storiesOf('Bar Chart', module) showOverlappingTicks={boolean('showOverlappingTicks bottom axis', false)} showOverlappingLabels={boolean('showOverlappingLabels bottom axis', false)} tickFormat={formatter} + tickLabelPadding={tickLabelPadding} /> { + const tickLabelPadding = number('Tick Label Padding', 0); return ( Date: Tue, 23 Apr 2019 16:36:09 -0600 Subject: [PATCH 04/18] feat(styling and axis): supress canvas padding manipulation feat(axis_utils): specify tickLabelPadding from axisSpec or axisConfig feat(bbox_calc axis_utils): remove optional padding for tickLabelPadding --- src/components/react_canvas/axis.tsx | 13 ++++++++++--- src/lib/axes/axis_utils.ts | 5 +++-- src/lib/axes/bbox_calculator.ts | 2 +- src/lib/axes/canvas_text_bbox_calculator.ts | 2 +- src/specs/settings.tsx | 1 - stories/styling.tsx | 2 +- 6 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/components/react_canvas/axis.tsx b/src/components/react_canvas/axis.tsx index 605cda13c9..e3fe80db2d 100644 --- a/src/components/react_canvas/axis.tsx +++ b/src/components/react_canvas/axis.tsx @@ -29,7 +29,12 @@ export class Axis extends React.PureComponent { return this.renderAxis(); } renderTickLabel = (tick: AxisTick, i: number) => { - const { padding, ...labelStyle } = this.props.chartTheme.axes.tickLabelStyle; + // suppress padding render from canvas + const labelStyle = { + ...this.props.chartTheme.axes.tickLabelStyle, + padding: 0, + }; + const { axisSpec: { tickSize, tickPadding, position }, axisTicksDimensions, @@ -37,6 +42,7 @@ export class Axis extends React.PureComponent { } = this.props; const tickLabelRotation = this.props.axisSpec.tickLabelRotation || 0; + const tickLabelPadding = this.props.axisSpec.tickLabelPadding || this.props.chartTheme.axes.tickLabelStyle.padding; const tickLabelProps = getTickLabelProps( tickLabelRotation, @@ -45,6 +51,7 @@ export class Axis extends React.PureComponent { tick.position, position, axisTicksDimensions, + tickLabelPadding, ); const { maxLabelTextWidth, maxLabelTextHeight } = axisTicksDimensions; @@ -64,8 +71,8 @@ export class Axis extends React.PureComponent { return ( - {debug && } - + {debug && } + ); } diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index d90f1f8dd1..ca3547b82b 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -82,13 +82,14 @@ export function computeAxisTicksDimensions( if (!scale) { throw new Error(`Cannot compute scale for axis spec ${axisSpec.id}`); } + const tickLabelPadding = axisSpec.tickLabelPadding || axisConfig.tickLabelStyle.padding; const dimensions = computeTickDimensions( scale, axisSpec.tickFormat, bboxCalculator, axisConfig, axisSpec.tickLabelRotation, - axisSpec.tickLabelPadding, + tickLabelPadding, ); return { @@ -160,7 +161,7 @@ export const getMaxBboxDimensions = ( maxLabelTextHeight: number; } => { - const bbox = bboxCalculator.compute(tickLabel, fontSize, fontFamily, tickLabelPadding).getOrElse({ + const bbox = bboxCalculator.compute(tickLabel, fontSize, tickLabelPadding, fontFamily).getOrElse({ width: 0, height: 0, }); diff --git a/src/lib/axes/bbox_calculator.ts b/src/lib/axes/bbox_calculator.ts index 7dbe2c7992..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, padding?: number ): Option; + compute(text: string, padding: number, fontSize?: number, fontFamily?: string): Option; destroy(): void; } diff --git a/src/lib/axes/canvas_text_bbox_calculator.ts b/src/lib/axes/canvas_text_bbox_calculator.ts index f3143cf9a8..7bc98b26d8 100644 --- a/src/lib/axes/canvas_text_bbox_calculator.ts +++ b/src/lib/axes/canvas_text_bbox_calculator.ts @@ -17,7 +17,7 @@ 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 = 1, fontSize = 16, fontFamily = 'Arial'): Option { if (!this.context) { return none; } diff --git a/src/specs/settings.tsx b/src/specs/settings.tsx index bbd3fd8a01..4ae7d06cc3 100644 --- a/src/specs/settings.tsx +++ b/src/specs/settings.tsx @@ -28,7 +28,6 @@ interface SettingSpecProps { /** Snap tooltip to grid */ tooltipSnap?: boolean; debug: boolean; - testUserInput: number; legendPosition?: Position; isLegendItemsSortDesc: boolean; showLegendDisplayValue: boolean; diff --git a/stories/styling.tsx b/stories/styling.tsx index af528f9dc1..4b792afb88 100644 --- a/stories/styling.tsx +++ b/stories/styling.tsx @@ -172,7 +172,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'), From ab3869ac794cba1ad63264c9eb75c284939faeb3 Mon Sep 17 00:00:00 2001 From: rshen91 Date: Thu, 25 Apr 2019 08:15:17 -0600 Subject: [PATCH 05/18] feat(axis_utils canvas_bbox): add theme tickLabelPadding refactor(axis_utils rendering): change parameter order and whitespace docs(chart_state): remove testUserInput test prop --- src/components/react_canvas/axis.tsx | 2 -- src/lib/axes/axis_utils.test.ts | 10 ---------- src/lib/axes/axis_utils.ts | 9 +++++---- src/lib/axes/canvas_text_bbox_calculator.test.ts | 14 +++++++------- src/lib/axes/canvas_text_bbox_calculator.ts | 2 +- src/lib/series/rendering.test.ts | 1 - src/lib/series/rendering.ts | 6 +++++- src/lib/themes/light_theme.ts | 2 +- src/specs/axis.tsx | 1 - src/state/chart_state.ts | 1 - 10 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/components/react_canvas/axis.tsx b/src/components/react_canvas/axis.tsx index e3fe80db2d..0ff6ed8b63 100644 --- a/src/components/react_canvas/axis.tsx +++ b/src/components/react_canvas/axis.tsx @@ -42,7 +42,6 @@ export class Axis extends React.PureComponent { } = this.props; const tickLabelRotation = this.props.axisSpec.tickLabelRotation || 0; - const tickLabelPadding = this.props.axisSpec.tickLabelPadding || this.props.chartTheme.axes.tickLabelStyle.padding; const tickLabelProps = getTickLabelProps( tickLabelRotation, @@ -51,7 +50,6 @@ export class Axis extends React.PureComponent { tick.position, position, axisTicksDimensions, - tickLabelPadding, ); const { maxLabelTextWidth, maxLabelTextHeight } = axisTicksDimensions; diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index bbaa070bdc..a83a8ee6b3 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -482,7 +482,6 @@ describe('Axis computational utils', () => { const tickSize = 10; const tickPadding = 5; const tickPosition = 0; - const tickLabelPadding = 10; let axisPosition = Position.Left; const unrotatedLabelProps = getTickLabelProps( @@ -492,7 +491,6 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, - tickLabelPadding, ); expect(unrotatedLabelProps).toEqual({ @@ -510,7 +508,6 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, - tickLabelPadding, ); expect(rotatedLabelProps).toEqual({ @@ -528,7 +525,6 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, - tickLabelPadding, ); expect(rightRotatedLabelProps).toEqual({ @@ -546,7 +542,6 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, - tickLabelPadding, ); expect(rightUnrotatedLabelProps).toEqual({ @@ -563,7 +558,6 @@ describe('Axis computational utils', () => { const tickPadding = 5; const tickPosition = 0; let axisPosition = Position.Top; - const tickLabelPadding = 10; const unrotatedLabelProps = getTickLabelProps( tickLabelRotation, @@ -572,7 +566,6 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, - tickLabelPadding, ); expect(unrotatedLabelProps).toEqual({ @@ -590,7 +583,6 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, - tickLabelPadding, ); expect(rotatedLabelProps).toEqual({ @@ -608,7 +600,6 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, - tickLabelPadding, ); expect(bottomRotatedLabelProps).toEqual({ @@ -626,7 +617,6 @@ describe('Axis computational utils', () => { tickPosition, axisPosition, axis1Dims, - tickLabelPadding, ); expect(bottomUnrotatedLabelProps).toEqual({ diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index ca3547b82b..a4b99aa2dd 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -82,14 +82,16 @@ export function computeAxisTicksDimensions( if (!scale) { throw new Error(`Cannot compute scale for axis spec ${axisSpec.id}`); } + const tickLabelPadding = axisSpec.tickLabelPadding || axisConfig.tickLabelStyle.padding; + const dimensions = computeTickDimensions( scale, axisSpec.tickFormat, bboxCalculator, axisConfig, - axisSpec.tickLabelRotation, tickLabelPadding, + axisSpec.tickLabelRotation, ); return { @@ -161,7 +163,7 @@ export const getMaxBboxDimensions = ( maxLabelTextHeight: number; } => { - const bbox = bboxCalculator.compute(tickLabel, fontSize, tickLabelPadding, fontFamily).getOrElse({ + const bbox = bboxCalculator.compute(tickLabel, tickLabelPadding, fontSize, fontFamily).getOrElse({ width: 0, height: 0, }); @@ -190,8 +192,8 @@ function computeTickDimensions( tickFormat: TickFormatter, bboxCalculator: BBoxCalculator, axisConfig: AxisConfig, + tickLabelPadding: number, tickLabelRotation: number = 0, - tickLabelPadding: number = 1, ) { const tickValues = scale.ticks(); const tickLabels = tickValues.map(tickFormat); @@ -267,7 +269,6 @@ export function getTickLabelProps( tickPosition: number, axisPosition: Position, axisTicksDimensions: AxisTicksDimensions, - tickLabelPadding?: number, ): TickLabelProps { const { maxLabelBboxWidth, maxLabelBboxHeight } = axisTicksDimensions; const isVerticalAxis = isVertical(axisPosition); 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 7bc98b26d8..f42c29ae20 100644 --- a/src/lib/axes/canvas_text_bbox_calculator.ts +++ b/src/lib/axes/canvas_text_bbox_calculator.ts @@ -17,7 +17,7 @@ export class CanvasTextBBoxCalculator implements BBoxCalculator { this.attachedRoot.appendChild(this.offscreenCanvas); this.scaledFontSize = scaledFontSize; } - compute(text: string, padding: number = 1, fontSize = 16, fontFamily = 'Arial'): Option { + compute(text: string, padding: number, fontSize = 16, fontFamily = 'Arial'): Option { if (!this.context) { return none; } diff --git a/src/lib/series/rendering.test.ts b/src/lib/series/rendering.test.ts index d3f140b143..5687c2fc01 100644 --- a/src/lib/series/rendering.test.ts +++ b/src/lib/series/rendering.test.ts @@ -99,7 +99,6 @@ describe('Rendering utils', () => { // no highlighted elements expect(defaultStyle).toEqual({ opacity: 1 }); - const customDefaultStyle = getGeometryStyle( geometryId, null, diff --git a/src/lib/series/rendering.ts b/src/lib/series/rendering.ts index ac27cfe78a..a94f7dbe5b 100644 --- a/src/lib/series/rendering.ts +++ b/src/lib/series/rendering.ts @@ -188,12 +188,14 @@ export function renderBars( barGeometries: BarGeometry[]; indexedGeometries: Map; } { + const indexedGeometries: Map = new Map(); const xDomain = xScale.domain; const xScaleType = xScale.type; const barGeometries: BarGeometry[] = []; const bboxCalculator = new CanvasTextBBoxCalculator(); + const padding = 1; const fontSize = seriesStyle && seriesStyle.displayValue ? seriesStyle.displayValue.fontSize : undefined; const fontFamily = seriesStyle && seriesStyle.displayValue ? seriesStyle.displayValue.fontFamily : undefined; @@ -234,7 +236,8 @@ export function renderBars( (barGeometries.length % 2 === 0 ? formattedDisplayValue : undefined) : formattedDisplayValue; - const computedDisplayValueWidth = bboxCalculator.compute(displayValueText || '', fontSize, fontFamily).getOrElse({ + const computedDisplayValueWidth = bboxCalculator.compute(displayValueText || '', padding, fontSize, fontFamily) + .getOrElse({ width: 0, height: 0, }).width; @@ -298,6 +301,7 @@ export function renderLine( lineGeometry: LineGeometry; indexedGeometries: Map; } { + const isLogScale = isLogarithmicScale(yScale); const pathGenerator = line() diff --git a/src/lib/themes/light_theme.ts b/src/lib/themes/light_theme.ts index c42baeafe5..adeafdae0b 100644 --- a/src/lib/themes/light_theme.ts +++ b/src/lib/themes/light_theme.ts @@ -91,7 +91,7 @@ export const LIGHT_THEME: Theme = { fontFamily: `'Open Sans', Helvetica, Arial, sans-serif`, fontStyle: 'normal', fill: 'gray', - padding: 0, + padding: 1, }, tickLineStyle: { stroke: 'gray', diff --git a/src/specs/axis.tsx b/src/specs/axis.tsx index cad2fd172d..e0aae05dc9 100644 --- a/src/specs/axis.tsx +++ b/src/specs/axis.tsx @@ -17,7 +17,6 @@ class AxisSpec extends PureComponent { tickPadding: 10, tickFormat: (tick: any) => `${tick}`, tickLabelRotation: 0, - tickLabelPadding: 10, }; componentDidMount() { const { chartStore, children, ...spec } = this.props; diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 8ac7079b4e..4fc4dd97db 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -162,7 +162,6 @@ export class ChartStore { customSeriesColors: Map = new Map(); seriesColorMap: Map = new Map(); totalBarsInCluster?: number; - testUserInput!: number; tooltipData = observable.array([], { deep: false }); tooltipType = observable.box(DEFAULT_TOOLTIP_TYPE); From 9bfa9b144b2e71b2c84d6fba9f7bc693e272f18c Mon Sep 17 00:00:00 2001 From: rshen91 Date: Mon, 20 May 2019 09:14:59 -0400 Subject: [PATCH 06/18] style(rendering.ts and test): define padding as default to 1 --- src/lib/series/rendering.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/series/rendering.ts b/src/lib/series/rendering.ts index a94f7dbe5b..e1d4332c72 100644 --- a/src/lib/series/rendering.ts +++ b/src/lib/series/rendering.ts @@ -195,6 +195,7 @@ 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; From 0a3f443b777d96c6ef7e4f64a7e657e2056f989b Mon Sep 17 00:00:00 2001 From: rshen91 Date: Tue, 21 May 2019 11:20:58 -0600 Subject: [PATCH 07/18] style(axis.tsx): improve comment line 32 style(stories/axis.tsx): remove explicit renderer for axis basic refactor(bar_chart rotations stories): remove tickLabelPadding --- src/components/react_canvas/axis.tsx | 2 +- stories/axis.tsx | 2 +- stories/bar_chart.tsx | 2 -- stories/rotations.tsx | 4 +--- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/components/react_canvas/axis.tsx b/src/components/react_canvas/axis.tsx index 0ff6ed8b63..799cb0eb60 100644 --- a/src/components/react_canvas/axis.tsx +++ b/src/components/react_canvas/axis.tsx @@ -29,7 +29,7 @@ export class Axis extends React.PureComponent { return this.renderAxis(); } renderTickLabel = (tick: AxisTick, i: number) => { - // suppress padding render from canvas + // suppress padding render from canvas to avoid conflict with tickLabelPadding const labelStyle = { ...this.props.chartTheme.axes.tickLabelStyle, padding: 0, diff --git a/stories/axis.tsx b/stories/axis.tsx index 578614357a..58149e5a4f 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -56,7 +56,7 @@ storiesOf('Axis', module) .add('basic', () => { const tickLabelPadding = number('Tick Label Padding', 0); return ( - + { const formatter = timeFormatter(niceTimeFormatByDay(1)); - const tickLabelPadding = number('Tick Label Padding', 0); return ( @@ -313,7 +312,6 @@ storiesOf('Bar Chart', module) showOverlappingTicks={boolean('showOverlappingTicks bottom axis', false)} showOverlappingLabels={boolean('showOverlappingLabels bottom axis', false)} tickFormat={formatter} - tickLabelPadding={tickLabelPadding} /> { - const tickLabelPadding = number('Tick Label Padding', 0); return ( Date: Tue, 21 May 2019 13:08:50 -0600 Subject: [PATCH 08/18] refactor(stories/axis): add tickLabelPadding to tick label rotation axes feat(specs.ts stories/axis.tsx): included tickLabelPadding prop hardcoded in axis.tsx marked optional in specs.ts style(react_canvas/axis): improve comment for padding 0 --- src/components/react_canvas/axis.tsx | 2 +- stories/axis.tsx | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/components/react_canvas/axis.tsx b/src/components/react_canvas/axis.tsx index 799cb0eb60..6a49239ced 100644 --- a/src/components/react_canvas/axis.tsx +++ b/src/components/react_canvas/axis.tsx @@ -29,7 +29,7 @@ export class Axis extends React.PureComponent { return this.renderAxis(); } renderTickLabel = (tick: AxisTick, i: number) => { - // suppress padding render from canvas to avoid conflict with tickLabelPadding +// padding is already being computed through bbox_calculator using tickLabelPadding, set padding to 0 to avoid conflict const labelStyle = { ...this.props.chartTheme.axes.tickLabelStyle, padding: 0, diff --git a/stories/axis.tsx b/stories/axis.tsx index 58149e5a4f..0817fda1c5 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -70,6 +70,7 @@ storiesOf('Axis', module) title={'Left axis'} position={Position.Left} tickFormat={(d) => Number(d).toFixed(2)} + tickLabelPadding={tickLabelPadding} /> { - const tickLabelPadding = number('Bottom Axis Tick Label Padding', 0); + const tickLabelPadding = number('Axis Tick Label Padding', 0); return ( Number(d).toFixed(2)} + tickLabelPadding={tickLabelPadding} /> Number(d).toFixed(2)} + tickLabelPadding={tickLabelPadding} /> Number(d).toFixed(2)} + tickLabelPadding={tickLabelPadding} /> From 2dfb61450347d5c66af160e43324f59ba90622d2 Mon Sep 17 00:00:00 2001 From: rshen91 Date: Wed, 22 May 2019 12:43:10 -0600 Subject: [PATCH 09/18] feat(axis styling.tsx): add tickLabelPadding story theme and prop --- stories/axis.tsx | 2 ++ stories/styling.tsx | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/stories/axis.tsx b/stories/axis.tsx index 0817fda1c5..a326ea6b07 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -333,6 +333,7 @@ storiesOf('Axis', module) .add('w many tick labels', () => { const dg = new DataGenerator(); const data = dg.generateSimpleSeries(31); + const tickLabelPadding = number('Axis Tick Label Padding', 0); return ( @@ -341,6 +342,7 @@ storiesOf('Axis', module) position={Position.Bottom} title={'Bottom axis'} showOverlappingTicks={true} + tickLabelPadding={tickLabelPadding} /> ); + }) + .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 tickLabelPadding = number('Tick Label Padding Prop', 0); + return ( + + + + Number(d).toFixed(2)} + tickLabelPadding={tickLabelPadding} + /> + + + ); }); From 929b0acaf1396c4c2443d21cb559e05d4c228610 Mon Sep 17 00:00:00 2001 From: rshen91 Date: Thu, 23 May 2019 08:33:15 -0600 Subject: [PATCH 10/18] style(axis.ts rendering.ts): min to -90 readability and comments --- src/components/react_canvas/axis.tsx | 6 +++++- src/lib/series/rendering.ts | 6 +++--- stories/axis.tsx | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/components/react_canvas/axis.tsx b/src/components/react_canvas/axis.tsx index 6a49239ced..2de8568823 100644 --- a/src/components/react_canvas/axis.tsx +++ b/src/components/react_canvas/axis.tsx @@ -29,7 +29,11 @@ export class Axis extends React.PureComponent { return this.renderAxis(); } renderTickLabel = (tick: AxisTick, i: number) => { -// padding is already being computed through bbox_calculator using tickLabelPadding, set padding to 0 to avoid conflict + /** + * 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, diff --git a/src/lib/series/rendering.ts b/src/lib/series/rendering.ts index e1d4332c72..2ff18cab73 100644 --- a/src/lib/series/rendering.ts +++ b/src/lib/series/rendering.ts @@ -239,9 +239,9 @@ export function renderBars( const computedDisplayValueWidth = bboxCalculator.compute(displayValueText || '', padding, fontSize, fontFamily) .getOrElse({ - width: 0, - height: 0, - }).width; + width: 0, + height: 0, + }).width; const displayValueWidth = displayValueSettings && displayValueSettings.isValueContainedInElement ? width : computedDisplayValueWidth; diff --git a/stories/axis.tsx b/stories/axis.tsx index a326ea6b07..ca9de40259 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -95,7 +95,7 @@ storiesOf('Axis', module) showOverlappingTicks={true} tickLabelRotation={number('bottom axis tick label rotation', 0, { range: true, - min: -900, + min: -90, max: 90, step: 1, })} From 0f39133b34692f1f9d4db539d584901da987746c Mon Sep 17 00:00:00 2001 From: rshen91 Date: Thu, 23 May 2019 09:11:50 -0600 Subject: [PATCH 11/18] refactor(styling): isolate spec and theme in tickLabelPadding story style(specs.ts): improve comment for tickLabelPadding spec --- src/lib/series/specs.ts | 2 +- stories/styling.tsx | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index 3903483b69..99b4175903 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -176,7 +176,7 @@ export interface AxisSpec { title?: string; /** If specified, it constrains the domain for these values */ domain?: DomainRange; - /** Handles the user supplied padding for axis debug */ + /** Specifies the amount of padding on the tick label bounding box */ tickLabelPadding?: number; } diff --git a/stories/styling.tsx b/stories/styling.tsx index d061be42aa..446c7167e5 100644 --- a/stories/styling.tsx +++ b/stories/styling.tsx @@ -675,7 +675,7 @@ storiesOf('Stylings', module) }, }; const customTheme = mergeWithDefaultTheme(theme, LIGHT_THEME); - const tickLabelPadding = number('Tick Label Padding Prop', 0); + const tickLabelPadding = number('Tick Label Padding Axis Spec', 0); return ( Number(d).toFixed(2)} - tickLabelPadding={tickLabelPadding} /> Date: Tue, 4 Jun 2019 08:49:03 -0600 Subject: [PATCH 12/18] refactor(canvas_bbox dark_theme): add validation for padding --- src/lib/axes/axis_utils.ts | 8 ++++---- src/lib/axes/canvas_text_bbox_calculator.ts | 5 +++++ src/lib/themes/dark_theme.ts | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index a4b99aa2dd..e35c3bf881 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -180,10 +180,10 @@ export const getMaxBboxDimensions = ( const prevLabelWidth = acc.maxLabelTextWidth; const prevLabelHeight = acc.maxLabelTextHeight; return { - maxLabelBboxWidth: prevWidth > width ? prevWidth : width, - maxLabelBboxHeight: prevHeight > height ? prevHeight : height, - maxLabelTextWidth: prevLabelWidth > labelWidth ? prevLabelWidth : labelWidth, - maxLabelTextHeight: prevLabelHeight > labelHeight ? prevLabelHeight : labelHeight, + maxLabelBboxWidth: prevWidth > width ? prevWidth : width, + maxLabelBboxHeight: prevHeight > height ? prevHeight : height, + maxLabelTextWidth: prevLabelWidth > labelWidth ? prevLabelWidth : labelWidth, + maxLabelTextHeight: prevLabelHeight > labelHeight ? prevLabelHeight : labelHeight, }; }; diff --git a/src/lib/axes/canvas_text_bbox_calculator.ts b/src/lib/axes/canvas_text_bbox_calculator.ts index f42c29ae20..4f6b51306c 100644 --- a/src/lib/axes/canvas_text_bbox_calculator.ts +++ b/src/lib/axes/canvas_text_bbox_calculator.ts @@ -22,6 +22,11 @@ export class CanvasTextBBoxCalculator implements BBoxCalculator { return none; } + // Avoid having negative padding that can obscure text + 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/themes/dark_theme.ts b/src/lib/themes/dark_theme.ts index dc317dda5a..0e2ea69140 100644 --- a/src/lib/themes/dark_theme.ts +++ b/src/lib/themes/dark_theme.ts @@ -91,7 +91,7 @@ export const DARK_THEME: Theme = { fontFamily: `'Open Sans', Helvetica, Arial, sans-serif`, fontStyle: 'normal', fill: 'white', - padding: 0, + padding: 1, }, tickLineStyle: { stroke: 'white', From f8e8f5f9a1fedbf90343353ea251b9b8a420be20 Mon Sep 17 00:00:00 2001 From: rshen91 Date: Mon, 24 Jun 2019 10:31:14 -0600 Subject: [PATCH 13/18] refactor: missed adding merge commits --- src/components/react_canvas/axis.tsx | 12 ++++++------ src/lib/axes/axis_utils.ts | 3 +-- src/lib/series/rendering.ts | 12 ++++++------ stories/axis.tsx | 2 +- stories/styling.tsx | 5 +---- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/components/react_canvas/axis.tsx b/src/components/react_canvas/axis.tsx index bfcd1ff1fb..67ba805a2e 100644 --- a/src/components/react_canvas/axis.tsx +++ b/src/components/react_canvas/axis.tsx @@ -29,11 +29,11 @@ export class Axis extends React.PureComponent { return this.renderAxis(); } renderTickLabel = (tick: AxisTick, i: number) => { - /** - * padding is already computed through width - * and bbox_calculator using tickLabelPadding - * set padding to 0 to avoid conflict - */ + /** + * 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, @@ -73,7 +73,7 @@ export class Axis extends React.PureComponent { return ( - {debug && } + {debug && } ); diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 18f9cdcc22..f7cf67b5f9 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -147,7 +147,6 @@ export const getMaxBboxDimensions = ( maxLabelTextWidth: number; maxLabelTextHeight: number; } => { - const bbox = bboxCalculator.compute(tickLabel, tickLabelPadding, fontSize, fontFamily).getOrElse({ width: 0, height: 0, @@ -169,8 +168,8 @@ export const getMaxBboxDimensions = ( maxLabelBboxHeight: prevHeight > height ? prevHeight : height, maxLabelTextWidth: prevLabelWidth > labelWidth ? prevLabelWidth : labelWidth, maxLabelTextHeight: prevLabelHeight > labelHeight ? prevLabelHeight : labelHeight, - }; }; +}; function computeTickDimensions( scale: Scale, diff --git a/src/lib/series/rendering.ts b/src/lib/series/rendering.ts index 795c72d598..132f5fead4 100644 --- a/src/lib/series/rendering.ts +++ b/src/lib/series/rendering.ts @@ -201,7 +201,6 @@ export function renderBars( barGeometries: BarGeometry[]; indexedGeometries: Map; } { - const indexedGeometries: Map = new Map(); const xDomain = xScale.domain; const xScaleType = xScale.type; @@ -256,10 +255,12 @@ export function renderBars( : undefined : formattedDisplayValue; - const computedDisplayValueWidth = bboxCalculator.compute(displayValueText || '', padding, 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; @@ -322,7 +323,6 @@ export function renderLine( lineGeometry: LineGeometry; indexedGeometries: Map; } { - const isLogScale = isLogarithmicScale(yScale); const pathGenerator = line() diff --git a/stories/axis.tsx b/stories/axis.tsx index a8badc4ebe..336260eeed 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -57,7 +57,7 @@ storiesOf('Axis', module) const tickLabelPadding = number('Tick Label Padding', 0); return ( - + - + Date: Tue, 25 Jun 2019 08:15:18 -0600 Subject: [PATCH 14/18] test(axis_utils.test.ts): add test to confirm positive padding --- src/lib/axes/axis_utils.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index b3302ab1fb..a8959bc928 100644 --- a/src/lib/axes/axis_utils.test.ts +++ b/src/lib/axes/axis_utils.test.ts @@ -1272,4 +1272,15 @@ 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)); + }); }); From 776239aa01ec4e63c83b7ce95a1b5611b8c80b8d Mon Sep 17 00:00:00 2001 From: rshen91 Date: Tue, 2 Jul 2019 12:10:23 -0600 Subject: [PATCH 15/18] refactor(axis_utils specs): create style object --- src/lib/axes/axis_utils.ts | 5 ++++- src/lib/series/specs.ts | 9 +++++++-- stories/axis.tsx | 29 +++++++++++++++++++---------- stories/styling.tsx | 6 ++++-- 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index f7cf67b5f9..e6a4bd5ab1 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -70,7 +70,10 @@ export function computeAxisTicksDimensions( throw new Error(`Cannot compute scale for axis spec ${axisSpec.id}`); } - const tickLabelPadding = axisSpec.tickLabelPadding || axisConfig.tickLabelStyle.padding; + const tickLabelPadding = + axisSpec.style && axisSpec.style.tickLabelPadding + ? axisSpec.style.tickLabelPadding + : axisConfig.tickLabelStyle.padding; const dimensions = computeTickDimensions( scale, diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index 5cbebe084b..8d157159db 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -218,12 +218,17 @@ export interface AxisSpec { title?: string; /** If specified, it constrains the domain for these values */ domain?: DomainRange; - /** Specifies the amount of padding on the tick label bounding box */ - tickLabelPadding?: number; + /** Object to hold custom styling */ + style?: CustomStyle; } export type TickFormatter = (value: any) => string; +interface CustomStyle { + /** 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/stories/axis.tsx b/stories/axis.tsx index 336260eeed..0286749640 100644 --- a/stories/axis.tsx +++ b/stories/axis.tsx @@ -54,7 +54,10 @@ function renderAxisWithOptions(position: Position, seriesGroup: string, show: bo storiesOf('Axis', module) .add('basic', () => { - const tickLabelPadding = number('Tick Label Padding', 0); + const customStyle = { + tickLabelPadding: number('Tick Label Padding', 0), + }; + return ( @@ -63,14 +66,14 @@ storiesOf('Axis', module) position={Position.Bottom} title={'Bottom axis'} showOverlappingTicks={true} - tickLabelPadding={tickLabelPadding} + style={customStyle} /> Number(d).toFixed(2)} - tickLabelPadding={tickLabelPadding} + style={customStyle} /> { - const tickLabelPadding = number('Axis Tick Label Padding', 0); + const customStyle = { + tickLabelPadding: number('Tick Label Padding', 0), + }; + return ( Number(d).toFixed(2)} - tickLabelPadding={tickLabelPadding} + style={customStyle} /> Number(d).toFixed(2)} - tickLabelPadding={tickLabelPadding} + style={customStyle} /> Number(d).toFixed(2)} - tickLabelPadding={tickLabelPadding} + style={customStyle} /> { const dg = new DataGenerator(); const data = dg.generateSimpleSeries(31); - const tickLabelPadding = number('Axis Tick Label Padding', 0); + const customStyle = { + tickLabelPadding: number('Tick Label Padding', 0), + }; + return ( @@ -332,7 +341,7 @@ storiesOf('Axis', module) position={Position.Bottom} title={'Bottom axis'} showOverlappingTicks={true} - tickLabelPadding={tickLabelPadding} + style={customStyle} /> @@ -695,7 +697,7 @@ storiesOf('Stylings', module) position={Position.Bottom} title={'Bottom axis'} showOverlappingTicks={true} - tickLabelPadding={tickLabelPadding} + style={customStyle} /> Date: Wed, 3 Jul 2019 08:57:20 -0600 Subject: [PATCH 16/18] test(axis_utils.test): add test for tickLabelPadding style prop or theme --- src/lib/axes/axis_utils.test.ts | 12 +++++++++++- src/lib/axes/axis_utils.ts | 13 +++++++++---- src/lib/axes/canvas_text_bbox_calculator.ts | 2 +- src/lib/series/specs.ts | 4 ++-- 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/lib/axes/axis_utils.test.ts b/src/lib/axes/axis_utils.test.ts index a8959bc928..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'; @@ -1283,4 +1284,13 @@ describe('Axis computational utils', () => { 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 e6a4bd5ab1..687729503a 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'; @@ -70,10 +71,7 @@ export function computeAxisTicksDimensions( throw new Error(`Cannot compute scale for axis spec ${axisSpec.id}`); } - const tickLabelPadding = - axisSpec.style && axisSpec.style.tickLabelPadding - ? axisSpec.style.tickLabelPadding - : axisConfig.tickLabelStyle.padding; + const tickLabelPadding = getAxisTickLabelPadding(axisConfig.tickLabelStyle.padding, axisSpec.style); const dimensions = computeTickDimensions( scale, @@ -89,6 +87,13 @@ export function computeAxisTicksDimensions( }; } +export function getAxisTickLabelPadding(axisConfigTickLabelPadding: number, axisSpecStyle?: AxisStyle): number { + if (axisSpecStyle && axisSpecStyle.tickLabelPadding) { + return axisSpecStyle.tickLabelPadding; + } + return axisConfigTickLabelPadding; +} + export function isYDomain(position: Position, chartRotation: Rotation): boolean { const isStraightRotation = chartRotation === 0 || chartRotation === 180; if (isVertical(position)) { diff --git a/src/lib/axes/canvas_text_bbox_calculator.ts b/src/lib/axes/canvas_text_bbox_calculator.ts index 4f6b51306c..82308cd654 100644 --- a/src/lib/axes/canvas_text_bbox_calculator.ts +++ b/src/lib/axes/canvas_text_bbox_calculator.ts @@ -22,7 +22,7 @@ export class CanvasTextBBoxCalculator implements BBoxCalculator { return none; } - // Avoid having negative padding that can obscure text + // Padding should be at least one to avoid browser measureText inconsistencies if (padding < 1) { padding = 1; } diff --git a/src/lib/series/specs.ts b/src/lib/series/specs.ts index 8d157159db..ab8599f20e 100644 --- a/src/lib/series/specs.ts +++ b/src/lib/series/specs.ts @@ -219,12 +219,12 @@ export interface AxisSpec { /** If specified, it constrains the domain for these values */ domain?: DomainRange; /** Object to hold custom styling */ - style?: CustomStyle; + style?: AxisStyle; } export type TickFormatter = (value: any) => string; -interface CustomStyle { +export interface AxisStyle { /** Specifies the amount of padding on the tick label bounding box */ tickLabelPadding?: number; } From 83fcc7d7827c0e9c3381a49c3122dc124bd8295e Mon Sep 17 00:00:00 2001 From: rshen91 Date: Wed, 3 Jul 2019 11:09:47 -0600 Subject: [PATCH 17/18] refactor(axis_utils): refactor getAxisTickLabelPadding --- src/lib/axes/axis_utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index 687729503a..b483507159 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -88,10 +88,10 @@ export function computeAxisTicksDimensions( } export function getAxisTickLabelPadding(axisConfigTickLabelPadding: number, axisSpecStyle?: AxisStyle): number { - if (axisSpecStyle && axisSpecStyle.tickLabelPadding) { - return axisSpecStyle.tickLabelPadding; + if (!axisSpecStyle || !axisSpecStyle.tickLabelPadding) { + return axisConfigTickLabelPadding; } - return axisConfigTickLabelPadding; + return axisSpecStyle.tickLabelPadding; } export function isYDomain(position: Position, chartRotation: Rotation): boolean { From 725177fba4a7c203dc447d095cad73707ede7cff Mon Sep 17 00:00:00 2001 From: rshen91 Date: Wed, 3 Jul 2019 12:09:40 -0600 Subject: [PATCH 18/18] refactor(axis_utils): improve if statement --- src/lib/axes/axis_utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/axes/axis_utils.ts b/src/lib/axes/axis_utils.ts index b483507159..114f3d86f4 100644 --- a/src/lib/axes/axis_utils.ts +++ b/src/lib/axes/axis_utils.ts @@ -88,10 +88,10 @@ export function computeAxisTicksDimensions( } export function getAxisTickLabelPadding(axisConfigTickLabelPadding: number, axisSpecStyle?: AxisStyle): number { - if (!axisSpecStyle || !axisSpecStyle.tickLabelPadding) { - return axisConfigTickLabelPadding; + if (axisSpecStyle && axisSpecStyle.tickLabelPadding !== undefined) { + return axisSpecStyle.tickLabelPadding; } - return axisSpecStyle.tickLabelPadding; + return axisConfigTickLabelPadding; } export function isYDomain(position: Position, chartRotation: Rotation): boolean {