Other charts not updating properly when using brush in series chart.

[RenaudF]

It's all in the title. See http://jsfiddle.net/RFontana/ShdEv/

[mtraynham]

Just encountered this. dc,compositeChart does not handle the _chart._brushing function correctly, nor does it call _chart.redrawGroup().

Should be changed to:

    _chart._brushing = function () {
        var extent = _chart.extendBrush();
        var rangedFilter = null;
        if(!_chart.brushIsEmpty(extent)) rangedFilter = dc.filters.RangedFilter(extent[0], extent[1]);

        dc.events.trigger(function () {
            for (var i = 0; i < _children.length; ++i) {
                if (!rangedFilter) _children[i].filter(null);
                else _children[i].replaceFilter(rangedFilter);
            }
            _chart.redrawGroup();
        }, dc.constants.EVENT_DELAY);
    };

[jrideout]

@mtraynham Would you mind submitting a patch with your proposed solution?

CC/ @matthull

[mtraynham]

@jrideout Seems that my changes didn't particularly work that well. I think the idea, more or less, is to filter the parent chart and not delegate any filters to the children. Because the filtering is always against the dimension, and the children share the dimension, we should just do it once in the parent. The child charts should be updated to reflect the dimension filter after the fact.

        _chart._brushing = function () {
            var extent = _chart.extendBrush();
            var rangedFilter = null;
            if(!_chart.brushIsEmpty(extent)) {
                rangedFilter = dc.filters.RangedFilter(extent[0], extent[1]);
            }

            dc.events.trigger(function () {
                if (!rangedFilter) {
                    _chart.filter(null);
                } else {
                    _chart.replaceFilter(rangedFilter);
                }
                _chart.redrawGroup();
            }, dc.constants.EVENT_DELAY);
        };

This is still incomplete, as there needs to be a way to let the children redraw filtered values accordingly.

[amergin]

Is this the same problem I have described in https://groups.google.com/forum/#!topic/dc-js-user-group/yI6_cbvgfbU?

[mtraynham]

I added a partial fix in the pull request, which fixes composite chart directly based on these fiddles:
Before:
http://jsfiddle.net/cBgkT/1/

After:
http://jsfiddle.net/cBgkT/1/

There is still something wrong with the way series chart is doing things though, as the same change fixes the brushing for Series charts, but incorrectly filters them. Seen in OP's fiddle after update:
http://jsfiddle.net/fnQk9/

[amergin]

Is there any progress on this? I don't see the fix being incorporated to master branch, probably because it failed a unit test.

I'm surprised this hasn't got a higher priority -- at least in my use-case the dc.js chart interplay is crippled by this bug (no filtering at all that would show up on the other charts). This bugfix does fix it.

[gordonwoodhull]

There isn't an active PR for this. @mtraynham, is it just because you meant to split it out into another PR, as you cited in the earlier one, or because of the doubts you express above?

Either way, I agree it's a serious problem. Note that the series charts have never officially been released; they're a 2.0 feature and there are a whole slew of bugs I dearly want to fix before releasing 2.0.

[mtraynham]

@gordonwoodhull I didn't want a partial fix preventing Bug #497 so I closed the PR and created Pull Request #555. I never did get back to this bug and validate why @RenaudF's fiddle still had some issues. Granted, I was pretty new to dc.js and didn't really know the ramifications of what I was changing.

Either way, I believe compositeChart should only delegate the filter to the parent instead of every child since they share the dimension (as shown in the code snippet above). That was one of the reasons for the weirdness. If I get some time today (or the next few days; kinda under water atm) I'll put together a fiddle showing the issues with seriesChart that don't work currently, but also don't work with my change applied.

[vprus]

Just wanted to ask whether work is happening on this? I've run into this issue, and the proposed fix did not really work for me. Is the fix expected in near future, or shall I try to sort it out myself - not that expect I'd be able to.

[gordonwoodhull]

This is near the top of my list when I get back in September. @mtraynham's fix seems like it should work but I haven't looked closely. More test cases and examples (in the form of fiddles or jasmine test cases) would be very welcome!

[gordonwoodhull]

Although, probably deep concentration is required more than more examples. :-(

[vprus]

In my case, I have a combination of bar chart and line chart (I don't have particularly good explanation why, it just feels best to me). I've noticed that with the current patch, the areas of line chart outside the filter area are not dimmed - which is probably something to check. I'll try to create a standalone example, but it might take unpredictable time, unfortunately.

[gordonwoodhull]

Related: #706 #677

[jcamins]

From mtraynham:

Either way, I believe compositeChart should only delegate the filter to the parent instead of every child since they share the dimension (as shown in the code snippet above). That was one of the reasons for the weirdness. If I get some time today (or the next few days; kinda under water atm) I'll put together a fiddle showing the issues with seriesChart that don't work currently, but also don't work with my change applied.

I don't necessarily disagree. However, I'm actually making use of the fact that the filter is on child rather than the parent, so how strongly would you object to my delegating the parent's filter to the children?

[gordonwoodhull]

It seems to me that "in reality" the parent and children all share one filter - it's one dimension and that dimension has a single filter - and it shouldn't be a choice one way or the other. Is there some way we could add a simple level of indirection so that we don't have to worry about this?

In a way this is also related to #682. I wonder if that could benefit from some indirection as well.

[gordonwoodhull]

(Not asking you to come up with the most general solution, a quick fix would be welcome for 2.0 - but in the long run I'd like to do this right. It may also be an issue for small multiples, grouped bars, etc etc.)

[jcamins]

This is definitely an issue for grouped bar charts. That's what I am doing. For me the largest problem is the lack of a filterAll, but I'd delegate both filter() and filterAll() to the children for an immediate fix.

[jcamins]

Also replaceFilter().

Of course, this hasn't been tested yet, I'm just making plans.

[gordonwoodhull]

You mean, to the first child? That sounds reasonable.

[jcamins]

The same filter has to be applied to each child. So my thinking is to iterate through the children and apply it to each.

[mtraynham]

@jcamins I think I overlooked your use case, but do you need to call filter on each child chart because they may have a different dimension? I've never used composite directly, series only expects you use a single dimension... so when I found this line, it surprised me:
https://github.com/dc-js/dc.js/blob/master/src/composite-chart.js#L61

It looks like composite already loops it's children and passes the filter:
https://github.com/dc-js/dc.js/blob/master/src/composite-chart.js#L74

[jcamins]

@mtraynham Same dimension, same group for all the bar charts. I'm applying the filter to each subchart because filtering the dimension directly doesn't affect charts of that dimension, just charts of other dimensions. My use case is filtering programmatically rather than just through the selection brush.

[gordonwoodhull]

I think that if #682 is generalized in the way I suggest there, it will solve this. It is basically the same problem: make sure that a "sync group" of charts which share a dimension, have their .filters() updated when any of them sets the filter.

@jcamins, @mtraynham, do you agree?

(It would even make sense to make it automatic for all charts sharing a dimension, as @vprus suggested, but I don't know how to do that cleanly.)

[jcamins]

@gordonwoodhull At first glance, that makes sense. I'll take a closer look when I can.

[gordonwoodhull]

Fixed by #1408 in 3.0.2