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

Y range reset on synchronize #1011

Open
davidporter opened this issue Jul 6, 2021 · 3 comments
Open

Y range reset on synchronize #1011

davidporter opened this issue Jul 6, 2021 · 3 comments

Comments

@davidporter
Copy link

davidporter commented Jul 6, 2021

https://dygraphs.com/tests/synchronize.html
When deselecting the checkbox to unsync the Y axis; the graphs all reset when a new zoom is made. The Y-axis also zooms out on every selection in another graph. I would expect the Y-axis to stay the same when another graph is zoomed but the graphs just reset and redraw.

Shouldn't the behavoir be: when a graph is zoomed in the y-direction and another graph is zoomed in the x-direction, only the x-direction is updated in all of the sync graphs and leave the y-zoom alone?

@mirabilos
Copy link
Collaborator

Okay, this is interesting. I just had a look at this.

There’s nothing in the synchroniser that resets the y zoom (value range) of the graphs.

This is an artefact of the behaviour of dygraphs to change the value range whenever you do something, whether that is zooming the y axis, panning the graph along its X axis (something that I personally found rather annoying as well) or via updateOptions.

I think what happens here is:

  • in the source graph, you zoom (x axis) or double-click
  • this is replicated to the destination graphs
  • there, dygraphs considers this an action “like panning the graph” and therefore recalculates the value range

I think we can only “fix” this by making it keep the value range set when manually set by zooming in. (Which I personally think would be a nice thing to have, but @danvk might be able to comment on why this is not currently done.) This involves first changing the code to keep track of whether the value range was set by explicit user action (y zoom) or not, as in the latter case you’d want it to automatically update.

So, I fear this is not something for v2.2.1 but rather for a v2.3.0 or so…

What do you think?

@danvk
Copy link
Owner

danvk commented Feb 7, 2023

The current behavior is that performing an x-zoom/pan will adapt the y-range to reflect the new data. I think this is desirable, e.g. if you're panning forward in time in an "up and to the right" graph. If you ever do a y-zoom (click and drag directly upwards) then you enter "2d pan" mode and you're in full control.

You can see this in the demo if you uncheck "y-axis" and then do a vertical zoom one one of the charts. Its y-axis will now be out of sync with the others.

@mirabilos what exactly are you proposing to change? I do thing the zoom-to-content feature when you're panning the x-axis is desirable and I wouldn't want to change that. The interaction between this behavior and ranges that are explicitly set via options is murky and has changed in the past (see the release notes / breaking changes for dygraphs 2.0.0 and the old isZoomedIgnoreProgrammaticZoom option (removed in #812).

@mirabilos
Copy link
Collaborator

Yes, I agree it’s desirable as default, but it throws off quick visual comparison (I’d not see it’s constantly “up and to the right” because it resets my viewpoint all the time).

2D pan mode is great (I learnt about its existence only today). I think some way of enabling that after a horizontal zoom might be useful, but will have to sleep on that as to how. Maybe if some other modifier is pressed in addition to Shift, but… we’ll see.

The problem in this issue is that the y axis does not stay out of sync: after disabling y-axis synchronisation and zooming vertically in one of the diagrams, zoom horizontally in another (or, if having zoomed in horizontally, double-click to zoom out); the first diagram will lose its value range. Finding a good solution to that is trickier than it sounds.

mirabilos added a commit that referenced this issue Feb 7, 2023
is most likely (part of) the #1011 cause

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

No branches or pull requests

3 participants