konvajs / konva

Konva.js is an HTML5 Canvas JavaScript framework that extends the 2d context by enabling canvas interactivity for desktop and mobile applications.
http://konvajs.org/
Other
11.72k stars 933 forks source link

Shape.prototype.getClientRect - returns wrong width/height (as getSelfRect returns width/height as strings) #626

Closed joantune closed 5 years ago

joantune commented 5 years ago

Hi Guys!

Sorry for not providing a Fiddle for this - but as you can see - the error is clear - and the solution is easy as well - we just have to make sure that we're not dealing with strings when doing the calculations.

I can probably provide a PR if you guys are interested As you can see on the image - the fillStrokeAndWidth value is wrong - as well as the preWidth You can see on the console an example of why - if you add "10" +0 - it returns the string 100

so - to correct this - I would parse to a float i.e. apply parseFloat to the getSelfRect() results before doing any calculations with this - this is a common error and a common fix - see https://www.w3schools.com/js/js_numbers.asp

screenshot-konva-issue-width

lavrton commented 5 years ago

Correct types for the properties is the user's responsibility. Konva may relay in many parts of the code that width and height are numbers and not something else, making the fix on every part is not cool.

Actually Konva already has validations for properties in development (not-minified) version. So if you use the wrong type - you will the an error.

joantune commented 5 years ago

a) I didn't get an error - and I'm using the latest not minified version. b) I checked to see if I was casting anything to strings - and AFAIK I'm not [just double checked.. although you're right - it's weird - I'm using Konva + KonvaVue - could that be the culprit somehow??]

I have investigated and saw that you call the validators when doing the getters/setters - I only use the width and height on config like objects - without me looking into the code - does that skip validation altogether or should it still be called?

joantune commented 5 years ago

I'll triple check on all the width / height configs with a call to your _isNumber call -to make sure that the issue isn't on my code - will update on that

lavrton commented 5 years ago

There are no issues with Vue. Looks like some bundlers may break minified version check. I will think about how to fix it. But anyway - just make sure you use correct types.

joantune commented 5 years ago

@lavrton: yup - somewhere along the way I was inputing strings - it is fixed and I'm now using the standard unmodified version from a CDN without parsing to floats - and without errors while applying filters.

Those warnings would be great to have in all versions - and - I would do a console.error actually - as some things work and then some other things break - and you have no clue as to why - and it makes Konva look faulty.

but anyway - thanks for the great framework - I sank my teeth on this a couple of weeks ago and It's been very useful