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

Separate groups for pie chart slices and labels #1167

Closed
wants to merge 1 commit into from

Conversation

maackle
Copy link
Contributor

@maackle maackle commented Jun 16, 2016

Adds another layer of g, one for each of slices and labels, to ensure the latter always appear after the former in the DOM

Fixes #664

@hanneshapke
Copy link

+1

@gordonwoodhull
Copy link
Contributor

Great, thanks @maackle!

@patrickbeeson
Copy link

👍

@gordonwoodhull
Copy link
Contributor

Hmm, the pointer-events solution isn't quite right, because the labels can also be external and a handy way to click on smaller slices. I'll see if I can find a better solution for that part, indeed it's annoying to have the highlight flash on and off as you pass over the labels (and the lines connecting to exterior labels).

gordonwoodhull added a commit that referenced this pull request Jul 22, 2016
for #1167
replaces 0a35ef6, which would have disabled clicking on exterior labels
@XaserAcheron
Copy link
Contributor

I think the pointer-events thing is tangential to the issue at hand anyway. I have such a thing in my own CSS but it's a pretty easy DIY whereas the slice layering isn't.

@maackle
Copy link
Contributor Author

maackle commented Jul 22, 2016

You're right, I'll remove that commit

@gordonwoodhull
Copy link
Contributor

I've already merged this to my working branch with a replacement (highlighting the slice when the corresponding label or path is hovered - the commit is referenced above). But I ran into problems writing the tests so it's not released yet.

gordonwoodhull added a commit that referenced this pull request Jul 25, 2016
for #1167
but wow! the test dynamically fails to compile due to dc.transition
returning a selection if duration===0
gordonwoodhull added a commit that referenced this pull request Jul 25, 2016
for #1167
apparently this is the only place where our use of dc.selection
is risky, because we don't use the *tween functions very much

places where this problem is currently avoided:
- lineChart has .ease commented-out
- numberDisplay does not use dc.transition
@gordonwoodhull gordonwoodhull modified the milestone: v2.0 Jul 27, 2016
gordonwoodhull added a commit that referenced this pull request Jul 28, 2016
for #1167
replaces 0a35ef6, which would have disabled clicking on exterior labels
gordonwoodhull added a commit that referenced this pull request Jul 28, 2016
for #1167
but wow! the test dynamically fails to compile due to dc.transition
returning a selection if duration===0
gordonwoodhull added a commit that referenced this pull request Jul 28, 2016
for #1167
apparently this is the only place where our use of dc.selection
is risky, because we don't use the *tween functions very much

places where this problem is currently avoided:
- lineChart has .ease commented-out
- numberDisplay does not use dc.transition
@gordonwoodhull
Copy link
Contributor

Rebased and merged in 2.0 beta 32, along with handlers highlighting the slice when a label or path is hovered. Thanks @maackle!

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

Successfully merging this pull request may close these issues.

5 participants