search

PMSI-AlignAlytics / dimple

An object-oriented API for business analytics
Other
2.73k stars 556 forks source link

offsetWidth and offsetHeight are deprecated #205

Open didiercrunch opened 8 years ago

didiercrunch commented 8 years ago

Google chrome will remove the offsetWidth and offsetHeight methods of SVGElement in April of this year. There are couple of places where those methods are use in the current code base, we should replace them. See https://www.chromestatus.com/features/5724912467574784 for more details.

Bellow is a log from my console

VM5875 dimple.v2.1.6.js?_=348714521eff5ba4a2ecba2a1f6a83a5:4810'SVGElement.offsetWidth' is deprecated and will be removed in M50, around April 2016. See https://www.chromestatus.com/features/5724912467574784 for more details.

laxgeek commented 8 years ago

Indeed they are. I locally changed them to be parent.getBoundingClientRect().width and parent.getBoundingClientRect().height. Not sure what the procedure is for getting this into the formal code.

kenips commented 8 years ago

thanks for looking into this @laxgeek, the correct place to change is this file: https://github.com/PMSI-AlignAlytics/dimple/blob/c3a10e5a2dbc7c1b67c361f480f6f896af47b22d/src/methods/_parentWidth.js. However I notice that even with removal the current code will still work with parent.clientWidth.

laxgeek commented 8 years ago

thanks @kenips! I agree that the code will work fine with parent.clientWidth. My change was to remove any warnings/errors that may occur in the background once the functionality is removed. I can make the change in the correct file (Github newbie mistake).....but which one? Should I simply remove the call to offsetWidth or change it to the code I originally inserted (parent.getBoundingClientRect().width)?

kenips commented 8 years ago

The file to change is dimple/src/methods/_parentWidth.js. You can leave the rest untouched. Does parent.getBoundingClientRect().width cover Firefox as well? If so you can really clean up that file :). Thanks!

laxgeek commented 8 years ago

After a bit of research and testing it appears that parent.getBoundingClientRect().width works correctly for Firefox. I'll fix up dimple/src/methods/_parentWidth.js with the changes. Thanks for the guidance!

kenips commented 8 years ago

That's great to know. Thanks for looking into this :beers:.

dss010101 commented 8 years ago

we have some code that is using dimple in production. Is this something to be concerned about? Will this be considered a hard break in the sense the chart won't render at all come April 1st?

johnkiernander commented 8 years ago

Thanks all, sorry I've been a bit quiet on here lately. I'll be implementing these fixes when I get a chance, however you don't need to be alarmed about the fail date, existing versions of dimple will fall back to methods implemented for other browsers. I'm really excited about getBoundingClientRect as a solution, I wasn't aware of that one. I'll be really pleased to lose the method I implemented for Firefox support.

How does getBoundingClientRect behave with viewbox because the method I've got at the moment doesn't work properly with viewbox.

laxgeek commented 8 years ago

Thanks @johnkiernander! Less code, simpler code, just makes you sleep better at night! :relieved: I have not personally used viewBox in my system, but I did find this StackOverflow item along with this JFiddle that seem to prove that it works as expected.