-
Notifications
You must be signed in to change notification settings - Fork 14.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(plugin-chart-echarts): tooltip overflow bug #22218
fix(plugin-chart-echarts): tooltip overflow bug #22218
Conversation
b31f2aa
to
4baf5a7
Compare
/testenv up |
@zhaoyongjie Container image not yet published for this PR. Please try again when build is complete. |
@zhaoyongjie Ephemeral environment creation failed. Please check the Actions logs for details. |
/testenv up |
@zhaoyongjie Ephemeral environment spinning up at http://52.12.29.135:8080. Credentials are |
Codecov Report
@@ Coverage Diff @@
## master #22218 +/- ##
=======================================
Coverage 66.92% 66.92%
=======================================
Files 1835 1835
Lines 69988 69988
Branches 7612 7612
=======================================
Hits 46839 46839
Misses 21183 21183
Partials 1966 1966
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
LGTM, tested the fix + verified that the old bug isn't reintroduced.
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.
There is an issue unrelated to this PR but label related when I test this. @jinghua-qa label.in.Pie.mov |
@zhaoyongjie this bug seems to be unrelated to this PR (I was able to repro on master), but I can look into it in a separate PR. Edit: This is a regression on the current stable version of ECharts and has been fixed here: apache/echarts#17715 |
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
This fixes a regression from #22082 which caused the tooltip to become cut when overflowing the chart DOM element. By making sure all tooltips have the
appendToBody
(see: https://echarts.apache.org/en/option.html#tooltip.appendToBody) property set totrue
we can ensure overflowing is handled correctly on all ECharts plugins.When reviewing the PR, make sure to use "hide whitespace", as the changes to the tooltip position function are all indentation changes.
AFTER
Now overflowing works as expected:
overflow-after.mp4
BEFORE
Previously overflowing on certain charts caused the tooltip to be cut:
overflow-before.mp4
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION