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

feat(plugin-chart-echarts): Echarts Waterfall #1150

Closed
wants to merge 10 commits into from

Conversation

stephenLYZ
Copy link
Contributor

@stephenLYZ stephenLYZ commented Jun 6, 2021

🏆 Enhancements

Waterfall charts are useful for understanding how an initial value (like net income) is affected by a series of positive and negative changes.

For example, we can see the trend of personal annual income. Here we use the year as the series.
image

Moreover, we can use the value in breakdowns to add additional data to the chart. It adds the contributors to increases or decreases for each series.
image

This pr refers to #705 (Thanks for Simcha's work.

@stephenLYZ stephenLYZ requested a review from a team as a code owner June 6, 2021 12:30
@vercel
Copy link

vercel bot commented Jun 6, 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/FJbino8EN3i1DjaNk6njydZ9RfP6
✅ Preview: https://superset-ui-git-fork-stephenlyz-feature-waterfall-superset.vercel.app

@xiezhongfu
Copy link
Contributor

Thanks stephen.
Core-code is LGTM for me. Next, I will review the code in detail.

@codecov
Copy link

codecov bot commented Jul 22, 2021

Codecov Report

Merging #1150 (4054041) into master (8249e92) will increase coverage by 0.37%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1150      +/-   ##
==========================================
+ Coverage   30.03%   30.41%   +0.37%     
==========================================
  Files         489      495       +6     
  Lines        9854     9958     +104     
  Branches     1654     1680      +26     
==========================================
+ Hits         2960     3029      +69     
- Misses       6653     6678      +25     
- Partials      241      251      +10     
Impacted Files Coverage Δ
...n-chart-echarts/src/Waterfall/EchartsWaterfall.tsx 0.00% <0.00%> (ø)
...lugins/plugin-chart-echarts/src/Waterfall/index.ts 0.00% <0.00%> (ø)
...lugins/plugin-chart-echarts/src/Waterfall/types.ts 0.00% <0.00%> (ø)
...s/plugin-chart-echarts/src/Waterfall/buildQuery.ts 66.66% <66.66%> (ø)
...ugin-chart-echarts/src/Waterfall/transformProps.ts 68.75% <68.75%> (ø)
...lugin-chart-echarts/src/Waterfall/controlPanel.tsx 100.00% <100.00%> (ø)
plugins/plugin-chart-echarts/src/controls.tsx 71.42% <100.00%> (ø)
... and 3 more

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 8249e92...4054041. 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.

Awesome works! Leave some issues.

  1. the null value needs to be handled in tooltip
    image

  2. please use seriese control as category, and need to DND support
    image

  3. I can't hidden metric value and tooltip on the viz

image

  1. the Breakdown is broken

image

buildQuery,
controlPanel,
loadChart: () => import('./EchartsWaterfall'),
metadata: new ChartMetadata({
Copy link
Contributor

Choose a reason for hiding this comment

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

needs category property here. Might Ranking is is an appropriate category

Copy link
Member

Choose a reason for hiding this comment

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

Evolution seems reasonable to me.

@stephenLYZ stephenLYZ changed the title feat(plugin-chart-echarts): Echarts Waterfall POC feat(plugin-chart-echarts): Echarts Waterfall Sep 12, 2021
@stephenLYZ
Copy link
Contributor Author

stephenLYZ commented Sep 13, 2021

Awesome works! Leave some issues.

  1. the null value needs to be handled in tooltip
    image
  2. please use seriese control as category, and need to DND support
    image
  3. I can't hidden metric value and tooltip on the viz

image

  1. the Breakdown is broken
image
  1. fixed.
  2. fixed.
  3. add show_value control and rich_tooltip control for this chart.
  4. The logic of the previous breakdown was to hide the breakdown of the first series and only show the total (a reference to powerBI?). Maybe it is strange because it will cause the data and the chart to not correspond, I removed this special logic first and need to discuss whether this special logic is needed.

@villebro
Copy link
Contributor

The codebase on this repo has been moved to the main Apache Superset repo, and consequently the repo is in the process of being archived. See the Superset Improvement Proposal for details: apache/superset#13013 . While all currently open issues and PRs will be closed, we encourage you to reopen this PR on the main repo, which should be as simple as moving over any code changes as follows:

  • core packages: packages/** (this repo) -> superset-frontend/packages/** (main repo)
  • plugins: plugins/** (this repo) -> superset-frontend/plugins/** (main repo)

If you need help with the migration, please post a message on the SIP or reach out on the community Slack.

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.

5 participants