search

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

Concept of `autoFocus`, range/focus moved to the compat layer #1790

Closed kum-deepak closed 3 years ago

kum-deepak commented 3 years ago

After #1785, the code specific to range/focus charts had reduced drastically. It is actually possible to create a conf flag autoFocus that will cause the chart to focus if a filter is specified.

This is not exact earlier functionality, however, quite close - the earlier code was also checking the domain of the other chart and constraining based on that.

The multi-focus example now works without custom code.

I will not commit other examples as of now - those are just to test.

kum-deepak commented 3 years ago

Merged main commits, the example would be updated further and merged later.

gordonwoodhull commented 3 years ago

Sorry I haven’t had time to review!

I’m not comfortable with dropping half of the range-focus feature, but I haven’t had time to investigate why you made this choice.

Seems like you would end up in an inconsistent state if you zoomed an autoFocus chart? Where the domain does not match the filter, but the brush is not displayed so the difference is not visible.

But I haven’t tried it, so I might be missing something.

kum-deepak commented 3 years ago

Please see #1792, I have restored to the PR. For some reason, GitHub was not updating this PR properly.

Please review commit https://github.com/dc-js/dc.js/pull/1792/commits/5303a0470241b74c41e57223553f0ab686b59196, I believe there is only one change with functional significance has been carried out. I have put a comment there for you to look at.

Please review this change.