Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

fix(plugin-chart-echarts): enable animation to clear old nodes #1200

Merged
merged 2 commits into from
Jul 2, 2021

Conversation

villebro
Copy link
Contributor

@villebro villebro commented Jul 2, 2021

🐛 Bug Fix

Due to a bug present in ECharts 5.1.2, Treemap doesn't properly rerender when the option changes when animation is disabled (see apache/echarts#15282 for details). Until the fix apache/echarts#15283 is merged and released, we should enable animations to work around the bug. I also consider the animation pretty nice, so I wouldn't mind leaving animations enabled.

In addition, the color scheme is changed to reference the group name, aligning it with colors in other charts in the dashboard.

AFTER

treemap-after2.mp4

BEFORE

treemap-before.mp4

@villebro villebro requested a review from a team as a code owner July 2, 2021 11:55
@vercel
Copy link

vercel bot commented Jul 2, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/superset/superset-ui/6VaAB5uyS2iarbVUyBUGbLKG5AbB
✅ Preview: https://superset-ui-git-fork-preset-io-villebro-treemap-clear-superset.vercel.app

@stephenLYZ
Copy link
Contributor

LGTM!

@codecov
Copy link

codecov bot commented Jul 2, 2021

Codecov Report

Merging #1200 (64b6d7d) into master (60215b5) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1200      +/-   ##
==========================================
+ Coverage   29.09%   29.12%   +0.02%     
==========================================
  Files         463      463              
  Lines        9358     9361       +3     
  Branches     1496     1496              
==========================================
+ Hits         2723     2726       +3     
  Misses       6424     6424              
  Partials      211      211              
Impacted Files Coverage Δ
...plugin-chart-echarts/src/Treemap/transformProps.ts 51.56% <ø> (ø)
plugins/plugin-chart-echarts/src/utils/prophet.ts 91.83% <0.00%> (ø)
...s/plugin-chart-echarts/src/Graph/transformProps.ts 71.23% <0.00%> (ø)
...plugin-chart-echarts/src/BoxPlot/transformProps.ts 56.25% <0.00%> (ø)
plugins/plugin-chart-echarts/src/utils/series.ts 92.64% <0.00%> (+0.10%) ⬆️
.../plugin-chart-echarts/src/Funnel/transformProps.ts 75.00% <0.00%> (+0.64%) ⬆️
...ins/plugin-chart-echarts/src/Pie/transformProps.ts 71.11% <0.00%> (+0.65%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60215b5...64b6d7d. Read the comment docs.

Copy link
Contributor

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

Nice! LGTM

@villebro villebro merged commit 1ee7f4e into apache-superset:master Jul 2, 2021
@villebro villebro deleted the villebro/treemap-clear branch July 2, 2021 13:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants