From 796671ff51f6474b92e0e07431f218e6eb9af794 Mon Sep 17 00:00:00 2001 From: Jae Sung Park Date: Tue, 24 Oct 2023 17:31:15 +0900 Subject: [PATCH] fix(axis): Fix x axis tick text overlap with legend Compute tick text's height and applies when is greater than the default height value. Fix #3485 --- src/ChartInternal/Axis/Axis.ts | 53 +++++++----- src/ChartInternal/internals/clip.ts | 8 +- src/ChartInternal/internals/size.axis.ts | 16 ++-- src/config/Store/State.ts | 8 +- test/internals/axis-spec.ts | 81 ++++++++++++++++++- .../plugin/stanford/stanford-elements-spec.ts | 2 +- 6 files changed, 132 insertions(+), 36 deletions(-) diff --git a/src/ChartInternal/Axis/Axis.ts b/src/ChartInternal/Axis/Axis.ts index 6f9f6508e..44d69436c 100644 --- a/src/ChartInternal/Axis/Axis.ts +++ b/src/ChartInternal/Axis/Axis.ts @@ -12,6 +12,7 @@ import AxisRenderer from "./AxisRenderer"; import {getScale} from "../internals/scale"; import {$AXIS} from "../../config/classes"; import {capitalize, isArray, isFunction, isString, isValue, isEmpty, isNumber, isObjectType, mergeObj, notEmpty, parseDate, sortValue} from "../../module/util"; +import type {AxisType} from "../../../types/types"; export default { getAxisInstance: function() { @@ -260,7 +261,7 @@ class Axis { ); } - // called from : getMaxTickWidth() + // called from : getMaxTickSize() getAxis(id, scale, outerTick, noTransition, noTickTextRotate): AxisRenderer { const $$ = this.owner; const {config} = $$; @@ -525,13 +526,13 @@ class Axis { return anchor; } - dyForAxisLabel(id: string) { + dyForAxisLabel(id: AxisType) { const $$ = this.owner; const {config} = $$; const isRotated = config.axis_rotated; const isInner = this.getAxisLabelPosition(id).isInner; const tickRotate = config[`axis_${id}_tick_rotate`] ? $$.getHorizontalAxisHeight(id) : 0; - const maxTickWidth = this.getMaxTickWidth(id); + const {width: maxTickWidth} = this.getMaxTickSize(id); let dy; if (id === "x") { @@ -575,14 +576,26 @@ class Axis { return dy; } - getMaxTickWidth(id: string, withoutRecompute?: boolean): number { + /** + * Get max tick size + * @param {string} id axis id string + * @param {boolean} withoutRecompute wheather or not to recompute + * @returns {object} {width, height} + * @private + */ + getMaxTickSize(id: AxisType, withoutRecompute?: boolean): {width: number, height: number} { const $$ = this.owner; const {config, state: {current}, $el: {svg, chart}} = $$; - const currentTickMax = current.maxTickWidths[id]; - let maxWidth = 0; + const currentTickMax = current.maxTickSize[id]; + const max = { + width: 0, + height: 0 + }; - if (withoutRecompute || !config[`axis_${id}_show`] || (currentTickMax.size > 0 && $$.filterTargetsToShow().length === 0)) { - return currentTickMax.size; + if (withoutRecompute || !config[`axis_${id}_show`] || ( + currentTickMax.width > 0 && $$.filterTargetsToShow().length === 0 + )) { + return currentTickMax; } if (svg) { @@ -639,23 +652,27 @@ class Axis { dummy.selectAll("text") .each(function(d, i) { - const currentTextWidth = this.getBoundingClientRect().width; + const {width, height} = this.getBoundingClientRect(); + + max.width = Math.max(max.width, width); + max.height = Math.max(max.height, height); - maxWidth = Math.max(maxWidth, currentTextWidth); // cache tick text width for getXAxisTickTextY2Overflow() if (!isYAxis) { - currentTickMax.ticks[i] = currentTextWidth; + currentTickMax.ticks[i] = width; } }); dummy.remove(); } - if (maxWidth > 0) { - currentTickMax.size = maxWidth; - } + Object.keys(max).forEach(key => { + if (max[key] > 0) { + currentTickMax[key] = max[key]; + } + }); - return currentTickMax.size; + return currentTickMax; } getXAxisTickTextY2Overflow(defaultPadding) { @@ -688,7 +705,7 @@ class Axis { const {axis, config, state} = $$; const isTimeSeries = axis.isTimeSeries(); - const tickTextWidths = state.current.maxTickWidths.x.ticks; + const tickTextWidths = state.current.maxTickSize.x.ticks; const tickCount = tickTextWidths.length; const {left, right} = state.axis.x.padding; let maxOverflow = 0; @@ -746,7 +763,7 @@ class Axis { }; Object.keys(labels).filter(id => !labels[id].empty()) - .forEach(v => { + .forEach((v: AxisType) => { const node = labels[v]; // @check $$.$T(node, withTransition) @@ -982,7 +999,7 @@ class Axis { // set/unset x_axis_tick_clippath if (type === "x") { - const clipPath = current.maxTickWidths.x.clipPath ? clip.pathXAxisTickTexts : null; + const clipPath = current.maxTickSize.x.clipPath ? clip.pathXAxisTickTexts : null; $el.svg.selectAll(`.${$AXIS.axisX} .tick text`) .attr("clip-path", clipPath); diff --git a/src/ChartInternal/internals/clip.ts b/src/ChartInternal/internals/clip.ts index 3453b3852..7a6db37f5 100644 --- a/src/ChartInternal/internals/clip.ts +++ b/src/ChartInternal/internals/clip.ts @@ -123,16 +123,16 @@ export default { setXAxisTickClipWidth(): void { const $$ = this; - const {config, state: {current: {maxTickWidths}}} = $$; + const {config, state: {current: {maxTickSize}}} = $$; const xAxisTickRotate = $$.getAxisTickRotate("x"); if (!config.axis_x_tick_multiline && xAxisTickRotate) { const sinRotation = Math.sin(Math.PI / 180 * Math.abs(xAxisTickRotate)); - maxTickWidths.x.clipPath = ($$.getHorizontalAxisHeight("x") - 20) / sinRotation; + maxTickSize.x.clipPath = ($$.getHorizontalAxisHeight("x") - 20) / sinRotation; } else { - maxTickWidths.x.clipPath = null; + maxTickSize.x.clipPath = null; } }, @@ -142,7 +142,7 @@ export default { if (svg) { svg.select(`#${clip.idXAxisTickTexts} rect`) - .attr("width", current.maxTickWidths.x.clipPath) + .attr("width", current.maxTickSize.x.clipPath) .attr("height", 30); } } diff --git a/src/ChartInternal/internals/size.axis.ts b/src/ChartInternal/internals/size.axis.ts index a5a33b845..9cbadc394 100644 --- a/src/ChartInternal/internals/size.axis.ts +++ b/src/ChartInternal/internals/size.axis.ts @@ -26,7 +26,7 @@ export default { if ($$.axis) { const position = $$.axis?.getLabelPositionById(id); - const width = $$.axis.getMaxTickWidth(id, withoutRecompute); + const {width} = $$.axis.getMaxTickSize(id, withoutRecompute); const gap = width === 0 ? 0.5 : 0; return width + ( @@ -47,8 +47,10 @@ export default { const isFitPadding = config.padding?.mode === "fit"; const isInner = config[`axis_${id}_inner`]; const hasLabelText = config[`axis_${id}_label`].text; + const defaultHeight = 13; let h = config.padding?.mode === "fit" ? (isInner && !hasLabelText ? (id === "y" ? 1 : 0) : 20) : 30; + if (id === "x" && !config.axis_x_show) { return 8; } @@ -67,14 +69,14 @@ export default { return isFitPadding ? 0 : rotatedPadding.top; } + const maxtickSize = $$.axis.getMaxTickSize(id); const rotate = $$.getAxisTickRotate(id); // Calculate x/y axis height when tick rotated if ( ((id === "x" && !isRotated) || (/y2?/.test(id) && isRotated)) && rotate ) { - h = 30 + - $$.axis.getMaxTickWidth(id) * + h += maxtickSize.width * Math.cos(Math.PI * (90 - Math.abs(rotate)) / 180); if (!config.axis_x_tick_multiline && current.height) { @@ -82,6 +84,8 @@ export default { h = current.height / 2; } } + } else if (maxtickSize.height > defaultHeight && config.legend_show) { + h += maxtickSize.height - defaultHeight; } return h + @@ -114,7 +118,7 @@ export default { if (config.axis_x_tick_fit && allowedXAxisTypes) { const xTickCount = config.axis_x_tick_count; - const currentXTicksLength = state.current.maxTickWidths.x.ticks.length; + const currentXTicksLength = state.current.maxTickSize.x.ticks.length; let tickCount = 0; if (xTickCount) { @@ -163,9 +167,9 @@ export default { const tickCountWithPadding = axis.x.tickCount + axis.x.padding.left + axis.x.padding.right; - const maxTickWidth = $$.axis.getMaxTickWidth("x"); + const {width} = $$.axis.getMaxTickSize("x"); const tickLength = tickCountWithPadding ? xAxisLength / tickCountWithPadding : 0; - return maxTickWidth > tickLength; + return width > tickLength; } }; diff --git a/src/config/Store/State.ts b/src/config/Store/State.ts index 93d5a4978..cf8653050 100644 --- a/src/config/Store/State.ts +++ b/src/config/Store/State.ts @@ -54,10 +54,10 @@ export default class State { height: 0, dataMax: 0, - maxTickWidths: { - x: {size: 0, ticks: [], clipPath: 0, domain: ""}, - y: {size: 0, domain: ""}, - y2: {size: 0, domain: ""} + maxTickSize: { + x: {width: 0, height: 0, ticks: [], clipPath: 0, domain: ""}, + y: {width: 0, height: 0, domain: ""}, + y2: {width: 0, height: 0, domain: ""} }, // current used chart type list diff --git a/test/internals/axis-spec.ts b/test/internals/axis-spec.ts index 37960f587..9a3e75945 100644 --- a/test/internals/axis-spec.ts +++ b/test/internals/axis-spec.ts @@ -1061,6 +1061,81 @@ describe("AXIS", function() { }); }); + describe("axis x height", () => { + before(() => { + args = { + data: { + x: "x", + xFormat: "%Y", + columns: [ + ["x", "2010", "2011", "2012", "2013", "2014", "2015"], + ["data1", 30, 200, 100, 400, 150, 250], + ["data2", 130, 340, 200, 500, 250, 350] + ], + type: "line" + }, + axis: { + x: { + type: "timeseries", + localtime: false, + tick: { + format: "%Y-%m-%d %H:%M:%S" + } + } + } + } + }); + + it("shouldn't overlap x Axis tick text with legend", () => { + const {axis: {x}, legend} = chart.internal.$el; + + const xBottom = x.node().getBoundingClientRect().bottom; + const legendTop = legend.node().getBoundingClientRect().top; + + expect(legendTop > xBottom).to.be.true; + }); + + it("set option: legend.show=false", () => { + args.legend = { + show: false + }; + }); + + it("x Axis tick text should stay within container", () => { + const {$el: {axis: {x}}, state} = chart.internal; + const xBottom = x.node().getBoundingClientRect().bottom; + + expect(xBottom).to.be.closeTo(state.current.height, 5); + }); + + it("set option: legend.show=false", () => { + args = { + data: { + x: "x", + columns: [ + ["x", "First Q\n2018", "Second\nQ 2018", "3Q\nYear\n2018", "Forth\nQuarter\n2018"], + ["data", 30, 100, 400, 150] + ], + type: "line" + }, + axis: { + x: { + type: "category" + } + } + }; + }); + + it("shouldn't overlap x Axis tick text with legend", () => { + const {axis: {x}, legend} = chart.internal.$el; + + const xBottom = x.node().getBoundingClientRect().bottom; + const legendTop = legend.node().getBoundingClientRect().top; + + expect(legendTop > xBottom).to.be.true; + }); + }); + describe("axis.x.tick.tooltip", () => { before(() => { args = { @@ -3188,12 +3263,12 @@ describe("AXIS", function() { }); it("x Axis tick width should be evaluated correctly", () => { - const {state: {current}} = chart.internal; + const {state: {current: {maxTickSize}}} = chart.internal; - const maxTickWidth = current.maxTickWidths.x.size; + const {width} = maxTickSize.x; const tickWdith = chart.$.main.select(`.${$AXIS.axisX} tspan`).node().getBoundingClientRect().width; - expect(maxTickWidth).to.be.equal(tickWdith); + expect(width).to.be.equal(tickWdith); }); }); diff --git a/test/plugin/stanford/stanford-elements-spec.ts b/test/plugin/stanford/stanford-elements-spec.ts index 9625378e2..c739220fb 100644 --- a/test/plugin/stanford/stanford-elements-spec.ts +++ b/test/plugin/stanford/stanford-elements-spec.ts @@ -157,7 +157,7 @@ describe("PLUGIN: STANFORD ELEMENTS", () => { expect(line.size()).to.be.equal(1); pos.forEach((v, i) => { - expect(v).to.be.closeTo(expected[i], 3); + expect(v).to.be.closeTo(expected[i], 10); }); done();