-
Notifications
You must be signed in to change notification settings - Fork 19.8k
feat(tooltip): add displayTransition option to control whether to enable the tooltip display transition#20966
feat(tooltip): add displayTransition option to control whether to enable the tooltip display transition#20966plainheart merged 7 commits intoapache:masterfrom
displayTransition option to control whether to enable the tooltip display transition#20966Conversation
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
- Resolves [Bug] tooltip appendtobodyHou Yin Cang Fang Shi Bu He Gua Hui Dao Zhi Chu Xian Gun Dong Tiao #17377
- Resolves [Bug] Tooltip is not removed nor set to display: none #17251
- Resolves [Feature] Remove tooltip DOM or hide it with
display: nonewhen it's invisible #17059
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
- When hovering over, extra space and scrollbars area created:
- When hovering away, the tooltip is only visually hidden (via
visibility: hidden), but the space and scrollbars remain in the layout.
After: How does it behave after the fixing?
- We updated the
hidefunction to setdisplay: none:
- After the fix, when hovering away, the tooltip is completely removed from the layout. The extra space and scrollbar are no longer present.
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@05bb8ae
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
|
Thanks for your contribution! |
|
@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! |
|
Thanks for your contribution! This is an issue like #17377 that needs discussion. I think we can add a new option |
|
@plainheart Thanks for the reply! |
|
@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. |
|
Hi @plainheart, thanks for your response, in our case now, when hovering over the bar, the tooltip appears and the also the scroll apears. When hovering away, no scroll bar is there, which is better than before: 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! |
|
@plainheart 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: 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! |
# src/component/tooltip/TooltipHTMLContent.ts
|
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 |
displayTransition option to control whether to enable the tooltip display transition
|
The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-20966@a61f52e |
|
Hi @jqqin, I've implemented a new option |
|
@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'; | ||
| } |
There was a problem hiding this comment.
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:
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.)
|
Congratulations! Your PR has been merged. Thanks for your contribution! |
|
@jqqin Thanks for your contribution. Do you mind adding the doc at https://github.com/apache/echarts-doc |