Skip to content

Conversation

@SBIN2010
Copy link
Contributor

This PR Waterfall chart improvement #27856

SUMMARY

  1. Added tooltips for series color settings
  2. Series settings, X-axis, Y-axis are separated into separate groups
  3. Added a change in the lable of the series

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Снимок экрана от 2025-08-25 22-52-45 Снимок экрана от 2025-08-25 22-54-28

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot bot added the viz:charts:echarts Related to Echarts label Aug 25, 2025
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

I've completed my review and didn't find any issues.

Files scanned
File Path Reviewed
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx
superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/transformProps.ts

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

@sfirke
Copy link
Member

sfirke commented Aug 25, 2025

Thank you for implementing the request at #27856! @kasiazjc I know you have given great design feedback on other PRs, do you have any feedback on the UI for these new controls?

@SBIN2010
Copy link
Contributor Author

SBIN2010 commented Sep 5, 2025

@yousoph @kasiazjc could you please give some feedback on the design of these new controls

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2025

⚠️ DEPRECATED WORKFLOW ⚠️

@kasiazjc This workflow is deprecated! Please use the new Superset Showtime system instead:

Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

@kasiazjc kasiazjc added 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR and removed testenv-up labels Sep 8, 2025
@kasiazjc
Copy link
Contributor

kasiazjc commented Sep 8, 2025

thanks @SBIN2010! I am spinning up a test env so that it's easier to see the changes.

I think visually the changes make a lot of sense. If I could ask you to change one thing that would help a lot - we are moving towards sentence case labels, and some slip - I can see that one of the checkboxes "Show Value" is still title case. Can you make it sentence too?

In terms of changes of the labels in x/y axis sections I'll wait for @yousoph to take a look to make sure, that it's all correct.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2025

@kasiazjc Ephemeral environment spinning up at http://54.188.60.228:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

@yousoph
Copy link
Member

yousoph commented Sep 8, 2025

hi @SBIN2010 ! Thanks for these additional customizations!

A few suggestions on wording:

  • The section header: Series setting -> Series settings
  • Above the color labels, add the word color: Increase -> Increase color, Decrease -> Decrease color, Total -> Total color
  • Tooltip for colors:
    • Choose the color for series increase -> Select the color used for values that indicate an increase in the chart
    • Choose the color for series decrease -> Select the color used for values that indicate an increase in the chart
    • Choose the color for series total -> Select the color used for values that represent total bars in the chart
  • Tooltip for Labels:
    • Change the increase label to text -> Customize the label displayed for increasing values in the chart tooltips and legend
    • Change the decrease label to text -> Customize the label displayed for decreasing values in the chart tooltips and legend
    • Change the total label to text -> Customize the label displayed for total values in the chart tooltips, legend, and chart axis.

@SBIN2010
Copy link
Contributor Author

SBIN2010 commented Sep 8, 2025

Hi @yousoph! Thanks for the suggestions!
Perhaps such a hint for labels will be misleading? The scope is not only in the hint, but also in the legend, and for the total, in the labels on the X axis.

@yousoph
Copy link
Member

yousoph commented Sep 15, 2025

Ah, thanks for the feedback - I didn't enable a legend so I missed those spots. I updated my previous comment, does the updated text capture it more accurately now?

@SBIN2010
Copy link
Contributor Author

Ah, thanks for the feedback - I didn't enable a legend so I missed those spots. I updated my previous comment, does the updated text capture it more accurately now?

Yes, now it looks good, thanks.

@SBIN2010
Copy link
Contributor Author

@michael-s-molina @stephenLYZ
Do you have any further comments on this PR?

Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @SBIN2010!

@michael-s-molina michael-s-molina merged commit 3e55467 into apache:master Sep 17, 2025
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugins 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR size/L viz:charts:echarts Related to Echarts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants