Skip to content

feat(tooltip): add displayTransition option to control whether to enable the tooltip display transition #20966

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

Merged
merged 7 commits into from
May 31, 2025

Conversation

jqqin
Copy link

@jqqin jqqin commented May 12, 2025

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Add new displayTransition option to control whether to enable the tooltip display transition. Setting it to false allows fully hiding the tooltip element on mouse out by setting display: 'none', which can prevent the scrollbar from appearing when the tooltip has an excessive height due to too many series.

Fixed issues

Details

Before: What was the problem?

When the tooltip content is very long and a user hovers over a bar, it creates extra space and scrollbars to accommodate the full tooltip. However, when the user moves the mouse away, the tooltip DOM element is only hidden via visibility: hidden, which keeps it in the layout flow:
This leads to:

  • Scrollbars remaining
  • Extra white space
  1. When hovering over, extra space and scrollbars area created:

image

  1. When hovering away, the tooltip is only visually hidden (via visibility: hidden), but the space and scrollbars remain in the layout.

image

After: How does it behave after the fixing?

  1. We updated the hide function to set display: none:
image
  1. After the fix, when hovering away, the tooltip is completely removed from the layout. The extra space and scrollbar are no longer present.

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

See test/tooltip-fadeTransition.html

Others

Merging options

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

Other information

Copy link

echarts-bot bot commented May 12, 2025

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.

⚠️ MISSING DOCUMENT INFO: Please make sure one of the document options are checked in this PR's description. Search "Document Info" in the description of this PR. This should be done either by the author or the reviewers of the PR.

@jqqin jqqin changed the title set tooltip display to none when hide (fix)set tooltip display to none when hide May 12, 2025
@jqqin
Copy link
Author

jqqin commented May 20, 2025

@Ovilia This PR addresses an issue where hidden tooltips still occupy space and trigger scrollbars due to visibility: hidden not fully removing them from layout. It adds display: none to cleanly remove the tooltip from the DOM flow when hiding.

Would appreciate it if you could take a quick look — or feel free to redirect me if someone else would be a more appropriate reviewer. Thanks so much!

@plainheart
Copy link
Member

plainheart commented May 21, 2025

Thanks for your contribution! This is an issue like #17377 that needs discussion. I think we can add a new option fadeAnimation of the boolean type to control the show/hide behavior.

@jqqin
Copy link
Author

jqqin commented May 21, 2025

@plainheart Thanks for the reply!
Just wondering — is there any rough timeline or milestone planned for when this might be discussed or decided? I noticed the 6.0.0 milestone tag and was curious if this would fall under that release. Appreciate any guidance!

@plainheart
Copy link
Member

@jqqin Hi, could you please add a test case? It seems the scrollbar doesn't appear in the latest browsers, even if the tooltip overflows the container.

@jqqin
Copy link
Author

jqqin commented May 27, 2025

Hi @plainheart, thanks for your response, in our case now, when hovering over the bar, the tooltip appears and the also the scroll apears.
Screenshot 2025-05-27 at 4 04 11 PM

When hovering away, no scroll bar is there, which is better than before:
image

I wonder what the changes have been made for this? and what are the browsers/versions you are using that has no scrollbar appearing ? For me, i used echart 5.6.0 and Chrome: Version 137.0.7151.56 (Official Build) (arm64).

Thanks a lot!

@jqqin
Copy link
Author

jqqin commented May 28, 2025

@plainheart
Hi again, just wanted to share an update based on further investigation for the last comment:

Our company uses Apache Superset, a business intelligence platform that internally uses ECharts for chart rendering. Recently, we noticed that the tooltip scroll overflow issue is no longer present in our Superset 5 environment — even though tooltips can still be quite long.

After digging into it, we found that the improved behavior is because Superset applies a global CSS override specifically to address this problem. The relevant rule is:
image

This was added as part of Superset issue #30058 and is included in their GlobalStyles.tsx. It cleanly resolves the stale tooltip layout issue by hiding the tooltip more thoroughly when it becomes invisible.

Since this CSS is functionally identical to the fix I originally proposed here (adding display: none), and is already used in production by a major project integrating ECharts, I was wondering if you'd consider accepting this contribution as a native fix within ECharts itself — or advise on an alternative you'd prefer.

Thanks again for the review and all the great work on ECharts!

@echarts-bot echarts-bot bot added PR: awaiting doc Document changes is required for this PR. and removed PR: doc unchanged labels May 29, 2025
Copy link

echarts-bot bot commented May 29, 2025

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.

@plainheart plainheart changed the title (fix)set tooltip display to none when hide feat(tooltip): add displayTransition option to control whether to enable the tooltip display transition May 29, 2025
Copy link
Contributor

github-actions bot commented May 29, 2025

The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-20966@a61f52e

@plainheart
Copy link
Member

Hi @jqqin, I've implemented a new option displayTransition to control the display behavior based on your PR. You can test it and share your feedback. I will also request other members to review this change, targeting inclusion in the upcoming v6.0.0 release.

@plainheart plainheart requested review from pissang and 100pah May 29, 2025 19:35
@plainheart plainheart added this to the 6.0.0 milestone May 29, 2025
@jqqin
Copy link
Author

jqqin commented May 29, 2025

@plainheart Thank you for the quick follow-up and thoughtful implementation! Really appreciate the work you’ve put into this. Looking forward to seeing it included in the upcoming 6.0.0 release!

}
else {
style.display = 'none';
}
Copy link
Member

Choose a reason for hiding this comment

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

I think it's reasonable to add an new option displayTransition.

But if displayTransition: true, this "scroll" problem still happen.
Perhaps the more completely solution could be:

private _displayNoneWhenTransitionEnd: () => void;

show() {
        // ...
        if (this._displayNoneWhenTransitionEnd) {
            el.removeEventListener('transitionend', this._displayNoneWhenTransitionEnd);
        }
}

hide() {
        const el = this.el;
        if (this._displayNoneWhenTransitionEnd) {
            el.removeEventListener('transitionend', this._displayNoneWhenTransitionEnd);
        }
        this._displayNoneWhenTransitionEnd = () => {
            el.style.display = 'none';
        };
        el.addEventListener('transitionend', this._displayNoneWhenTransitionEnd);

}

(But I haven't test that code above.)

@plainheart plainheart merged commit 4ea877a into apache:master May 31, 2025
2 checks passed
Copy link

echarts-bot bot commented May 31, 2025

Congratulations! Your PR has been merged. Thanks for your contribution! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: awaiting doc Document changes is required for this PR. PR: first-time contributor size/M
Projects
None yet
3 participants