- Notifications
You must be signed in to change notification settings - Fork27.4k
perf(ngAnimate): cache repeated calls to addClass/removeClass#14166
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Applies to 1.4.x and 1.5.x and masterClosesangular#14165Closesangular#14166
Don't merge this yet. There's a cache issue. |
Applies to 1.4.x and 1.5.x and masterClosesangular#14165Closesangular#14166
OK now it's fixed. |
Applies to 1.4.x and 1.5.x and masterClosesangular#14165Closesangular#14166
Wow that's a ton of changes. I'll try to review today. However could you update the commit message with an explanation of the issue and the fix? |
| from: {background: 'red' }, | ||
| to: {background: 'blue', 'font-size': '50px' } | ||
| from: {height: '100px' }, | ||
| to: {height: '200px', 'font-size': '50px' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@matsko why change these styles? It seems arbitrary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
When the element is attached to a parent element then the colors are actually applied and getComputedStyle (which is what element.css in jQuery uses) actually figures out what the color is. Beforehand it was just using the inline style. Since we now deal with actually getting the RGB value for the color it was easier just to use a more stable value like a height measurement value.
Oh and@matsko can you add a demo that shows the speed up that this introduces? |
Here's a plnkr to test the performance:http://plnkr.co/edit/bnaaaHbWkyQULeoOOg2X?p=preview There's a simple ngRepeat that an ngClass on it. In my tests, ng animate with this test is about 30% faster. However, if you include the Regarding animating nth-child(x) elements in a repeater, that doesn't seem to work anyway:http://plnkr.co/edit/klCK5E22YPJ9Zfbz2meF?p=preview |
…imation has no durationBackground:ngAnimate writes helper classes to DOM elements to see if animations are defined on them. If manyelements have the same definition, we can cache the definition and skip the application of thehelper classes altogether. This helps particularly with large ngRepeat collections.Closesangular#14165Closesangular#14166
…imation has no durationBackground:ngAnimate writes helper classes to DOM elements to see if animations are defined on them. If manyelements have the same definition, we can cache the definition and skip the application of thehelper classes altogether. This helps particularly with large ngRepeat collections.Closesangular#14165Closesangular#14166
…imation has no durationBackground:ngAnimate writes helper classes to DOM elements to see if animations are defined on them. If manyelements have the same definition, we can cache the definition and skip the application of thehelper classes altogether. This helps particularly with large ngRepeat collections.Closesangular#14165Closesangular#14166
…imation has no durationBackground:ngAnimate writes helper classes to DOM elements to see if animations are defined on them. If manyelements have the same definition, we can cache the definition and skip the application of thehelper classes altogether. This helps particularly with large ngRepeat collections.Closesangular#14165Closesangular#14166
…imation has no durationBackground:ngAnimate writes helper classes to DOM elements to see if animations are defined on them. If manyelements have the same definition, and the same parent, we can cache the definition and skip theapplication of the helper classes altogether. This helps particularly with large ngRepeatcollections.Closes#14165Closes#14166Closes#16613
Applies to 1.4.x and 1.5.x and master
Closes#14165
Closes#14166