Overhaul zoomable behavior

[sclevine]

See: https://github.com/NickQiZhu/dc.js/wiki/Zoom-Behaviors-Combined-with-Brush-and-Range-Chart

Original Issue:

Steps to reproduce:

1. Load http://nickqizhu.github.io/dc.js/
2. Select a range using the range chart below the graph titled "Monthly Index Abs Move & Volume/500,000 Chart"
3. Attempt to pan the graph by clicking and dragging it

Expected:
The graph should pan normally.

Observed behavior:
See screenshot below. I've tested this using both Firefox 23.0.1 and Chrome 29.0.1547.57 on OS X 10.8.4.

If a range is selected by zooming in using the mouse wheel (to the same effect as selecting a range using the range chart), panning does not appear to be broken.

[jrideout]

@sclevine Thanks for the report! This seems to be ok on the latest master. Can you please confirm if this is still a valid issue?

[sclevine]

Thanks for taking a look at this!

The version of dc.js on http://nickqizhu.github.io/dc.js/ still appears to have this problem. I can still replicate the screenshot shown above by following the steps described. I can also replicate this on my own graphs with dc.js from the latest master.

To replicate this on the http://nickqizhu.github.io/dc.js/ page, make sure you select a small range using the range chart, and then attempt to pan the graph by clicking and dragging on the graph itself (not on the range chart below it).

[sclevine]

Here's a video of this issue on the example page:
http://youtu.be/8822mJ-Zvm4

We're experiencing this behavior with our own graphs as well.

[jrideout]

@sclevine thanks for the detail on this. I can replicate it. I'll try to dig into the problem sometime in the next week or two unless you or someone else beats me to it.

[jrideout]

@sclevine Would you mind reviewing #347?

[sclevine]

#347 looks fine to me. We built and tested the #347 branch and it seems to fix the strange behavior Matt describes. It is not directly related to the above issue, as far as we can tell.

We were able to temporarily fix this issue by calling chart.render() on filter events (in lieu of chart.redraw()), although I'm still not sure what causes it for the example page.

[matthull]

I think this and #347 may have the same root cause - how drag events are handled when d3 brush and zoom behavior are both enabled.

I'll take a look at this one over the weekend. I'm no d3 guru, but maybe can make some sense of it and update #347 to fix this as well.

[matthull]

It seems that this issue relates to https://github.com/mbostock/d3/wiki/Zoom-Behavior#wiki-x - specifically the need to call zoom.x() again if x domain or range is modified programatically. When you zoom in with mouse wheel, it's OK because d3.behavior.zoom itself is manipulating the x domain/range. But with range chart dc.js is modifying x of another chart programatically.

A working fix I've tested is to have the focusChart() "filtered" event listener call enableMouseZoom() so that zoom.x() gets updated each time range chart is manipulated. However seems like the real solution to prevent similar future issues is to somehow observe changes to chart.x() and respond by updating behavior.zoom.x().

@jrideout: If you have no objections I'll update #347 with a fix for this since the solution is related even though root cause really isn't. That can then be either merged in or at least referenced as working example of fix.

[jrideout]

@matthull I'm fine with you combining both changes in a single PR. Also If you could rebase against master when you are ready, that will help with code review.

[jrideout]

@matthull Also take a look at d3/d3#1299

[matthull]

@jrideout Seems like a good idea to have the zoom out restriction logic leverage d3/d3#1299 somehow (maybe monkey patch until that PR gets merged?)

Seems there's a broader issue here that goes beyond the range chart+brush+zoom issue documented in #253. For instance, right now you can use mouse wheel to zoom out to a multi-century date range in the stock example's monthly move chart despite zoomOutRestrict(true).

[jrideout]

Seems there's a broader issue

@matthull Agreed, the whole thing needs a close inspection and overhaul. Perhaps we should start by documenting the current, and agreeing on the expected, range-chart+zoom+brush behavior (on a wiki page) ? That might give a clear way to explore the possible remedies.

[matthull]

@jrideout Yeah there is a lot going on here. Created https://github.com/NickQiZhu/dc.js/wiki/Zoom-Behaviors-Combined-with-Brush-and-Range-Chart with my current impressions.

[jrideout]

@matthull That is fantastic work!. I just gave it a cursory glance, but it seems to capture what we need to work the kinks out. Many thanks for pulling it together. I will dive into the details sometime soon.

CC @sclevine @NickQiZhu @gordonwoodhull When you get a chance, can you guys take a look at Matt's great rundown of our current zoom/brushing/range behaviors? In particular, if we can gain a consensus on the Proposed sections that should give us a clear spec going forward.

[NickQiZhu]

Agree! Great analysis and sensible solutions to this problem, thanks
@matthull

On Tue, Oct 22, 2013 at 4:08 PM, Jacob Rideout notifications@github.comwrote:

@matthull https://github.com/matthull That is fantastic work!. I just
gave it a cursory glance, but it seems to capture what we need to work the
kinks out. Many thanks for pulling it together. I will dive into the
details sometime soon.

CC @sclevine https://github.com/sclevine @NickQiZhuhttps://github.com/NickQiZhu
@gordonwoodhull https://github.com/gordonwoodhull When you get a
chance, can you guys take a look at Matt's great rundown of our current
zoom/brushing/range behaviorshttps://github.com/NickQiZhu/dc.js/wiki/Zoom-Behaviors-Combined-with-Brush-and-Range-Chart?
In particular, if we can gain a consensus on the Proposed sections that
should give us a clear spec going forward.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/253#issuecomment-26840106
.