fix(ngModel): minimize jank when toggling CSS classes during validation #9263
Conversation
|
@jeffbcross / @tbosch also it would be very good if you could take a look and see if this is the way we want to minimize the jank, or if there are any other places you think might benefit from it |
Previously, class toggling would always occur immediately. This causes problems in cases where class changes happen super frequently, and can result in flickering in some browsers which do not handle this jank well. Closes angular#8234
|
http://plnkr.co/edit/d2qK45maTpcPTkU64ewI?p=preview here's a fork of the plunker from the original issue, which when tested on SL with IE11, seems to indicate that this fix does what we want. I was able to reproduce with the beta 15 plunker pretty quickly (using both the ng-if button and the ng-repeat button), (although the ng-if button took a few more tries to see the blinking), but after a few minutes of trying, the forked plunker would not display any of the blinking. |
|
asked some volunteers to test with whatever version of IE they had handy on their windows machines, and it sounds like this does the right thing |
|
I am not sure whether this fixes the right problem: Adding and removing classes should not be a problem for flashing as long as it is done in sync and no one causes a rendering. Could you investigate a little bit more? |
|
As far as I'm aware, we are not asynchronously changing classes. Certain CSS properties are known to cause relayout when used/changed (box-shadow may be one of these, which is used in the plunker). Non-IE browsers don't suffer from this jank, so it's possible that IE is simply re-rendering immediately when the attribute is changed. The jank just doesn't affect other browsers, and I don't have a native windows machine so it's a bit hard to debug effectively. However, the fix does prevent it I'm not able to reproduce with a trivial example, but I think we can likely see some performance benefits if we avoid switching the CSS classes back and forth so much, regardless |
|
http://jsbin.com/butusalilifi/1/edit I can't reproduce it with a trivial reproduction, but as I posted in the issue itself, some classes are changing back and forth a lot more than the "once for each control" that should be happening. It's possible that the amount of back-and-forth switching in a single turn has an effect on this. But I don't believe we're deferring anything to a different turn, since ngAnimate isn't being used. |
|
It looks to me like the node gets created, inserted, and runs its first set of validation functions --- but in the same digest, ngModelWatch happens and re-validates it, and then the maxlength attribute is observed and validators run yet again. Somehow, IE is jankier than other browsers with this So, it's hard to reproduce this exactly in a minimal scenario, but with this fix, class changes from those three validation runs are combined into one set of changes. (there may be a slight perf increase this way, since we're switching into native code less frequently --- just the slight cost of constructing a new object to throw away for each model that gets processed) |
|
Anyways, having researched this a bit, I think this is the best solution --- we can't really defer validation, otherwise values will not be what people expect --- but we can defer updating CSS classes. The only improvement that I can think of would be reusing the pendingClassChanges object instead of throwing it away every digest. But at least it gets reused by all of the different validator runs that happen within the digest. |
|
@caitp This would be a more general solution that would also work for other cases... |
|
Same is true for |
|
I will look at refactoring it to do that |
|
Thanks! |
|
I've looked into it, I think there are some things that suck about this --- here we can use the local scope, but we can't really force ngAnimate / $animate to do that. This is great for ng-click or whatever but really sucks for anyone that wants to use local digests. |
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes angular#8234 Closes angular#9263
|
We really should have |
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes angular#8234 Closes angular#9263
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes angular#8234 Closes angular#9263
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes angular#8234 Closes angular#9263
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes angular#8234 Closes angular#9263
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes angular#8234 Closes angular#9263
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes angular#8234 Closes angular#9263
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes angular#8234 Closes angular#9263
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. Closes angular#8234 Closes angular#9263
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. BREAKING CHANGE: The $animate class API will always defer changes until the end of the next digest. This allows ngAnimate to coalesce class changes which occur over a short period of time into 1 or 2 DOM writes, rather than many. This prevents jank in browsers such as IE, and is generally a good thing. If you're finding that your classes are not being immediately applied, be sure to invoke $digest(). Closes #8234 Closes #9263
When ngAnimate is used, it will defer changes to classes until postDigest. Previously, AngularJS (when ngAnimate is not loaded) would always immediately perform these DOM operations. Now, even when the ngAnimate module is not used, if $rootScope is in the midst of a digest, class manipulation is deferred. This helps reduce jank in browsers such as IE11. BREAKING CHANGE: The $animate class API will always defer changes until the end of the next digest. This allows ngAnimate to coalesce class changes which occur over a short period of time into 1 or 2 DOM writes, rather than many. This prevents jank in browsers such as IE, and is generally a good thing. If you're finding that your classes are not being immediately applied, be sure to invoke $digest(). Closes angular#8234 Closes angular#9263
Previously, class toggling would always occur immediately. This causes
problems in cases where class changes happen super frequently, and can result
in flickering in some browsers which do not handle this jank well.
/cc @matsko please help review this! It is very much your area of expertise!
Closes #8234