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

Drag zoom loosing data points #52

Closed
jordanpapaleo opened this issue Nov 2, 2016 · 15 comments · Fixed by #161
Closed

Drag zoom loosing data points #52

jordanpapaleo opened this issue Nov 2, 2016 · 15 comments · Fixed by #161
Labels

Comments

@jordanpapaleo
Copy link

Hi

I am working on an implementation of the zoom plugin using the drag feature. I am following the samples/zoom-time.html example. When i drag a zoom area, the resulting line chart looses data points. This happens in both my chart and the example chart.

Drag:

screen shot 2016-11-01 at 4 56 34 pm 2

Result:

screen shot 2016-11-01 at 4 56 40 pm

As you can see the first data point is gone and the resulting chart is off too. The result in mine is even more extreme with more of a flattening line. It kind of looks like the chart data is averaged instead of zoomed.

Here is the zoom config I am using:

zoom: {
  enabled: true,
  drag: true,
  mode: 'x',
  limits: {
    max: 10,
    min: 0.5
  }
}

limits was in the example but I do not see in in chart.zoom.js

I'd love some help here :)

Thanks,

Jordan

@etimberg etimberg added the bug label Nov 2, 2016
@etimberg
Copy link
Member

etimberg commented Nov 2, 2016

I think the flattening is coming from how the bezier control points are calculated. We'll need to do some investigation to figure out why the points are lost. My likely theory is that the zoom is incorrectly calculated and the first point is now outside the new axis minimum

@marco-silva0000
Copy link

hey. You have any progress on this? is there somewhere you may need help? My team has this bug as well and if we could we would like to help it be solved asap

@etimberg
Copy link
Member

etimberg commented Jan 6, 2017

Hey, i haven't looked into this more yet. Do you have an example that reproduces for debugging?

@marco-silva0000
Copy link

marco-silva0000 commented Jan 6, 2017 via email

@marco-silva0000
Copy link

do you need any help moving this forward?

@marco-silva0000
Copy link

any progress on this?

@ghost
Copy link

ghost commented Oct 9, 2017

Same issue here.

@CalvinChhour
Copy link

Noticing this issue as well.

@jonathanface
Copy link

jonathanface commented Nov 7, 2017

I have put together a fiddle to illustrate this issue: https://jsfiddle.net/opr9rgo6/

In my case it occurs within a time chart - not sure if the issue is specific to time or not, but I see the first cited example is also a time chart.

To see the issue dragover a section which includes the first 2 data points like so:
prezoom

You will see the zoomed content does not include the first data point:
postzoom

Also, for this fiddle I used chart bundle 2.5, since zoom doesn't work at all with more recent versions.

@Skuhoo
Copy link

Skuhoo commented Jan 21, 2018

Any progress on this? I tried looking into it awhile back but had no luck.

@benmccann
Copy link
Collaborator

Have you tried again with the latest version 0.6.2 which has many fixes included?

@Skuhoo
Copy link

Skuhoo commented Jan 23, 2018

@benmccann Just tried the latest version and the issue is still there, unchanged.

The issue is that the zoomed scale it calculates from the selection is incorrect unless the center of the selection is at the center of the chart. The farther away the center of the selection is from the center of the chart, the more inaccurate the zoomed scale becomes.

@benmccann
Copy link
Collaborator

Ok. I've been trying to merge PRs with fixes, but there's no one actively fixing bugs in this library. So really anyone who wants this fixed will have to send a PR

@chriser-
Copy link

The usage of the zoom functions when using the drag selection is pretty stupid, there is already a good function to get the value at a pixel. I refactored the mouse up handler, and it works perfectly now with a time graph. I haven't tested it with non-time graphs, but it should work exactly the same.

     chartInstance.zoom._mouseUpHandler = function(event){
        if (chartInstance.zoom._dragZoomStart) {
          helpers.each(chartInstance.scales, function(scale, id) {
            if(scale.isHorizontal()) {
              var beginPoint = chartInstance.zoom._dragZoomStart;
              var offsetX = beginPoint.target.getBoundingClientRect().left;
              var startX = Math.min(beginPoint.clientX, event.clientX) - offsetX;
              var endX = Math.max(beginPoint.clientX, event.clientX) - offsetX;
              var limitObject = scale.options.type == 'time' ? scale.options.time : scale.options.ticks;
              limitObject.min = scale.getValueForPixel(startX);
              limitObject.max = scale.getValueForPixel(endX);
            }
          });
          chartInstance.zoom._dragZoomStart = null;
          chartInstance.zoom._dragZoomEnd = null;
        }
      };

If someone wants to submit a PR or fix it better, feel free to do so.

@JoshAudibert
Copy link

I'm only using time graphs for drag-zoom, and @chriser-'s solution works great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants