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

Ordinal bar chart bars overlap and ticks don't align in 2.0 #533

Closed
bhatup opened this issue Mar 3, 2014 · 13 comments
Closed

Ordinal bar chart bars overlap and ticks don't align in 2.0 #533

bhatup opened this issue Mar 3, 2014 · 13 comments
Milestone

Comments

@bhatup
Copy link

bhatup commented Mar 3, 2014

Hi,

There is issue is composite chart.
There is no proper gap between 2 bars (though specified with .gap(10)). And ticks are not in center of bars.

barchartoverlapping and x-axis ticks align issue

@gordonwoodhull
Copy link
Contributor

Again, @bhatup, would you please link to a short self-contained example?

@bhatup
Copy link
Author

bhatup commented Mar 3, 2014

Sorry Gordon,

Please look into this example.
http://jsfiddle.net/av5nj/

@gordonwoodhull
Copy link
Contributor

Thanks, that is very helpful, and a very simple example which should work.

I can confirm this is a regression in 2.0.0, possibly due to the new work on ordinal bar charts.

I simplified your example to a simple bar chart, and it still fails:
http://jsfiddle.net/gordonwoodhull/fWhhz/

A mostly-equivalent version using 1.6.0 works:
http://jsfiddle.net/gordonwoodhull/B77Ut/16/

I'm changing the issue title accordingly.

@bhatup
Copy link
Author

bhatup commented Mar 3, 2014

Hello @gordonwoodhull,

I have resolved this issue by doing modification in calculateBarWidth() function in dc.js
bugfixinordinal case

Just added the statement:
numberOfBars = (_chart.isOrdinal()) ? numberOfBars + 1 : numberOfBars;

This statement has been added in dc.1.6.js

@gordonwoodhull
Copy link
Contributor

Okay, thanks. Looks like that line got lost in
03cf450 Use RangeBands for Ordinal Coordinate Grid Charts
and I verified your fix locally.

I'm wary of just pushing the fix, since the changes in that commit look intentional. Let's see if we can summon @jrideout...

@gordonwoodhull
Copy link
Contributor

I determined that this is not the right fix. It helps but it just errs the other way. Something is deeply messed up about bar widths. Plan to fix for 2.0

@gordonwoodhull gordonwoodhull added this to the v2.0 milestone Jun 5, 2014
@umdjm
Copy link

umdjm commented Jul 20, 2014

Issue raised again at http://stackoverflow.com/questions/24848550/bar-overlapping-in-dc-bar-chart

"Bar overlapping in DC bar chart"

@gordonwoodhull
Copy link
Contributor

Thanks for linking. Yep this code is seriously screwed up. Maybe I'll hit this next.

Also: #593 changing number of bars.

@gordonwoodhull
Copy link
Contributor

I think this is possibly the worst bug in dc right now. Just frightening results, yuk.

@mr23
Copy link
Contributor

mr23 commented Jul 20, 2014

I agree its at the top. Just had to work around it Friday.

On July 20, 2014 4:10:52 PM CDT, Gordon Woodhull notifications@github.com wrote:

I think this is possibly the worst bug in dc right now. Just
frightening results, yuk.


Reply to this email directly or view it on GitHub:
#533 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@gordonwoodhull
Copy link
Contributor

PRs always welcome.

My preference is to ditch the custom logic, or use it only when .gap is specified, and otherwise always do this the d3 way, with rangeBands. I think that will scale to grouped bars better, too.

@gordonwoodhull
Copy link
Contributor

As a workaround, use .barPadding, which disables .gap and uses the d3 bar allocation algorithms instead. I'm still going to fix this, because unfortunately a gap of 2 is currently the default, and the d3 methods don't have a way to set a pixel gap, only a percentage.

@gordonwoodhull
Copy link
Contributor

next time around, use rangeBounds for all bar charts unless _gap is set

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

No branches or pull requests

4 participants