Skip to content
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

add bar chart support to echarts and stylo-pie-table #6

Merged
merged 2 commits into from
Nov 12, 2021
Merged

Conversation

EvilDrW
Copy link

@EvilDrW EvilDrW commented Nov 10, 2021

🏆 Enhancements
implemented rudimentary bar chart in echarts (because i wasn't able to use the nvd3 bar chart from within stylo-pie-table). expanded stylo-pie-table to have a choice of using bar or pie charts for drilldown-to-table.

Copy link

@followbl followbl left a comment

Choose a reason for hiding this comment

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

I think the one thing we should just make a determination on is if we're going to be doing named vs default exports - named is usually better for debugging and stack traces - but obviously we should stick to a convention as a team

plugins/plugin-chart-echarts/src/Bar/buildQuery.ts Outdated Show resolved Hide resolved
@EvilDrW
Copy link
Author

EvilDrW commented Nov 11, 2021

I think the one thing we should just make a determination on is if we're going to be doing named vs default exports - named is usually better for debugging and stack traces - but obviously we should stick to a convention as a team

i don't have an opinion on this. i used default because it seemed to be very common in the superset-ui code. if you'd like me to change it, just say the word.

@EvilDrW EvilDrW merged commit a43888e into master Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants