-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
fix: revert chore(deps): bump d3-svg-legend in /superset-frontend (#19846) #19972
fix: revert chore(deps): bump d3-svg-legend in /superset-frontend (#19846) #19972
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19972 +/- ##
=======================================
Coverage 66.29% 66.29%
=======================================
Files 1712 1712
Lines 63962 63962
Branches 6731 6731
=======================================
Hits 42404 42404
Misses 19847 19847
Partials 1711 1711
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
LGTM! Let's migrate to Echarts version next! |
Agreed @stephenLYZ , it seems like a perfect drop in replacement! |
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!
More hackery! removing some giant padding lint add dedicated mobile collapse button style action buttons replace down with up arrow Make it sticky Adding viewport tag Making Filters a Sidebar Again (and letting the header scroll away) delint Revert "chore(deps): bump d3-svg-legend in /superset-frontend (apache#19846)" (apache#19972) This reverts commit 528a9cd. customize font sizes
…#19846)" (apache#19972) This reverts commit 528a9cd.
…#19846)" (apache#19972) This reverts commit 528a9cd.
SUMMARY
d3-svg-legend
version 2 is incompatible withd3
version 3 (see: susielu/d3-legend#58), causing the Heatmap chart to fail to render. I contemplated the option of bumpingd3
to version 4, but decided against it due to the following reasons:d3
version 4 and would have required fairly big changesd3
is now on version 7d3-svg-legend
has been stale for 4 yearsInstead I think we should just migrate this plugin to
d3
version 7 or the equivalent ECharts version (https://echarts.apache.org/examples/en/editor.html?c=heatmap-cartesian) while migrating this plugin to the v1 chart data endpoint.Related PR: #19846
AFTER
BEFORE
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION