search

heavysixer / d4

A friendly reusable charts DSL for D3
MIT License
432 stars 46 forks source link

Dual-axis capability breaks when using groupedColumnSeries #36

Closed yanofsky closed 9 years ago

yanofsky commented 9 years ago

http://jsfiddle.net/q16eqjg4/

It looks like the y attributes are correct, but their height attribute is not because it is set by the chart y()instead of the custom scale set to it.

Possibly due to this line? https://github.com/heavysixer/d4/blob/master/src/features/grouped-column-series.js#L42

heavysixer commented 9 years ago

yes I think the error goes even further back because: https://github.com/heavysixer/d4/blob/master/src/features/grouped-column-series.js#L82-L97

assumes you'll only have a dimension of x or y. We probably need to set this to use the scaleId or yScale accessor like we did in the previous fix.

https://github.com/heavysixer/d4/blob/master/src/features/x-axis.js#L97-L99

nsonnad commented 9 years ago

Hey @heavysixer, in case you haven't gathered yet @yanofsky and I are working together and using d4 in our project. Have you started working on the dual-axis groupedColumnSeries issue? Would you like us to have a go at it?

heavysixer commented 9 years ago

hey @nsonnad i figured you two were in cahoots some how given the branch and fork tree. I would love for you to take a crack at it, I've been slammed with other work this week. I am happy to help you suss out where this bug is exactly, and I don't think it should take too long once you get the feature pointed at the correct scale I just don't have time to look at it yet.

Any help is greatly appreciated :) !

nsonnad commented 9 years ago

Have a PR for this, #37

heavysixer commented 9 years ago

@yanofsky @nsonnad thanks again for this PR. One quick question, i know you are working on a project using D4, is it now or will it be open to the public? I would very much like to see what the two of you have cooked up.

yanofsky commented 9 years ago

yup!

We should be open sourcing it in the next few weeks