Skip to content

Commit

Permalink
fix(axis): Fix x axis tick text overlap with legend
Browse files Browse the repository at this point in the history 
Compute tick text's height and applies when is greater than the
default height value.

Fix naver#3485
  • Loading branch information
@netil
netil authored Oct 24, 2023
1 parent 416204b commit 796671f
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 36 deletions.
53 changes: 35 additions & 18 deletions src/ChartInternal/Axis/Axis.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -260,7 +261,7 @@ class Axis {
);
}

// called from : getMaxTickWidth()
// called from : getMaxTickSize()
getAxis(id, scale, outerTick, noTransition, noTickTextRotate): AxisRenderer {
const $$ = this.owner;
const {config} = $$;
Expand Down Expand Up @@ -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") {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/ChartInternal/internals/clip.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
},

Expand All @@ -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);
}
}
Expand Down
16 changes: 10 additions & 6 deletions src/ChartInternal/internals/size.axis.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 + (
Expand All @@ -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;
}
Expand All @@ -67,21 +69,23 @@ 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) {
if (h > current.height / 2) {
h = current.height / 2;
}
}
} else if (maxtickSize.height > defaultHeight && config.legend_show) {
h += maxtickSize.height - defaultHeight;
}

return h +
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
};
8 changes: 4 additions & 4 deletions src/config/Store/State.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ export default class State {
height: 0,
dataMax: 0,

maxTickWidths: {
x: {size: 0, ticks: <number[]> [], clipPath: 0, domain: ""},
y: {size: 0, domain: ""},
y2: {size: 0, domain: ""}
maxTickSize: {
x: {width: 0, height: 0, ticks: <number[]> [], clipPath: 0, domain: ""},
y: {width: 0, height: 0, domain: ""},
y2: {width: 0, height: 0, domain: ""}
},

// current used chart type list
Expand Down
81 changes: 78 additions & 3 deletions test/internals/axis-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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);
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/plugin/stanford/stanford-elements-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 796671f

Please sign in to comment.