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

Fix: CircleControls transparentCorners styling #7015

Merged
merged 2 commits into from
Apr 30, 2021

Conversation

Kronoxis
Copy link
Contributor

CircleControls didn't listen to the transparentCorners styling of the fabricObject.

Steps to reproduce

You can verify this issue in the controls customization demo: http://fabricjs.com/controls-customization

  1. Set cornerStyle to option 2 (circular)
  2. Toggle transparentCorners on

CircleControls now render transparent when the transparentCorners style is set on the fabricObject
@asturur
Copy link
Member

asturur commented Apr 17, 2021

Hi, thanks for the PR.
One visual test around circle coners is indeed failing, and this is expected.

Could you please have a look at why is failing and fix it?

Do you know how to run the test suite locally?

@Kronoxis
Copy link
Contributor Author

Could you please have a look at why is failing and fix it?

I'm assuming the test image controls4.png is wrong, as this has solid circle controls while transparent are used in the test code. How do I go about re-rendering this image so I can provide the repo with the correct test image?

Do you know how to run the test suite locally?

I'm not familiar with writing and running tests in JavaScript, could you guide me through how I can do this? So far I have cloned the repo on my machine and installed the npm dependencies.

@asturur
Copy link
Member

asturur commented Apr 17, 2021

if you are past npm install, you should be able to do the following:

npm run build:fast will build fabric.js with your fix
npm run test:visual will run the tests and give you a single failure
once you are sure that the old image was wrong and now with the new code needs an updated, you can search it and delete the image.
run again the tests with npm run test:visual and the test won't find the image and create it.
before committing the changes, you should remove the built dist file with git checkout master dist
then the usual git add . and git commit -am 'corrected the snapshot' and you should be done.

The image for `circle corners green` of the visual tests is now correctly rendered with transparent corners
@Kronoxis
Copy link
Contributor Author

I have uploaded the new controls4.png image and the visual unit test succeeds now.

@asturur asturur merged commit 3a6029a into fabricjs:master Apr 30, 2021
@asturur
Copy link
Member

asturur commented Apr 30, 2021

Thanks!

@asturur asturur mentioned this pull request May 22, 2021
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