Fix the spell error which makes the gradientTransform doesn't work

[corleone113]

No description provided.

[corleone113]

Just fix the spell error in the source files, which may cause the gradientTransform doesn't work at some condition.

[asturur]

hi @corleone113. Thanks for the fix.
We are fixing something, but the tests are all passing.
That means that this particular code change isn't covered by any test. This also means that if we break it again, we do not have alarms for it.

Could you please try to add a test?
We need a visual test that give a gradient with gradientTrasnform will render properly.
Both for text and any other object.

If you need help to write it i can help, but you can find here examples:
https://github.com/fabricjs/fabric.js/blob/5c4c9573e382066cc701b49a5099fb454ea9e788/test/visual/generic_rendering.js

[corleone113]

Hi @asturur.
I've add a visual test. Because the fix just change few codes, so the test cases is few. If that didn't meet your standards, you can notify me as soon.

[asturur]

@corleone113 the test can be added in the existing file with other tests. Otherwise i have to add this new file to the website and is extra work. The generic render file is good enough ( the one i linked ).

The other important thing is that your test is sort of invalid.
You need to create a standard fabricJS gradient, while what you created is sort of mocked object for it.

look how this test is defined:

fabric.js/test/visual/generic_rendering.js

Line 209 in 5c4c957

function backgroundWithGradientNoVpt(canvas, callback) {

[asturur]

please remove this stroke: { toLive: .... } the correct form, as you can see in other tests is:

var line = new fabric.Line([10, 10, 200, 200], {
  stroke: new fabric.Gradient({ ... options })
})

[corleone113]

I'm sorry for not replying in time and thank you for your guidance.I have some other things to do, so I can only fix these errors later.

…

---Original--- From: "Andrea ***@***.***> Date: Mon, May 17, 2021 19:17 PM To: ***@***.***>; Cc: "corleone ***@***.******@***.***>; Subject: Re: [fabricjs/fabric.js] Fix the spell error which makes the gradientTransform doesn't work (#7059) @asturur commented on this pull request. In test/visual/generic_rendering.js: > @@ -361,5 +361,112 @@ height: 100, }); + function gradientStroke(canvas, callback) { +var line = new fabric.Line([10, 10, 200, 200], { +stroke: { +toLive(ctx) { please remove this stroke: { toLive: .... } the correct form, as you can see in other tests is: var line = new fabric.Line([10, 10, 200, 200], { stroke: new fabric.Gradient({ ... options }) }) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[corleone113]

I've fixed the invalid test codes, please review again.

[asturur]

Code looks good, i need to validate the test manually the text test seems too small to fail with a 9% rate.
i'll do it today and in case merge

[stale]

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.