search

novus / nvd3

A reusable charting library written in d3.js
http://nvd3.org/
Other
7.22k stars 2.15k forks source link

One chart and multiple svgs #1024

Open RenatoUtsch opened 9 years ago

RenatoUtsch commented 9 years ago

I was inspecting this code: https://github.com/novus/nvd3/blob/development/src/models/scatter.js#L58

And I noticed that it creates a chart for each svg that is in the selection. This allows me to do something like this: http://jsfiddle.net/rnp7xb7p/3/

I create two chart svgs with the same chart object. You can notice that the charts get all messed up because of this. Parts of the chart work on the first one and parts on the second one.

This shouldn't be allowed to happen, should it? Maybe the selection.each() should only consider the first svg of the selection and ignore the rest.

What do you guys think?

RenatoUtsch commented 9 years ago

I can make a pull request that fixes this on all charts.

liquidpele commented 9 years ago

Might look at that much later on again, but I didn't modify that because the data and foreach expects the passed data object to be an array so I didn't want to mess with possibly breaking all nvd3 installs by redoing how the data is passed in. People can just not try to do that, so it's not really hurting anything :)

RenatoUtsch commented 9 years ago

Maybe removing the foreach and getting just the selection[0]? Would this cause problems? It is a bit hacky though, but better than the current situation.

liquidpele commented 9 years ago

I wonder if novus is using that loop for anything... @robinfhu @joshjordan thoughts?

RenatoUtsch commented 9 years ago

If novus is then whatever they are doing is completely broken for a long time.

liquidpele commented 9 years ago

Perhaps, but this isn't a bug so much as just a design change so we should wait for them to chime in.