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

Fixes StaticCanvas.onBeforeScaleRotate not triggering when canvas is zoomed #4748

Merged
merged 2 commits into from
Feb 20, 2018

Conversation

scriptspry
Copy link
Contributor

Fixes #4746

@asturur
Copy link
Member

asturur commented Feb 20, 2018

Cool!
You should remove the changes to dist before pushing

try with

git checkout master dist

and then commit/push again

@asturur
Copy link
Member

asturur commented Feb 20, 2018

Would be nice to add a small test

@asturur
Copy link
Member

asturur commented Feb 20, 2018

If there are no test examples with event generation i ll help, but i think there is something around.

@scriptspry
Copy link
Contributor Author

Took me an hour to figure out where, how and what to test. Let me know if there are any changes. Will do it tomorrow. First time took me a lot of time. Shouldn't take long next time.

@scriptspry
Copy link
Contributor Author

I have another doubt: shouldn't onBeforeScaleRotate be member of Canvas instead of StaticCanvas?

@asturur
Copy link
Member

asturur commented Feb 20, 2018

definetely. but do not worry to change it now, is a potentially dangerous change for which may be test missing.


canvas.zoomToPoint({ x: 0, y: 0 }, 1);
canvas.onBeforeScaleRotate = _onBeforeScaleRotate;
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect test.

@asturur
Copy link
Member

asturur commented Feb 20, 2018

Is good, thanks for the test, is extensive and well written.

@asturur asturur merged commit d569407 into fabricjs:master Feb 20, 2018
thiagocunha pushed a commit to thiagocunha/fabric.js that referenced this pull request Nov 18, 2019
…zoomed (fabricjs#4748)

* Fixed StaticCanvas.onBeforeScaleRotate not getting triggered when canvas is zoomed.

* Added test for Canvas._beforeTransform.
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

Successfully merging this pull request may close these issues.

2 participants