Skip to content

Commit

Permalink
more specific test for filter function
Browse files Browse the repository at this point in the history 
for #990
@esjewett, I think this is a slightly safer way to check for a custom
filter function such as those in dc.filters
  • Loading branch information
@gordonwoodhull
gordonwoodhull committed Sep 18, 2015
1 parent e66c536 commit 4d62f65
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/base-mixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ dc.baseMixin = function (_chart) {
var _filterHandler = function (dimension, filters) {
if (filters.length === 0) {
dimension.filter(null);
} else if (filters.length === 1 && !Array.isArray(filters[0])) {
} else if (filters.length === 1 && !filters[0].isFiltered) {
// single value and not a function-based filter
dimension.filterExact(filters[0]);
} else {
dimension.filterFunction(function (d) {
Expand Down

2 comments on commit 4d62f65

@esjewett
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, seems so. Previous approach would have broken if the dimension was made up of arrays?

@gordonwoodhull
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's more about clarity - AFAICS either way should detect the current filters fine.

The only potential bug I imagined for the previous approach is if a filter was not an array. All the ones in dc.filters are annotated arrays. However, in theory a custom filter could be any kind of object that supports isFiltered and filterType, and the fact that the current ones are arrays seems like an accident.

I added some documentation around this.

Please sign in to comment.