fix/perf(tabs): Fix performance and visual issues.

Initially, the tabs used DOMSubtreeModified to listen for changes and update the height, pagination and inkbar styles. At some point, we used a different method to update the height, but we were still watching all DOM changes and updating the pagination and inkbar styles whenever the contents changed. Fix this with the following two changes: 1. Move the DOM watching to a new md-tabs-dummy-wrapper directive which only watches for changes to the tab's labels (not the content too) so that the updates are called less frequently. 2. Replace DOMSubtreeModified watching with new MutationObservers which are more drastically more performant. Additionally, some recent changes caused some bugs in the tabs by using a cached version of the internal elements instead of calling `getElements()` which searches the DOM and refreshes the cached elements. Fix by adding more calls to `getElements()` where appropriate to properly update the cache after tabs may have changed. Fixes #5681. References #4940, and might cause PR #8293 to need a rebase if this goes in first. Closes #8496
angular · May 19, 2016 · 77958a1 · 77958a1
1 parent 077769b
commit 77958a1
Show file tree

Hide file tree

Showing 5 changed files with 123 additions and 12 deletions.
diff --git a/src/components/tabs/js/tabsController.js b/src/components/tabs/js/tabsController.js
@@ -159,6 +159,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * @param stretchTabs
    */
   function handleStretchTabs (stretchTabs) {
+    var elements = getElements();
     angular.element(elements.wrapper).toggleClass('md-stretch-tabs', shouldStretchTabs());
     updateInkBarStyles();
   }
@@ -169,6 +170,8 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
 
   function handleMaxTabWidth (newWidth, oldWidth) {
     if (newWidth !== oldWidth) {
+      var elements = getElements();
+
       angular.forEach(elements.tabs, function(tab) {
         tab.style.maxWidth = newWidth + 'px';
       });
@@ -200,7 +203,9 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * @param left
    */
   function handleOffsetChange (left) {
+    var elements = getElements();
     var newValue = ctrl.shouldCenterTabs ? '' : '-' + left + 'px';
+
     angular.element(elements.paging).css($mdConstant.CSS.TRANSFORM, 'translate3d(' + newValue + ', 0, 0)');
     $scope.$broadcast('$mdTabsPaginationChanged');
   }
@@ -212,7 +217,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    */
   function handleFocusIndexChange (newIndex, oldIndex) {
     if (newIndex === oldIndex) return;
-    if (!elements.tabs[ newIndex ]) return;
+    if (!getElements().tabs[ newIndex ]) return;
     adjustOffset();
     redirectFocus();
   }
@@ -320,6 +325,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * Slides the tabs over approximately one page forward.
    */
   function nextPage () {
+    var elements = getElements();
     var viewportWidth = elements.canvas.clientWidth,
         totalWidth    = viewportWidth + ctrl.offsetLeft,
         i, tab;
@@ -334,7 +340,8 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * Slides the tabs over approximately one page backward.
    */
   function previousPage () {
-    var i, tab;
+    var i, tab, elements = getElements();
+
     for (i = 0; i < elements.tabs.length; i++) {
       tab = elements.tabs[ i ];
       if (tab.offsetLeft + tab.offsetWidth >= ctrl.offsetLeft) break;
@@ -355,7 +362,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
   }
 
   function handleInkBar (hide) {
-    angular.element(elements.inkBar).toggleClass('ng-hide', hide);
+    angular.element(getElements().inkBar).toggleClass('ng-hide', hide);
   }
 
   /**
@@ -461,6 +468,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * @returns {*|boolean}
    */
   function canPageForward () {
+    var elements = getElements();
     var lastTab = elements.tabs[ elements.tabs.length - 1 ];
     return lastTab && lastTab.offsetLeft + lastTab.offsetWidth > elements.canvas.clientWidth +
         ctrl.offsetLeft;
@@ -497,7 +505,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
   function shouldPaginate () {
     if (ctrl.noPagination || !loaded) return false;
     var canvasWidth = $element.prop('clientWidth');
-    angular.forEach(elements.dummies, function (tab) { canvasWidth -= tab.offsetWidth; });
+    angular.forEach(getElements().dummies, function (tab) { canvasWidth -= tab.offsetWidth; });
     return canvasWidth < 0;
   }
 
@@ -553,6 +561,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * Sets or clears fixed width for md-pagination-wrapper depending on if tabs should stretch.
    */
   function updatePagingWidth() {
+    var elements = getElements();
     if (shouldStretchTabs()) {
       angular.element(elements.paging).css('width', '');
     } else {
@@ -566,7 +575,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
   function calcPagingWidth () {
     var width = 1;
 
-    angular.forEach(elements.dummies, function (element) {
+    angular.forEach(getElements().dummies, function (element) {
       //-- Uses the larger value between `getBoundingClientRect().width` and `offsetWidth`.  This
       //   prevents `offsetWidth` value from being rounded down and causing wrapping issues, but
       //   also handles scenarios where `getBoundingClientRect()` is inaccurate (ie. tabs inside
@@ -616,13 +625,15 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * issues when attempting to focus an item that is out of view.
    */
   function redirectFocus () {
-    elements.dummies[ ctrl.focusIndex ].focus();
+    getElements().dummies[ ctrl.focusIndex ].focus();
   }
 
   /**
    * Forces the pagination to move the focused tab into view.
    */
   function adjustOffset (index) {
+    var elements = getElements();
+
     if (index == null) index = ctrl.focusIndex;
     if (!elements.tabs[ index ]) return;
     if (ctrl.shouldCenterTabs) return;
@@ -669,6 +680,8 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
     if (!ctrl.dynamicHeight) return $element.css('height', '');
     if (!ctrl.tabs.length) return queue.push(updateHeightFromContent);
 
+    var elements = getElements();
+
     var tabContent    = elements.contents[ ctrl.selectedIndex ],
         contentHeight = tabContent ? tabContent.offsetHeight : 0,
         tabsHeight    = elements.wrapper.offsetHeight,
@@ -727,6 +740,8 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * @returns {*}
    */
   function updateInkBarStyles () {
+    var elements = getElements();
+
     if (!elements.tabs[ ctrl.selectedIndex ]) {
       angular.element(elements.inkBar).css({ left: 'auto', right: 'auto' });
       return;
@@ -755,6 +770,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * Adds left/right classes so that the ink bar will animate properly.
    */
   function updateInkBarClassName () {
+    var elements = getElements();
     var newIndex = ctrl.selectedIndex,
         oldIndex = ctrl.lastSelectedIndex,
         ink      = angular.element(elements.inkBar);
@@ -770,6 +786,8 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * @returns {*}
    */
   function fixOffset (value) {
+    var elements = getElements();
+
     if (!elements.tabs.length || !ctrl.shouldPaginate) return 0;
     var lastTab    = elements.tabs[ elements.tabs.length - 1 ],
         totalWidth = lastTab.offsetLeft + lastTab.offsetWidth;
@@ -784,6 +802,7 @@ function MdTabsController ($scope, $element, $window, $mdConstant, $mdTabInkRipp
    * @param element
    */
   function attachRipple (scope, element) {
+    var elements = getElements();
     var options = { colorElement: angular.element(elements.inkBar) };
     $mdTabInkRipple.attach(scope, element, options);
   }

diff --git a/src/components/tabs/js/tabsDirective.js b/src/components/tabs/js/tabsDirective.js
@@ -154,7 +154,7 @@ function MdTabs () {
                   'md-scope="::tab.parent"></md-tab-item> ' +
               '<md-ink-bar></md-ink-bar> ' +
             '</md-pagination-wrapper> ' +
-            '<div class="_md-visually-hidden md-dummy-wrapper"> ' +
+            '<md-tabs-dummy-wrapper class="_md-visually-hidden md-dummy-wrapper"> ' +
               '<md-dummy-tab ' +
                   'class="md-tab" ' +
                   'tabindex="-1" ' +
@@ -168,7 +168,7 @@ function MdTabs () {
                   'ng-repeat="tab in $mdTabsCtrl.tabs" ' +
                   'md-tabs-template="::tab.label" ' +
                   'md-scope="::tab.parent"></md-dummy-tab> ' +
-            '</div> ' +
+            '</md-tabs-dummy-wrapper> ' +
           '</md-tabs-canvas> ' +
         '</md-tabs-wrapper> ' +
         '<md-tabs-content-wrapper ng-show="$mdTabsCtrl.hasContent && $mdTabsCtrl.selectedIndex >= 0" class="_md"> ' +

diff --git a/src/components/tabs/js/tabsDummyWrapperDirective.js b/src/components/tabs/js/tabsDummyWrapperDirective.js
@@ -0,0 +1,36 @@
+angular
+  .module('material.components.tabs')
+  .directive('mdTabsDummyWrapper', MdTabsDummyWrapper);
+
+/**
+ * @private
+ *
+ * @param $mdUtil
+ * @returns {{require: string, link: link}}
+ * @constructor
+ * 
+ * @ngInject
+ */
+function MdTabsDummyWrapper ($mdUtil) {
+  return {
+    require: '^?mdTabs',
+    link:    function link (scope, element, attr, ctrl) {
+      if (!ctrl) return;
+
+      var observer = new MutationObserver(function(mutations) {
+        ctrl.updatePagination();
+        ctrl.updateInkBarStyles();
+      });
+      var config = { childList: true, subtree: true };
+
+      observer.observe(element[0], config);
+
+      // Disconnect the observer
+      scope.$on('$destroy', function() {
+        if (observer) {
+          observer.disconnect();
+        }
+      });
+    }
+  };
+}
diff --git a/src/components/tabs/js/tabsTemplateDirective.js b/src/components/tabs/js/tabsTemplateDirective.js
@@ -15,13 +15,12 @@ function MdTabsTemplate ($compile, $mdUtil) {
   };
   function link (scope, element, attr, ctrl) {
     if (!ctrl) return;
+
     var compileScope = ctrl.enableDisconnect ? scope.compileScope.$new() : scope.compileScope;
+
     element.html(scope.template);
     $compile(element.contents())(compileScope);
-    element.on('DOMSubtreeModified', function () {
-      ctrl.updatePagination();
-      ctrl.updateInkBarStyles();
-    });
+
     return $mdUtil.nextTick(handleScope);
 
     function handleScope () {

diff --git a/src/components/tabs/tabs.spec.js b/src/components/tabs/tabs.spec.js
@@ -232,6 +232,63 @@ describe('<md-tabs>', function () {
       expect(tabs1[ 0 ].querySelector('md-tab-content').textContent.trim()).toBe('content that!');
     });
 
+    it('updates pagination and ink styles when string labels change', function(done) {
+      inject(function($rootScope) {
+        // Setup our initial label
+        $rootScope.$apply('label = "Some Label"');
+
+        // Init our variables
+        var template = '<md-tabs><md-tab label="{{label}}"></md-tab></md-tabs>';
+        var tabs = setup(template);
+        var ctrl = tabs.controller('mdTabs');
+
+        // Setup spies
+        spyOn(ctrl, 'updatePagination');
+        spyOn(ctrl, 'updateInkBarStyles');
+
+        // Change the label
+        $rootScope.$apply('label="Another Label"');
+
+        // Use window.setTimeout to add our expectations to the end of the call stack, after the
+        // MutationObservers have already fired
+        window.setTimeout(function() {
+          // Fire expectations
+          expect(ctrl.updatePagination.calls.count()).toBe(1);
+          expect(ctrl.updateInkBarStyles.calls.count()).toBe(1);
+
+          done();
+        });
+      })
+    });
+
+    it('updates pagination and ink styles when HTML labels change', function(done) {
+      inject(function($rootScope) {
+        // Setup our initial label
+        $rootScope.$apply('label = "Some Label"');
+
+        // Init our variables
+        var template = '<md-tabs><md-tab><md-tab-label>{{label}}</md-tab-label></md-tab></md-tabs>';
+        var tabs = setup(template);
+        var ctrl = tabs.controller('mdTabs');
+
+        // Setup spies
+        spyOn(ctrl, 'updatePagination');
+        spyOn(ctrl, 'updateInkBarStyles');
+
+        // Change the label
+        $rootScope.$apply('label="Another Label"');
+
+        // Use window.setTimeout to add our expectations to the end of the call stack, after the
+        // MutationObservers have already fired
+        window.setTimeout(function() {
+          // Fire expectations
+          expect(ctrl.updatePagination.calls.count()).toBe(1);
+          expect(ctrl.updateInkBarStyles.calls.count()).toBe(1);
+
+          done();
+        });
+      })
+    });
   });
 
   describe('aria', function () {