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

Separate Zoom for X-Y axis #176

Open
jeremyVignelles opened this issue Dec 15, 2015 · 10 comments
Open

Separate Zoom for X-Y axis #176

jeremyVignelles opened this issue Dec 15, 2015 · 10 comments

Comments

@jeremyVignelles
Copy link
Contributor

Hi,

I'd like to have separate X/Y scales when zooming.
In other words, I'd like to return {x: true, y:false} from beforeZoom, as this is done in beforePan.

I have two use cases in my project:

  • A timeline, where you can chose a date. If the timeline is horizontal, you don't have to scale the y axis, but there is a need to scale the x axis
  • A line chart, where i'd like to have different scales on my two axis.

Implementation details:

  • Add an option named enableSeparateZooms, default: false.
  • If this option is set, each zoom-related options handles {x:, y:}-like zooms, if not set, the current behavior is kept
  • Add a method panZoomToRect({x,y,width,height}) to pan and zoom to a specific viewbox, potentially with different x/y zoom factors. width or height may be ommited (but not both), in which case the current scale ratio is kept

What do you think about the idea? Is there anything else to add ? I can try to make the PR if you want.

@bumbu
Copy link
Owner

bumbu commented Dec 16, 2015

For now I would change all zoom methods to accept a second (optional argument). Or maybe accept as a scale not only a number but also an object such as {x: 1, y: 2} (thus we'll keep the compatibility.

Having that functionality will allow to write a custom panZoomToRect method that shouldn't be in the core (but maybe as a plugin as per #98).

And yes, a pull request is very welcome : )

@bumbu bumbu changed the title [feature-proposal] Separate Zoom X-Y Separate Zoom fox X-Y axis Dec 16, 2015
@jeremyVignelles
Copy link
Contributor Author

I'm trying to implement this, but I have some questions:

  • I'm doing separate methods getZooms, get/computeRelativeZooms that returns {x:,y:} objects. If enableSeparateZooms is not set, the two values will be identical. Question : what will be the behavior of the old methods if the enableSeparateZooms options is set? Should they throw an Error?(my choice until you tell me)
  • In processCTM, what will be the behavior of fit and contain if the option enableSeparateZooms is set? Should we use the same behavior and give the same X and Y zoom ?

I'll keep in touch if I have other questions. Merry christmas :)

Edit: I just renamed enableSeparateZooms to separateZoomsEnabled for consistency

@bumbu
Copy link
Owner

bumbu commented Dec 25, 2015

  • If enableSeparateZooms is set and old getZoom is called - we can simply return X zoom but show a warning in console. Throwing an error may make sense so that people will not get in trouble, but this may break existing solutions so I would go with something that still works (even it not perfectly).
  • fit and center will have to be updated to work as intended. If for example X scale is 1 and Y is 2 then fit should take this into account and fit by Y axis if that Y axis is proportionally bigger than X in relation to SVG container.

But I'll think more about this during the holidays and I'll let you to know it anything else comes to my mind. Merry Christmas!

@ariutta ariutta mentioned this issue Dec 28, 2015
34 tasks
jeremyVignelles added a commit to jeremyVignelles/svg-pan-zoom that referenced this issue Jan 3, 2016
@jeremyVignelles jeremyVignelles changed the title Separate Zoom fox X-Y axis Separate Zoom for X-Y axis Jan 3, 2016
@jeremyVignelles
Copy link
Contributor Author

There's a work-in-progress at jeremyVignelles@60eb591
Methods are implemented, but I didn't have much time for manual tests, automated tests nor documentation yet.

If you have time to give it a try, this would be helpful. Otherwise, I will use this feature at work in the following days so I will test a few things at that time.

@bumbu
Copy link
Owner

bumbu commented Jan 3, 2016

Nice work. It looks really good.

I'm currently working on #98 and I do plan to add separate zooms into it. There are many changes in core but I may use pieces of your code for that.

But I'm still thinking if we need an option to enable separate zooms or it should be a default thing. Currently I'm inclined towards having it as a default for v4. But in the same time it should be easy to use the library for SVGs that don't require separate zooms. I'll let you to know on my progress on this progress.

@jeremyVignelles
Copy link
Contributor Author

I agree that it would be better if enabled by default, but I will need this feature pretty soon, this is why I implemented it as an option, for backward compatibility. This compatibility can be dropped in v4 since this is a major release.

Do you plan to introduce this feature in v3 or wait for v4?

@bumbu
Copy link
Owner

bumbu commented Jan 3, 2016

If your work works well and has tests then I'll merge it into v3.
v4 is pretty big in term of internal changes so it may take some time to bring it to a stable release. And your change can be useful to other people much sooner : )

bumbu added a commit that referenced this issue Jan 10, 2016
@smimon
Copy link

smimon commented Feb 15, 2018

Forgive me for digging up an old thread, but I was wondering if you had any suggestions on how to implement the panZoomToRect function, on top of this branch?

I'm trying my best to backwards engineer it, but being unfamiliar with the project (and SVG g transforms in general) it's slow going.

Thanks

@sancelot
Copy link

sancelot commented Apr 4, 2019

I would need this feature for a gantt view and angular.
So, I setted up the plugins branch using
npm install --save svg-pan-zoom#plugins
But unfortunately now, the implementation does not work anymore .

I was loading the library as follow and it has not worked anymore:

import * as svgPanZoom from 'svg-pan-zoom';
scheme : svgPanZoom.Instance;
this.scheme = svgPanZoom(....

@sancelot
Copy link

sancelot commented Apr 5, 2019

Ok, I noticed that typescript is not available in plugins branch, I will try to make it available.

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

No branches or pull requests

4 participants