fix(Controls): allow for smooth scaling fo text at any zoom level #10186
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
fixes #10180 |
| Math.abs((localPoint.x * multiplier) / target.scaleX) - strokePadding, | ||
| ); | ||
| newWidth = | ||
| Math.abs((localPoint.x * multiplier) / target.scaleX) - strokePadding; |
There was a problem hiding this comment.
Since this was avoiding newWidth to go under 1, could you change the line below to
target.set('width', Math.max(newWidth, 1)); ?
There was a problem hiding this comment.
Also please check if there is a test for changeWidth and extend it to cover this change.
There was a problem hiding this comment.
@asturur Sure I can make that change but I just want to make sure it's necessary as versions before v6 didn't have that restriction.
Also, I'm not familiar with how the tests are set up but there doesn't appear to be any existing test that run against the changeWidth function. I did try making changes locally to the controls_handlers test script but it's unclear to me how I'm supposed to reference the changeWidth function there.
There was a problem hiding this comment.
if the previous code was using Math.ceil, it means that any float that was smaller than 1 was rounded to 1.
As a consequence newWidth was never smaller than 1.
Now your changes remove that to obtain smoothness at higher zoom levels but that 1 barrier should stay, and the line below is a perfect place to reimplement it.
A test i can add when the code changes are done
There was a problem hiding this comment.
I've just committed that change.
| expect(target.width).toBe(199.2); | ||
| expect(target.left).toBe(0); | ||
| expect(target.top).toBe(0); | ||
| }); |
There was a problem hiding this comment.
@Soldier-B there were plenty of tests and adding a new one was trivial
asturur
changed the title
Description
Removed a call to Math.ceil when calculating new width in the mr and ml control resize handlers.