Skip to content

fix: xAxis label overflow fixed when containLabel=true #16880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

fix: xAxis label overflow fixed when containLabel=true #16880

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c80f015
fix: xAxis label overflow fixed when containLabel=true
jiawulin001 Apr 13, 2022
c2bb3b0
Update testcase and lint style error
jiawulin001 Apr 13, 2022
6b57c89
fix lint style error
jiawulin001 Apr 13, 2022
c8a4906
fix lint style error 2
jiawulin001 Apr 13, 2022
21b4380
Update first/last label get with bounding rect clone
jiawulin001 Apr 18, 2022
f00c4ec
Update method with getTicksCoord
jiawulin001 Apr 19, 2022
f53c271
fix lint error
jiawulin001 Apr 19, 2022
7ee50c2
find exceed by creating bounding rect
jiawulin001 Apr 21, 2022
b30b557
fix lint errors
jiawulin001 Apr 21, 2022
0fc9077
Delete step change to correct tickvalue for labelview
jiawulin001 Apr 25, 2022
22d6449
Update the condition check for index of label rect
jiawulin001 Apr 25, 2022
095110e
fix: time axis label rect do not use tickvalue
jiawulin001 Apr 27, 2022
eb5be1e
fix: Update estimateLabelRects early return
jiawulin001 Apr 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/coord/axisHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,9 @@ export function getAxisRawValue(axis: Axis, tick: ScaleTick): number | string {

/**
* @param axis
* @return Be null/undefined if no labels.
* @return Return the largest, first and the last label BoundingRects. Be null/undefined if no labels.
*/
export function estimateLabelUnionRect(axis: Axis) {
export function estimateLabelRect(axis: Axis) {
const axisModel = axis.model;
const scale = axis.scale;

Expand All @@ -319,6 +319,8 @@ export function estimateLabelUnionRect(axis: Axis) {
const labelFormatter = makeLabelFormatter(axis);

let rect;
let firstLabelRect;
let lastLabelRect;
let step = 1;
// Simple optimization for large amount of labels
if (tickCount > 40) {
Expand All @@ -333,11 +335,16 @@ export function estimateLabelUnionRect(axis: Axis) {
const label = labelFormatter(tick, i);
const unrotatedSingleRect = axisLabelModel.getTextRect(label);
const singleRect = rotateTextRect(unrotatedSingleRect, axisLabelModel.get('rotate') || 0);

if (i === 0) {
firstLabelRect = new BoundingRect(singleRect.x, singleRect.y, singleRect.width, singleRect.height);
}
if (i + step >= tickCount) {
lastLabelRect = new BoundingRect(singleRect.x, singleRect.y, singleRect.width, singleRect.height);
}
rect ? rect.union(singleRect) : (rect = singleRect);
}

return rect;
return {rect, firstLabelRect, lastLabelRect};
}

function rotateTextRect(textRect: RectLike, rotate: number) {
Expand Down
34 changes: 27 additions & 7 deletions src/coord/cartesian/Grid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@
*/

import {isObject, each, indexOf, retrieve3, keys} from 'zrender/src/core/util';
import {getLayoutRect, LayoutRect} from '../../util/layout';
import {box, getLayoutRect, LayoutRect} from '../../util/layout';
import {
createScaleByModel,
ifAxisCrossZero,
niceScaleExtent,
estimateLabelUnionRect,
estimateLabelRect,
getDataDimensionsOnAxis
} from '../../coord/axisHelper';
import Cartesian2D, {cartesian2DDimensions} from './Cartesian2D';
Expand Down Expand Up @@ -179,16 +179,14 @@ class Grid implements CoordinateSystemMaster {
});

this._rect = gridRect;

const axesList = this._axesList;

adjustAxes();

// Minus label size
if (isContainLabel) {
each(axesList, function (axis) {
if (!axis.model.get(['axisLabel', 'inside'])) {
const labelUnionRect = estimateLabelUnionRect(axis);
const labelUnionRect = estimateLabelRect(axis).rect;
if (labelUnionRect) {
const dim: 'height' | 'width' = axis.isHorizontal() ? 'height' : 'width';
const margin = axis.model.get(['axisLabel', 'margin']);
Expand All @@ -202,10 +200,32 @@ class Grid implements CoordinateSystemMaster {
}
}
});

//Adjust grid.width to keep xAxis labels in dom
const [xAxis, yAxis] = axesList[0].isHorizontal() ? axesList : axesList.slice().reverse();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need complex logic to check the first label and last label here.

In the

echarts/src/coord/cartesian/Grid.ts

Line 193 in c8a4906

gridRect[dim] -= labelUnionRect[dim] + margin;
we only adjust x/width based on the labels of yAxis. We can also check if the union rect of xAxis labels excced the canvas size. Then adjust x/width to avoid labels overflow.

Copy link
Member Author

@jiawulin001 jiawulin001 Apr 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function labelUnionRect actually returns the largest bounding box of all labels of an axis, not the union bounding box. So I am not sure how to get the union rect of xAxis with this function. Also it would be hard to get the real size of displayed bounding box of labels because echarts seems to automatically clip labels according to axis length. That's why we just need to make sure the first/last label does not exceed the dom. Please correct me if I misunderstand you.

Copy link
Contributor

@pissang pissang Apr 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function labelUnionRect actually returns the largest bounding box of all labels of an axis, not the union bounding box

The method estimateLabelUnionRect returns the union bounding box of all labels, not the largest.

so it would be hard to get the real size of displayed bounding box of labels because echarts seems to automatically clip labels according to axis length

I'm not sure what do you mean by clip labels. Correct me if I'm wrong. Indeed we only pick some of the labels if there are amount of theme, especially in category axis.

echarts/src/coord/axisHelper.ts

Line 324 in 4a52199

if (tickCount > 40) {

Perhaps the last label will also be skipped. We can add some extra logic to ensure the first label and last label are also included.

Copy link
Contributor

@pissang pissang Apr 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the current logic of estimating interval and get formatted labels seems has been capsulated in the method

echarts/src/coord/Axis.ts

Line 223 in 4a52199

getViewLabels(): ReturnType<typeof createAxisLabels>['labels'] {

Using this method to get all formatted labels will be more accurate and performant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method estimateLabelUnionRect returns the union bounding box of all labels, not the largest.

The logic of function rect.union is returning the union of bounding rect.
But here in echarts, if you trace the dataflow, every singleRect has {x: 0, y: 0}. So when you use union on two rects at the same position, you are just returning the larger one. That's why I say it actually returns the largest bounding rect of all labels.

echarts/src/coord/axisHelper.ts

Lines 327 to 338 in 4a52199

for (let i = 0; i < tickCount; i += step) {
const tick = realNumberScaleTicks
? realNumberScaleTicks[i]
: {
value: categoryScaleExtent[0] + i
};
const label = labelFormatter(tick, i);
const unrotatedSingleRect = axisLabelModel.getTextRect(label);
const singleRect = rotateTextRect(unrotatedSingleRect, axisLabelModel.get('rotate') || 0);
rect ? rect.union(singleRect) : (rect = singleRect);
}

This doesn't result in errors for now because only x/height and y/width is used in older version, and they are the same for union rect and largest rect.

However, thanks to your suggestion, I double-checked Axis.ts and found the function getTicksCoords, which returns all displayed labels' (or ticks', they bind each other) coordinates. So I rewrite the changes and now I can accurately grab the first and the last label and check whether they would exceed the dom. Please review again.

P.S. Do you mind telling me how to get the dom size in Echarts? I struggled to get dom attributes but failed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what I want to emphasize is right-most DISPLAYED label. The function getTicksCoord would only return the labels that would be DISPLAYED. Those which are automatically hidden will not be included so I can just call getTicksCoord()[0] to get the left-most DISPLAYED label and the right-most alike.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, xAxis.inverse is also considered. For example, left-most label will be treated as the right-most if xAxis.inverse = true.

Copy link
Contributor

@pissang pissang Apr 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what I want to emphasize is right-most DISPLAYED label. The function getTicksCoord would only return the labels that would be DISPLAYED. Those which are automatically hidden will not be included so I can just call getTicksCoord()[0] to get the left-most DISPLAYED label and the right-most alike.

I think that's not the difference. The union bounding rect should also be from the displayed labels .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, do you mean use getTicksCoord to find displayed labels index, find their bounding box and implement union bounding rects on them? I don't see why we need to do that. Are you worrying that some too long in-middle label would exceed the dom too?

Copy link
Contributor

@pissang pissang Apr 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getViewLabels will return the labels that will be finally displayed on the chart. See how it is used when rendering the axis.
https://github.com/apache/echarts/blob/master/src/component/axis/AxisBuilder.ts

However the getTicksCoord will have a different result when user-set axisTick.interval and axisLabel.interval are different. getTicksCoord will use the axisTick.interval to skip the tick lines and getViewLabels will use axisLabel.interval to skip the coord.

image

const {firstLabelRect, lastLabelRect} = estimateLabelRect(xAxis);
const labelUnionRect = estimateLabelRect(yAxis).rect;
const margin = xAxis.model.get(['axisLabel', 'margin']);
//When yAxis is on the right, check the left margin instead
if (yAxis.position === 'right') {
if (firstLabelRect.width / 2 >= boxLayoutParams.left) {
gridRect.width -= firstLabelRect.width / 2 - Number(boxLayoutParams.left) + margin;
gridRect.x += firstLabelRect.width / 2 - Number(boxLayoutParams.left) + margin;
}
}
else {
//Long last label exceeds the right boundary
if (lastLabelRect.width / 2 >= boxLayoutParams.right) {
gridRect.width -= lastLabelRect.width / 2 - Number(boxLayoutParams.right) + margin;
}
//Long first label still exceeds the left boundary even when yAxis on the left
let leftMargin = Number(boxLayoutParams.left);
if (firstLabelRect.width / 2 >= leftMargin + labelUnionRect.width) {
gridRect.width -= firstLabelRect.width / 2 - leftMargin - labelUnionRect.width + margin;
gridRect.x += firstLabelRect.width / 2 - leftMargin - labelUnionRect.width + margin;
}
}
adjustAxes();
}

each(this._coordsList, function (coord) {
// Calculate affine matrix to accelerate the data to point transform.
// If all the axes scales are time or value.
Expand Down
Loading