Skip to content

fix(axis): fix time axis labels and ticks being overlapped #17311

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.

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 5 commits into
base: master
Choose a base branch
from

Conversation

Ovilia
Copy link
Contributor

@Ovilia Ovilia commented Jul 4, 2022

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Fix the cases when time axis labels and ticks being overlapped.

Fixed issues

#17198

Details

Before: What was the problem?

Some of the labels are too close to each other.

After: How does it behave after the fixing?

Change 1. Labels with priority

Before: Labels with different levels (year/month/day/...) have the same priority. So even #15583 can help remove the labels when overlapped (BWT, it seems not always remove overlapped labels), more important information can be lost.

image

After: Labels with larger level (e.g., year labels have larger levels than month labels) are displayed with higher priority and then display labels with smaller levels when there is enough space.

image

Change 2. A new option axisLabel.formatterMinUnit

Before: If a developer want to display year labels when zoomed out and day labels when zoomed in but never display hour/minute/... level labels, the only thing can be done is by setting formatter to be:

formatter: {
  hour: '{d}', // or set to be '' to hide labels but you cannot hide the ticks
  minute: '{d}',
  second: '{d}',
  millisecond: '{d}'
}

This is not intuitive and it cannot help remove these ticks.

After: By setting axisLabel.formatterMinUnit to be 'day', ticks with levels smaller than 'day' will not be displayed.

Change 3. Align showMinLabel/showMaxLabel logic with other types of axis

Before: The default value of showMinLabel/showMaxLabel of time axis is set to be false while other types axis is set to be null as the document states:

It is auto determined by default, that is, if labels are overlapped, the label of the min tick will not be displayed.

showMinLabel/showMaxLabel is set to be false by default because time axis may have min values like 2022-10-12 12:34:23 123, which is formatted to be 12:34:23 123 and we don't expect the start/end values to be like this. Instead, developers may expect to display 2022-10-12 or 10-12, etc.

After: The default value of showMinLabel/showMaxLabel of time axis is set to be null and the first/last label is displayed when there is enough space. Since we now have formatterMinUnit, developers can easily set it to be 'day' to hide it when its level is less than day level.

Another useful way to use it is to set showMinLabel/showMaxLabel to be true so that the first/last label should always be displayed. But don't worry. In this case, if the level of the label is less than formatterMinUnit, then the formatter of formatterMinUnit is used. For example, if we set formatterMinUnit to be 'day' and the start label value is 2022-10-12 12:34:23 123, then the formatter of day is used so it's formatted to be 10-12:

axisLabel: {
  formatterMinUnit: 'day',
  formatter: {
    day: 'MMM-d'
  }
}

Change 4:

Before: axisLabel.padding is not considered when checking overlapping.

image

After: axisLabel.padding is considered when checking overlapping.

image

Change 5:

Before: When in day unit, the time labels are too crowded.

image

After: Use a larger approximate interval for day unit even if they don't overlap.

image

Document Info

One of the following should be checked.

  • This PR doesn't relate to document changes
  • The document should be updated later
  • The document changes have been made in apache/echarts-doc#xxx

Misc

ZRender Changes

  • This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

  • timeScale-formatter.html

Others

Merging options

  • Please squash the commits into a single one when merging.

Other information

@echarts-bot
Copy link

echarts-bot bot commented Jul 4, 2022

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

The pull request is marked to be PR: author is committer because you are a committer of this project.

@pissang
Copy link
Contributor

pissang commented Jul 4, 2022

I'm not sure but this issue seems to have been addressed in #15583

@Ovilia
Copy link
Contributor Author

Ovilia commented Jul 5, 2022

image

#15583 's fixing may have a potential problem that the ticks of the overlapped labels are not hidden, so that the ticks are not always the same with the labels. @pissang Do you think it a better idea to avoid overlapping as in this PR so that the ticks are always the same with the labels?

And after the change in #15712, it requires hideOverlap to avoid overlap, but I think this should be a default behavior (or at least the default value of hideOverlap of the axis labels should be set to be true). This should not considered as backward compatibility because the overlapped labels are bugs.

@Ovilia
Copy link
Contributor Author

Ovilia commented Jul 5, 2022

@pissang If you think I'm on the right track, I can keep fixing this PR. For now, this PR has a problem that when calculating the bounding box of each label, the unparsed text is used. For example, {primary|8th} may be used to calculate the text width rather than 8th so it may overkill the overlapping. This is also a bug for category axis labels.

If you think it is not a big deal if the ticks are not always the same as the labels, we can keep the current code and discuss if it's better to make hideOverlap: true by default.

@pissang
Copy link
Contributor

pissang commented Jul 8, 2022

#15583 's fixing may have a potential problem that the ticks of the overlapped labels are not hidden, so that the ticks are not always the same with the labels. @pissang Do you think it a better idea to avoid overlapping as in this PR so that the ticks are always the same with the labels?

Ticks lines can follow the visibility of labels. It should be easy. But the current design is not following it.

And after the change in #15712, it requires hideOverlap to avoid overlap, but I think this should be a default behavior (or at least the default value of hideOverlap of the axis labels should be set to be true). This should not considered as backward compatibility because the overlapped labels are bugs.

Adding an extra option is what I suggested. Because the overlapping issue is brought by the new time axis refactoring in the 5.0 version. The best way to fix it is to optimize the interval strategy of the new time axis. Current strategy may give too many ticks than the specified splitNumber. Hiding overlapped labels is just a workaround.

@echarts-bot
Copy link

echarts-bot bot commented Nov 22, 2022

Document changes are required in this PR. Please also make a PR to apache/echarts-doc for document changes and update the issue id in the PR description. When the doc PR is merged, the maintainers will remove the PR: awaiting doc label.

@echarts-bot echarts-bot bot added the PR: awaiting doc Document changes is required for this PR. label Nov 22, 2022
@Ovilia Ovilia marked this pull request as ready for review November 23, 2022 02:29
@@ -221,7 +221,8 @@ class Axis {
}

getViewLabels(): ReturnType<typeof createAxisLabels>['labels'] {
return createAxisLabels(this).labels;
const labels = createAxisLabels(this).labels;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change should be reverted in later commits.

@Ovilia Ovilia modified the milestones: 5.5.1, 6.0.0 Jan 31, 2024
@Ovilia Ovilia modified the milestones: 6.0.0, 6.x Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

[Feature] Time axis interval [Bug] X-Axis labels are too close to each other during month change on weekly interval
3 participants