Revert "Merge pull request #137 from ramirezcgn/master"

georgemincof commented 2 years ago

Please revert #137

The behavior of #137 causes an SVG that is wider than its height to shrink.

It's a breaking change. Consider adding this under a new option.

Until the 10.0.5 version, this does not happen.

This reverts commit be4d9baa8c724344d51c3c502cce334570063b54, reversing changes made to eb26c81075907783f175253d6ce5a1d700d72aa7.

Fixes #

Proposed changes

reverts commit be4d9baa8c724344d51c3c502cce334570063b54

Code quality

[x] I made some tests for my changes
[ ] I added my name in the contributors field of the package.json file

License

To get your contribution merged, you must check the following.

[x] I read the project license in the LICENSE file
[x] I agree with publishing under this project license

Join

[ ] I wish to join the core team
[ ] I agree that with great powers comes responsibilities
[ ] I'm a nice person

My NPM username:

nfroidure commented 2 years ago

cc @ramirezcgn WDYT?

ramirezcgn commented 2 years ago

Hi, I made this change to achieve the same behavior as other tools like IcoMoon looking for icon size uniformity, and the problem was precisely that if all the icons were 16x16 (for example) and there was one 30x16, that icon did not respect the aspect ratio of the other icons, I think a better solution would be to add one more option to configure that.

Regards

georgemincof commented 2 years ago

I updated the PR. If you want the default value for the "preserveAspectRatio" option to be true, preferably increase the major version of the package. As I said, the previous change was a breaking change.

Feel free to change the name of the option if it's not intuitive. Thanks!

coveralls commented 2 years ago

Coverage remained the same at 57.868% when pulling 04f72d8bfda3a69f436bde60c473a4cdf39e68c0 on georgemincof:revert-#137 into 7a31efb5500faeeae32d635ed0b0281733b120c8 on nfroidure:master.

nfroidure commented 2 years ago

I did some mistakes with versions but the v12.0.0 includes your changes @georgemincof you should safely be able to upgrade to it while the one that satisfied with the breaking change will have to add the option to upgrade.

nfroidure / svgicons2svgfont