PolymerElements / iron-fit-behavior

Fits an element into another element
17 stars 34 forks source link

Perf degradation due to calls to window.getComputedStyle in attached #68

Open benoitjchevalier opened 7 years ago

benoitjchevalier commented 7 years ago

In attached we remember if the element is RTL or not: this._isRTL = window.getComputedStyle(this).direction == 'rtl';

Even though this is designed as too avoid having to make recurrent call to getComputedStyle attached can be called again when moving the element around in the dom, performing an unneeded calculation/layout.

In our case the performance hit actually shows when moving an item around having 40 subitems which all get "attached".

madiyetov commented 7 years ago

getComputedStyle added for the first render +60ms. May be there is another way to identify LTR/RTL. For example providing as property and setting by default to false or calculating in setTimeout. Why this issue is still opened?

mkorenko commented 6 years ago

This issue causes ~20+% rendering performance degradation in the real app, please see: https://jsfiddle.net/xh0q7oqv/

image

All reds you see - forced reflow @ iron-fit-behavior.html:262

PolymerElements/paper-dropdown-menu#284

chiefcll commented 6 years ago

Chiming in a little late - but we have the same issue in our app. We are including lots of elements which use iron-fit-behavior and each one is performing this calculation. Can you do this as a scoped variable in iron-fit-behavior so it is calculated once upon load.

krozycki commented 6 years ago

Same here. Performance degradation because of many elements triggering the calculation.

benoitjchevalier commented 6 years ago

Maybe a decent solution would be to add a new feature, along the lines of what @stat92 mentioned. Add an isRtl property, if the property has been set externally then skip the getComputedStyle in attached, otherwise keep the same behavior. That way it's backward compatible and only require a minor bump, people having issue with performance regarding this specific scenario can fix it and other people can keep using it as before.

Not sure how easy it would be to get some Polymer people to have a look at it but might be worth issuing a PR and tagging some of them