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

heatMap.filter breaks usage with multiple filter elements #1515

Closed
ghost opened this issue Feb 19, 2019 · 6 comments
Closed

heatMap.filter breaks usage with multiple filter elements #1515

ghost opened this issue Feb 19, 2019 · 6 comments

Comments

@ghost
Copy link

ghost commented Feb 19, 2019

DC-Version: 3.0.11
D3-Version: 5.5.0
Crossfilter: 1.4.6
latest Firefox

I tried to use the function replaceFilter for heatmap.
It only works if i select only one value, but on 2 or more it fails.

There is something weired in dc.filters.TwoDimensionalFilter -> f.isFiltered:

  • If I click on a box, both "value" an "f "are a single array containg x&y.
  • If I select a value from external "f "is an array of arrays containig x&y. So it shouldn't work, but it works on a single value => ?????
    .... anyways I'v rewritten the f.isFiltered to accept "f" beeing an array of arrays, but it didn't work (of course Ive tested if it returns true).

=> it looks like that the result of f.isFiltered is totally ignored. Maybe its used for the clicks on the boxes, but definatly not for replaceFilters()

@gordonwoodhull
Copy link
Contributor

Right, the filter format is a bit weird, taking an extra level of array from what you might expect.

It is documented here:
http://dc-js.github.io/dc.js/docs/html/dc.baseMixin.html#filter

dc.filters.TwoDimensionalFilter only accepts one two dimensional location. When multiple items are selected (including rows or columns), dc.heatMap creates one of these objects for each cell selected. Then the base chart creates a filterFunction which accepts any row which matches any of the filters.

You might want to look at the heatmap source to understand what filter objects it is creating. It is not a lot of code.

If you continue to run into trouble, please create a reproducible example (eg a jsfiddle, bl.ock, or observable notebook) and I'll be glad to help. But I think you are probably just missing a pair of square brackets. :)

@ghost
Copy link
Author

ghost commented Feb 19, 2019

No, I'm not missing a pair of square brackets!
I've made a JQuery-Plugin which creates a select2-Dropdown and handels the communication with the chart. I've works perfect with rowChart and geoChoroplethChart and yes I've made it compatbile for using dimensions with an array.

I'v tried everything .... with, without those brackets, JSON.stringify or cast the array to a string, even tried if the filter is matching the instances of the array and not their values ....

Its definatly the not working
https://jsfiddle.net/f3gchz8q/11/
Instead of a dropdown I've made 2 buttons
.... you can add/remove the brackets @ line 58 if yu want to -> no effect ... not working for the multiselect one!
And if there would be an other chart connected by crossfilter ... nothing matches to this dimension -> everthing is grey

@gordonwoodhull
Copy link
Contributor

Thanks for the repro. I'll take a look! I am sure there is a way to get it to work, but it might not be obvious.

@gordonwoodhull
Copy link
Contributor

Hi @koallalays. My apologies for the misunderstanding.

dc.heatMap is indeed doing some fishy things in its filter handlers - it looks like someone intentionally overrode .filter() in order to deal with the case with a single coordinate, but this breaks the case where multiple coordinates are passed in.

Related issue: #532

Here is the workaround for your fiddle:

  • convert the array of coordinates to TwoDimensionalFilters
  • use _filter instead of filter of replaceFilter
  • call _filter(null)._filter([fil]) in order to reset and replace the filters.

In code:

    fil = fil.map(c => dc.filters.TwoDimensionalFilter(c));
    chart._filter(null)._filter([fil]);

Here is my fork of your fiddle: https://jsfiddle.net/gordonwoodhull/dL5myjc3/15/

If you are having issues with other charts not driving the heatmap correctly, I think that is probably a crossfilter setup issue. As you can see in the heatmap filtering example, it definitely is possible to filter using another chart.

I doubt it is related, but feel free to open another issue or reply with another fiddle if you are still having trouble with that.

I haven't looked into it deeply, but I think the way to fix the current issue would be to override .filter() in a less stupid way that properly handles all the cases that baseChart.filter does. Or perhaps remove this overload, since it seems like the user should just pass a TwoDimensionalFilter as they do for other charts like the brushed-ranged charts.

    dc.override(_chart, 'filter', function (filter) {
        if (!arguments.length) {
            return _chart._filter();
        }
        return _chart._filter(dc.filters.TwoDimensionalFilter(filter));
    });

I'll retitle this accordingly.

@gordonwoodhull gordonwoodhull changed the title replaceFilter doesnt work @ heatmap heatMap.filter breaks usage with multiple filter elements Feb 19, 2019
@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Mar 22, 2019

fixed in 3.0.12 by deprecating heatmap.filter taking a raw coordinate - all chart filter functions should take filter objects or arrays of filter objects.

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Mar 27, 2019

Now I see that scatterPlot does exactly the same thing, and there it is necessary currently because the compositeChart will explicitly filter each of its children with the RangedFilter, which needs to be cast to a RangedTwoDimensionalFilter for the 2D keys... This class in turn defaults Ys to -Infinity and Infinity if not present.

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

1 participant