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

Add skewX/Y methods to API #25

Closed
wants to merge 1 commit into from
Closed

Add skewX/Y methods to API #25

wants to merge 1 commit into from

Conversation

jamesots
Copy link
Contributor

SVG defines six transformations: translate, rotate, scale, matrix, skewX and skewY. I thought it would be sensible and useful to have all of these in the API.

@puzrin
Copy link
Member

puzrin commented Sep 11, 2016

This module is old/mature and nobody still said that it lacks of SkewX/Y. I prefer to keep APIs clear & simple and avoid adding everything possible without practical reasons. Do you have ones?

Those rare operations are still available via .matrix() / .transform(), and i don't know why it worth to pin those at top level.

@jamesots
Copy link
Contributor Author

I was writing some code and tried to call skewX, assuming it would be in the API as it's an SVG transformation, but it wasn't there. I initially copied the code out of the Matrix class, but then thought it would be better not to duplicate code but to make the API complete instead, and have all six SVG transformations available.

If you think this is a bad idea for some reason, I can just use my fork, I guess.

@puzrin
Copy link
Member

puzrin commented Sep 11, 2016

Ok, let's accept it, but please add tests first (copy from transform) & squash to single commit.

@puzrin puzrin changed the title Add skewX/Y methods to API, and add typescript definitions Add skewX/Y methods to API Sep 11, 2016
@puzrin
Copy link
Member

puzrin commented Sep 11, 2016

Added by 44f5c8e.

I can release it, if you don't plan to PR anything else (typescript definition?).

@puzrin puzrin closed this Sep 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants