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

Why not drawing dots when brushOn #1152

Closed
tornord opened this issue Jun 1, 2016 · 5 comments
Closed

Why not drawing dots when brushOn #1152

tornord opened this issue Jun 1, 2016 · 5 comments

Comments

@tornord
Copy link

tornord commented Jun 1, 2016

I'll like to remove the brushOn criteria in drawDots, line 5891 in dc.js. Why is the test there?

function drawDots (chartBody, layers) {
    if (!_chart.brushOn() && _chart.xyTipsOn()) {
@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Jun 1, 2016

I think that is there because the tooltip / title / popup function of the dots will interfere with the brush.

Do you want the tips or just the dots?

@tornord
Copy link
Author

tornord commented Jun 1, 2016

I understand. I want the dots and skip tips when brushOn.

Comment out brushOn check gives me that feature

function drawDots (chartBody, layers) {
if (/_!chart.brushOn() &&/ _chart.xyTipsOn()) {

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Jun 1, 2016

Links to previous discussion.

The current functionality was implemented somewhat backward from how you might expect, for historical reasons, see #735. Previously the dots always appeared when the brush was off, xyTipsOn disables them also then.

#689 discussed this feature, of forcing the dots always on when xyTipsOn is set. @mtraynham mentions a workaround for composite charts.

I'm not sure, but I think that changing the flag to a three-way switch could be a backward-compatible solution. Default is 'auto', turn on or off depending on brush. False always disables the dots. True always enables them (and disables crosshairs and tips when the brush is on). This assumes that no one is setting the flag true currently.

Alternately, even more paranoidly backward compatible, true is auto and 'always' enables them always.

@tornord
Copy link
Author

tornord commented Jun 2, 2016

Thanks for a very detailed answer. I'm using D3/DC/Crossfilter more and more and hope I also will contribute

@gordonwoodhull gordonwoodhull added this to the v2.0 milestone Jun 14, 2016
@gordonwoodhull
Copy link
Contributor

I see now. There is no issue with the tips interfering with the brush, because the events don't get through.

But I'm going to pull the @dalle fix from https://github.com/Kurappu/dc.js/tree/mks_patches for this, which implements the paranoid solution I described above, because xyTipsOn() defaults to true and I don't want to change the current behavior in 2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants