search

chrvadala / react-svg-pan-zoom

:eyes: A React component that adds pan and zoom features to SVG
https://chrvadala.github.io/react-svg-pan-zoom/
MIT License
684 stars 126 forks source link

Added option to align svg while fitting it to the viewer (#65) #120

Closed ghost closed 6 years ago

ghost commented 6 years ago

Alignment can be passed as optional props SVGAlignX and SVGAlignY to ReactSVGPanZoom and Toolbar components; or via arguments to the base fitToViewer function. (What is passed to ReactSVGPanZoom will also be passed to the toolbar.)

Default alignment values are provided - aligning to top left, as before.

I will need the ability to fit and center svgs in one of my projects, so I thought I might as well write a bit more general solution for left/center/top horizontal alignment and top/center/bottom vertical alignment..

ghost commented 6 years ago

The prop names are probably misleading, now that I think of it. For ReactSVGPanZoom they may suggest that svg will be aligned when first rendered - not only when fitToViewer is called explicitly. Perhaps that would actually be desirable to have an option to align svgs, even without fitting them?

chrvadala commented 6 years ago

Thanks for your PR. It's a really useful feature and will close this issue https://github.com/chrvadala/react-svg-pan-zoom/issues/65

I agree that is better to renames these two new props, because after the change of the method fitToViewer() to fitToViewer(SVGAlignX, SVGAlignY) you will have that only the toolbar will consume these values.

I think that we cold introduce a more general property that is toolbarProps Thought?

ghost commented 6 years ago

Oh sure, it's better to have fitToViewer(SVGAlignX, SVGAlignY) than just fitToViewer() - and so toolbarProps would totally make sense. I could change both things, but I'm not sure what other props you would like to have in toolbarProps.

BTW, thanks for the awesome project! - really useful! :smiley:

chrvadala commented 6 years ago

You are right, probably there're other props that they would be better moved into toolbarProps, but move them breaks the current API and I think that is something that we have to avoid. For now we can just move these two new props (SVGAlignY, SVGAlignX) in toolbarProps and keep the other things unchanged. In the future I will release a new major release and I'll move also the remaining part. LMK Don't forget to add your name in README.md ;-)

ghost commented 6 years ago

Right, obviously it's better to defer moving other props till the next major release. Only SVG alignment in the toolbarProps then!

chrvadala commented 6 years ago

Thanks for your PR. I'm going to release with 2.18 version

chrvadala commented 6 years ago

Feature released with 2.18. Thanks ;)