dc-js / dc.js

Multi-Dimensional charting built to work natively with crossfilter rendered with d3.js
Apache License 2.0
7.41k stars 1.81k forks source link

Maintain filter information with dimensions #1777

Closed kum-deepak closed 3 years ago

kum-deepak commented 3 years ago

Approach:

gordonwoodhull commented 3 years ago

Earlier, #681 / #682

I agree it's messy to attach this to the dimension and could cause issues if the backend is not crossfilter. We could use Map, which allows the identity of objects as keys. So, all charts whose dimensions are === would share a filter list.

Otherwise users would have to declare which filter group to use, which is redundant and might be a source of bugs. That's how earlier proposals worked.

What if you wanted most of your dimensions' charts to have linked filters, but there was one dimension represented by two charts that you don't want linked, or that have incompatible filter types? Could you opt a dimension out?

kum-deepak commented 3 years ago

Thanks for the idea to use Map.

I actually did a simplistic implementation. It starts showing issues. I am moving code to anchor to the beginning of the constructor. This will allow us to use anchorName early on.

I will merge generic changes that allow thinner commits for the actual change. This will take time and iteration, but that way will not block other activities.

One thing I already noticed that filter changed notification needs to be sent if the dimension received a filter due to activity in another chart that shares the same dimension.

I also think we will need to provide mechanisms to include/exclude specific charts from these mechanisms.

There are also simpler questions:

I have sense for answers to these questions, however, will like to userstand cases where the above do not hold true. That will help in designing the change.

kum-deepak commented 3 years ago

I have read #681 / #682, the current approach is quite similar in concept to the final suggested solution. I have the following plan:

In addition code for range/filter will need to be reviewd and updated as needed.

Composite chrat code may also need changes.

gordonwoodhull commented 3 years ago

I started to respond yesterday, then stalled out because I know I have seen cases where unshared dimension make sense for composite and range/focus but couldn't remember what. I think it's stuff like

I use a composite key in order to split a dimension multiple ways, and then a fake group to remove one of the parts of the key. Then I have another dimension which uses just the kept part of the first dimension's key. So there are two dimensions but filtering is equivalent across them.

Defaulting to shareFilters but allowing a flag to escape makes sense, and I think it should cover these cases fine.

I think you are referring to my proof of concept in the sync-filter branch - the big idea was to separate concerns in .filter() so that

FWIW it looks like the relevant commit was 522adb6. Glad to hear that you independently derived much the same result!

kum-deepak commented 3 years ago

Thanks, it all makes sense. Actually I did not entirely come to this solution on my own. It was over your work :+1:

kum-deepak commented 3 years ago

I will assume that range/filter charts will always share dimension.

gordonwoodhull commented 3 years ago

Again, I think there are valid reasons to have different but equivalent dimensions for range/focus via fake groups. The code doesn't anticipate this, so it probably requires filter handlers.

So I think your assumption is valid, but not always true. If that makes sense. 😄

IMO there are too many assumptions with range/focus, like the way that both charts will apply the same filter, and the way mouseZoomable is tied to the focus chart, causing #1772. It's a demo that became a feature without conscious design.

But I digress. The relevant point is that decoupling and separation of concerns are badly needed here

kum-deepak commented 3 years ago

Implemented in #1785.