-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
feat(plugin-chart-echarts): support horizontal bar chart #19918
feat(plugin-chart-echarts): support horizontal bar chart #19918
Conversation
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx
Outdated
Show resolved
Hide resolved
…Regular/Bar/controlPanel.tsx Co-authored-by: Evan Rusackas <evan@preset.io>
I suspect this is one of the most anticipated PRs ever, love it! ❤️ |
/testenv up |
@rusackas Ephemeral environment spinning up at http://18.236.159.61:8080. Credentials are |
I think there are two little problems that need to be addressed in this PR. The small nitpicky one: And the trickier one: Thoughts @stephenLYZ or @villebro ? |
/testenv up FEATURE_GENERIC_CHART_AXES=true |
@rusackas Ephemeral environment spinning up at http://52.33.242.130:8080. Credentials are |
What my last comment didn't say is that this is AWESOME. Works great in |
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.
This is great work! A few small improvement proposals, other than that LGTM.
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
Outdated
Show resolved
Hide resolved
…transformProps.ts Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
…transformProps.ts Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
…Regular/Bar/controlPanel.tsx Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
@rusackas Yeah. That makes sense! The reason that I prefer to make a whole section for Bar orientation is to have all the following x/y control swaps (include "Chart Title" section). Maybe we can implement it by dynamic control. |
Codecov Report
@@ Coverage Diff @@
## master #19918 +/- ##
==========================================
+ Coverage 66.22% 66.52% +0.30%
==========================================
Files 1715 1716 +1
Lines 64179 65164 +985
Branches 6753 6775 +22
==========================================
+ Hits 42501 43350 +849
- Misses 19960 20091 +131
- Partials 1718 1723 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up FEATURE_GENERIC_CHART_AXES=true |
@stephenLYZ Ephemeral environment spinning up at http://34.216.73.213:8080. Credentials are |
/testenv up FEATURE_GENERIC_CHART_AXES=true |
@stephenLYZ Ephemeral environment spinning up at http://18.236.109.218:8080. Credentials are |
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.
The community has needed this feature for a long time, Thanks for addressing this!
I totally agree, the label name of
The Bar chart transposition is a circumstance that a mixed query fragment and viz definition.
Due to this confusing reason, lots of BI tools use |
@rusackas @zhaoyongjie That's a good input and currently it's based on the ability of the dynamic control to rotate - you can see that after setting the chart orientation, all x/y controls swap. Let's merge this one and improve this in the next PR. |
Ephemeral environment shutdown and build artifacts deleted. |
* feat(plugin-chart-echarts): support horizontal bar chart * Update superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx Co-authored-by: Evan Rusackas <evan@preset.io> * Update superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> * Update superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> * Update superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> * improve controlpanel * default value * fix ut Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
Will this also work without time-series? Just with the regular bar chart? I just want to do a horizontal bar chart without time column... I'm asking here because I don't have acess to the superset version or upgrading it right now, with Superset 2.0 will I be able to do this? |
I also wondered the same. I am trying to do it without time-series now |
@Vicinius Yeah. You can see this PR supports both Time-series and Caterical bar chart. |
Maybe I'm dense (entirely possible), but I can't get it to work. I have the GENERIC_CHART_AXES feature set in superset_config.py. I'm using docker compose from near top of master, built the docker image to incorporate some other changes. Can't get the v2 bar to function any differently. What am I missing? |
@redwards101 see if this helps you #5059 (comment) |
Thanks, but that's how I got here. There's clearly something I'm not doing right. Just don't know what it is yet. Looking for anyone who has it working in a docker environment. |
Ok, it's working now. I don't know why. I brought up the container again and suddenly I am now seeing the X-AXIS field that wasn't there before. The only notable recent difference is I decided to just set the DEFAULT_FEATURE_FLAGS in config.py and rebuild the image. I really can't see that as a reason, though. Maybe something was cached? Anyway, it's working great now. |
@redwards101 were you able to sort the bars? that was a complaint on the comment that I pointed above |
No. Can't control the axis sorting. |
@stephenLYZ Thank you so much for this feature. I can see the feature for the new V2 time series but the "normal" bar doesn't give option to switch orientation. Also, I'm not seeing a "normal" bar chart v2 in the latest superset version. Am I missing something ? |
|
@Vicinius I saw your question, and I thought the same thing. |
Hey @Hugh-Guan and @Vicinius , I'm not sure if you've been following the "generic x-axis" development work recently, but the "Time-series line/bar/scatter" etc charts from the ECharts chart plugin nowadays support using any x-axis type if you enable the |
@villebro First of all, I am very sorry for taking so long to reply. Then, thank you very much for your advice and help. |
SUMMARY
This PR supports horizontal bar chart for the new bar EChart. Here we switch these two bar orients (vertical and horizontal) by adding a
Bar Orient
control.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Time series
Vertical
Horizontal
Caterical
Vertical
Horizontal
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION