-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Grouped and Stacked Bars2 #1459
base: develop
Are you sure you want to change the base?
Conversation
Thanks @jaklub! I have some ideas about a cleaner, easier-to-maintain implementation, but you are right that porting this to v3 is the first step. For now, could you please add an example of grouped+stacked bars? The grouped-bar example is very helpful but it would be great to see the whole thing in action. |
Also, if you're up for it, could you also enable brushing and put another chart into the example? Let's start a discussion about how interaction should work. I tried enabling brushes in the original plunkr in the previous PR, and it looks like brushing does not work on the grouped&stacked chart:
https://plnkr.co/edit/3q8pK4NGHYw6h7Rck7t0?p=preview It looks like you may have done some additional work in this area? I think this can be made to work, although it would have to round the brush so that a whole group is selected at once, not just individual bars. |
Hey @gordonwoodhull! I was using one of the earlier grouped barchart PRs, but I kind of wanted to use the new shiny dc v3 :). So I tried to cook up something that worked (at least for me). But I must admit, I mostly considered my own use cases when creating this PR. Didn't for instance try to make it work with both grouped and stacked bars in the same chart as in the plunkr of the previous PR. Is that something that you would like to be working? Tried quickly with brushing also, but don't think it worked. I can look into it bit more. Added one more example though, with two grouped and filtering barcharts. And yes, I did work some on the select/deselect portion of the code. It kind of works for my use cases, but it dosen't feel clean. I did add a few lines of code that overrides the click event for instance, feels a bit like a hack. I am a bit of a beginner with this library (and d3), so please point out my newbie mistakes :). And by the way thanks for all work with dc, its a great lib. |
@Frozenlock I've made a jsfiddle https://jsfiddle.net/jaklub/8dcsmbuk/ could you try to fork and modify to reproduce your problem? |
Hmm... I get an error about .x(d3.scaleLinear().domain([0, 9]))
.xUnits(function () {return 10;}) Did I forget something? |
@Frozenlock I've updated the grouped barchart to be able to handle non ordinal charts. Also found small mistake in the fiddle when using the non ordinal bar chart. dim = ndx.dimension(function(d) {return "Run " + d.Run;}), => dim = ndx.dimension(function(d) {return d.Run;}), https://jsfiddle.net/jaklub/hnaumer2/5/ @gordonwoodhull: Played around with the brush, it seems to be working somewhat now. Added a new example with three charts, one with the brush on. |
Hi @jaklub - awesome, thanks for following through and providing these fixes. The examples looks very good. It strikes me that we might want to change the ordinal-grouped hover and click behavior. Since you always get the whole group rather than an individual bar, it seems to me that
These are just ideas for future work. My main concern with merging this remains the way the chart reaches into its parent the composite chart and its siblings, in order to decide how to draw. I feel that this could be clearer and easier to maintain, but I don't yet have a concrete plan. Until then, thanks for bringing this PR into the new world and making it available for everyone to try out! |
@jaklub Now it works as expected. Thank you! |
@gordonwoodhull - I Agree that would be nice, to have the groups behave more like a group when hovering and selecting. In my opinion, this implementation makes the "physical" grouping of the bars a bit warped from how you might perceive them as a developer or user. I mean each "bar group" contains one bar from each child in the composite. This approach might also make it more difficult to implement something like you are suggesting above (and implementing #1428 which also sounds like a great idea). And I guess this also forces the charts to reach out to its siblings. I was looking through some of the earlier grouped bar chart versions. Think I was using the @gazal-k branch from #1043 earlier myself. Do you think that would be a better approach? To use stacking and an extra property like groupBars? Would of course make it more challenging to implement grouped and stacked bars in the same chart (don't know how requested that feature is though). Or do you have any other thoughts on how to implement this feature in an more maintainable way? Would love to see grouped bar charts, in one form or another, eventually make its way to the master branch :). |
One of the arguments from the previous PR that made me prefer this approach, is that you can switch between a composite of line charts and stacked bars with mostly the same code. What's missing, I think, is a class that sits between the composite chart and the bar chart and says "you are part of a group". This class would figure out the offsets of the bars within their groups, and could also mediate the mouse events we're talking about here. Then the offset is just another property of the bar chart and hopefully it doesn't have to look at its siblings. I'm glad you mentioned #1428 - that's exactly what I was thinking for "clicking on the group". |
@gordonwoodhull I've been using the dependency
in my project. It was the dependency you had suggested in issue #1356 . But recently when I went to build my project I got an NPM warning
That tells me that NPM was looking for a specific commit, but that perhaps the hash tag #grouped-stacked is no longer valid(?). If that is in fact the case, what is the correct dependency to use? If possible, I would like to know both the dependency for the grouped-stacked PR based on dc v2.1.x and the grouped-stacked PR based on dc v3.0.x. |
Hi @grugknuckle, I wasn't able to repro that with [email protected], node@10. When I use the above line in package.json it installs a version of dc.js with the header /*!
* dc 2.1.10-groupstack
* http://dc-js.github.io/dc.js/
* Copyright 2012-2016 Nick Zhu & the dc.js Developers
* https://github.com/dc-js/dc.js/blob/master/AUTHORS which is what I expect. |
Ok ... Thank you @gordonwoodhull . Apologies for polluting this PR with my troubleshooting problems. |
No problem, @grugknuckle, it's good to be aware of these issues. I would just check that it's installing what you expect and not worry about it unless it really is dropping the branch info. For dc.js 3.0 compatibility, I also pushed a branch for this PR; you should be able to use: "dc": "git://github.com/dc-js/dc.js.git#grouped-stacked-3", This will install /*!
* dc 3.0.5-groupstack
* http://dc-js.github.io/dc.js/
* Copyright 2012-2016 Nick Zhu & the dc.js Developers
* https://github.com/dc-js/dc.js/blob/master/AUTHORS |
Excellent ! Thanks @gordonwoodhull . |
Would it be possible to update the grouped branch with the newest release? |
Okay, the branch
Sigh, really hope to resolve this deadlock soon. I am not completely happy with either solution but these sure are useful features!!! |
Thank you very much! |
Hi all, I'm playing with 3.0.8-groupstack and it works very well. I've just a question. I have a dashboard with a lot of other charts and one of this is a piechart with the values that the groupchart is using for his x axis. Example It's that correct? |
Hi @dadokkio, that has nothing to do with this chart. It's because a group does not observe its own dimension's filters. If you want two charts to filter each other, they need to be on different dimensions. |
Hello @gordonwoodhull, Have you made a decision as to which implementation should be merged into master? |
I dislike both implementations for different reasons. I am currently of the opinion that stacking needs to be expanded to multiple dimensions, but I haven't tried it. Sorry for the disappointment. Maybe someone should fork and maintain their own version of this. |
Updated @OivalfMarques grouped-stack-bar-chart #1356 to work with version 3 of dc.