Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

feat: add line-bar chart from nvd3 #39

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thunter009
Copy link

🏆 Enhancements

This PR adds the line-bar chart from the nvd3 library. See example below:

image

Inspired by:

apache/superset#5144

Related issues from the main superset repo:

apache/superset#5530

Reference PR on primary repo:
apache/superset#7216

@kristw
Copy link
Collaborator

kristw commented Apr 17, 2019

Thank you for your contribution. However, we are inclining on not adding new chart types to the current packages at the moment, especially when it introduces more complication on top of nvd3 legacy vis. That file has too many charts combined plus additional complexity due to annotations, etc. and became really difficult to maintain. Recently we have tried to fix many open bugs and fixing one chart ends up affecting another.

@GGPay
Copy link

GGPay commented Aug 20, 2019

Hello @kristw and @thunter009.

I really appreciate for your contribution to superset. I do understand that new chart introduce more complication and difficulties in supporting.
Would you be so kind to tell:

  1. Do you have any plan how and when you gonna add new charts to master?
  2. @thunter009 Did you add line-bar chart to your version? If yes - could you explain a little bit how?

Thank you guys.

@thunter009
Copy link
Author

Hello @kristw and @thunter009.

I really appreciate for your contribution to superset. I do understand that new chart introduce more complication and difficulties in supporting.
Would you be so kind to tell:

1. Do you have any plan how and when you gonna add new charts to master?

2. @thunter009 Did you add line-bar chart to your version? If yes - could you explain a little bit how?

Thank you guys.

The implementation is in these two branches:

https://github.com/thunter009/incubator-superset/tree/line_bar
https://github.com/thunter009/superset-ui-plugins/tree/feature/line_bar

Happy to walk you through them.

@kristw
Copy link
Collaborator

kristw commented Aug 22, 2019

I am working on a way for the plugin to be able to define control panel in itself. This way you can publish your own plugin as an npm package and enable the plugin in your own instance of Superset without adding the chart to master and affect everyone. That should wrap up end of Q3.

@thunter009
Copy link
Author

I am working on a way for the plugin to be able to define control panel in itself. This way you can publish your own plugin as an npm package and enable the plugin in your own instance of Superset without adding the chart to master and affect everyone. That should wrap up end of Q3.

Sounds good -- if you need help porting legacy charts to the new preset/plugin system, let me know. In addition to the nvd3 line-bar implementation, I have some additional feature improvements to the current legacy nvd3 bubble chart I've been maintaining in a private fork that I'd like to commit back once the new chart plugin system is implemented.

@john-sandall
Copy link

john-sandall commented Dec 31, 2019

Thank you for adding this, it's greatly appreciated. What's the easiest way of getting this working with our Superset install?

As I understand it, this won't be merged as the intention is going forwards for community-contributed plugins to be published independently to npm? But this fork is currently unpublished I think?

@thunter009
Copy link
Author

Thank you for adding this, it's greatly appreciated. What's the easiest way of getting this working with our Superset install?

As I understand it, this won't be merged as the intention is going forwards for community-contributed plugins to be published independently to npm? But this fork is currently unpublished I think?

@john-sandall Apologies for the tardy reply. The easiest way is to fork both repos (main superset and superset plugins) and point your main superset fork's package.json to use the local fork of superset-ui-plugins instead of pulling from global npm.

It's a bit hacky (I also combine the main superset repo, superset-ui, and superset-ui-plugins into one big repo for ease of use) but it's done the trick so far. See this debugging guide for some further details:

https://github.com/apache-superset/superset-ui/blob/master/docs/debugging.md

@wuxuedaifu
Copy link

hello, thunter009, is it possible to plot several bars ( more than 2 bars )mixed with lines( more than 2 lines) in ONE chart using your code?
Thank you.

@thunter009
Copy link
Author

@wuxuedaifu no it is currently not possible. We're working on integrating echarts for more advanced charting in this respect

1 similar comment
@thunter009
Copy link
Author

@wuxuedaifu no it is currently not possible. We're working on integrating echarts for more advanced charting in this respect

nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
Since it's 100% it seems worthwhile to add ;)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants