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

Why is the strokeUniform variable remarked in the toObject function? #6641

Closed
joyk1130 opened this issue Oct 9, 2020 · 8 comments · Fixed by #6772
Closed

Why is the strokeUniform variable remarked in the toObject function? #6641

joyk1130 opened this issue Oct 9, 2020 · 8 comments · Fixed by #6772

Comments

@joyk1130
Copy link

joyk1130 commented Oct 9, 2020

I set the strokeUniform variable to true to fix the stroke size when increasing or decreasing the object scale.
When I use the clone function, the stroke result is different.
The strokeUniform value is not copied when creating a clone.

If you look at the fabric.js file, the strokeUniform value is remarked in the toObject function, but I wonder why!

This is my example :
http://jsfiddle.net/Nicky_Jo/zb6Lrvod/

@asturur
Copy link
Member

asturur commented Oct 9, 2020

Unclear how to use the fiddle.

What do you mean by remarked?

@joyk1130
Copy link
Author

joyk1130 commented Oct 9, 2020

image

I mean this line.
I use fabric 4.1.0 version.

This is my fiddle how to use.

  1. Click a orange color rectangle.
  2. scaleup select object.
  3. ctrl+c -> orange rectangle
  4. ctrl+v

Result:
image

@asturur
Copy link
Member

asturur commented Oct 10, 2020

that line shouldn't be commented. Is wrong. Should be fixed.

@stale
Copy link

stale bot commented Oct 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue marked as stale by the stale bot label Oct 24, 2020
@stale stale bot closed this as completed Oct 31, 2020
@jaltekruse
Copy link

@asturur I'm interested in getting this fixed. I see the TODO comment was added/removed in these PRs, but I'm not sure that it addressed any/all possible problems that were the reason it was commented originally.

added - https://github.com/fabricjs/fabric.js/pull/6118/files
TODO removed - commented line remains - https://github.com/fabricjs/fabric.js/pull/6280/files

Could you point me at some areas to test/fix to enable this to be safely uncommented? Happy to contribute fixes, just a pointer in the right direction would be helpful.

@asturur
Copy link
Member

asturur commented Dec 31, 2020

I think the issue was just me have a terrible test suite and need to update all the things manually.
Let's do that and let's get this thing solved.

@asturur asturur reopened this Dec 31, 2020
@stale stale bot removed the stale Issue marked as stale by the stale bot label Dec 31, 2020
@jaltekruse
Copy link

Got it, there are probably just tests that have hard-coded exports saved, and those need to be updated? Sounds like a good newbie task, I'll uncomment and try to get the tests all passing.

@asturur
Copy link
Member

asturur commented Dec 31, 2020 via email

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 a pull request may close this issue.

3 participants