fix(uiGridHeader): Allow header to shrink in size

The combo of ui-grid-selection and filtering was causes the headers to be unable to shrink in size if filtering were toggled off, because the selection grid header had an explicit height and when we saw it we would use that as the height to match up with. This change removes all explicit heights from the "max height" check, so we only rely on rendered dynamic heights to calculate the max height. A unit test has also been added to the selection specs to cover this case. Fixes #3138
angular-ui · Mar 30, 2015 · 7c5cdca · 7c5cdca
1 parent 7375c49
commit 7c5cdca
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 25 deletions.
diff --git a/src/features/selection/test/uiGridSelectionDirective.spec.js b/src/features/selection/test/uiGridSelectionDirective.spec.js
@@ -2,21 +2,19 @@ describe('ui.grid.selection uiGridSelectionDirective', function() {
   var parentScope,
       elm,
       scope,
-      gridCtrl;
+      gridCtrl,
+      $compile,
+      $rootScope,
+      uiGridConstants;
 
   beforeEach(module('ui.grid.selection'));
 
-  beforeEach(function() {
-    var rootScope;
+  beforeEach(inject(function(_$rootScope_, _$compile_, _uiGridConstants_) {
+    $compile = _$compile_;
+    $rootScope = _$rootScope_;
+    uiGridConstants = _uiGridConstants_;
 
-    inject([
-        '$rootScope',
-        function (rootScopeInj) {
-            rootScope = rootScopeInj;
-        }
-    ]);
-
-    parentScope = rootScope.$new();
+    parentScope = $rootScope.$new();
 
     parentScope.options = {
       columnDefs : [{field: 'id'}]
@@ -32,19 +30,14 @@ describe('ui.grid.selection uiGridSelectionDirective', function() {
     }
 
     var tpl = '<div ui-grid="options" ui-grid-selection options="options"></div>';
-
-    inject([
-        '$compile',
-        function ($compile) {
-        elm = $compile(tpl)(parentScope);
-    }]);
+    elm = $compile(tpl)(parentScope);
 
     parentScope.$digest();
     scope = elm.scope();
 
     gridCtrl = elm.controller('uiGrid');
 
-  });
+  }));
 
   it('should set the "enableSelection" field of the row using the function specified in "isRowSelectable"', function() {
     for (var i = 0; i < gridCtrl.grid.rows.length; i++) {
@@ -61,4 +54,47 @@ describe('ui.grid.selection uiGridSelectionDirective', function() {
       }
     }
   });
+
+  describe('with filtering turned on', function () {
+    var elm, $timeout;
+
+    /*
+      NOTES
+       - We have to flush $timeout because the header calculations are done post-$timeout, as that's when the header has been fully rendered.
+       - We have to actually attach the grid element to the document body, otherwise it will not have a rendered height.
+    */
+
+    beforeEach(inject(function (_$timeout_) {
+      $timeout = _$timeout_;
+
+      parentScope.options.enableFiltering = true;
+
+      elm = angular.element('<div style="width: 300px; height: 500px" ui-grid="options" ui-grid-selection></div>');
+      document.body.appendChild(elm[0]);
+      $compile(elm)(parentScope);
+      $timeout.flush();
+      parentScope.$digest();
+    }));
+
+    afterEach(function () {
+      $(elm).remove();
+    });
+
+    it("doesn't prevent headers from shrinking when filtering gets turned off", function () {
+      // Header height with filtering on
+      var filteringHeight = $(elm).find('.ui-grid-header').height();
+
+      dump(elm.controller('uiGrid').grid.api.core.notifyDataChange);
+
+      parentScope.options.enableFiltering = false;
+      elm.controller('uiGrid').grid.api.core.notifyDataChange( uiGridConstants.dataChange.COLUMN );
+      $timeout.flush();
+      parentScope.$digest();
+
+      var noFilteringHeight = $(elm).find('.ui-grid-header').height();
+
+      expect(noFilteringHeight).not.toEqual(filteringHeight);
+      expect(noFilteringHeight < filteringHeight).toBe(true);
+    });
+  });
 });
diff --git a/src/js/core/factories/Grid.js b/src/js/core/factories/Grid.js
@@ -1877,8 +1877,9 @@ angular.module('ui.grid')
 
             container.innerHeaderHeight = innerHeaderHeight;
 
-            // Save the largest header height for use later
-            if (innerHeaderHeight > maxHeaderHeight) {
+            // If the header doesn't have an explicit height set, save the largest header height for use later
+            //   Explicit header heights are based off of the max we are calculating here. We never want to base the max on something we're setting explicitly
+            if (!container.explicitHeaderHeight && innerHeaderHeight > maxHeaderHeight) {
               maxHeaderHeight = innerHeaderHeight;
             }
           }
@@ -1893,8 +1894,9 @@ angular.module('ui.grid')
               rebuildStyles = true;
             }
 
-            // Save the largest header canvas height for use later
-            if (headerCanvasHeight > maxHeaderCanvasHeight) {
+            // If the header doesn't have an explicit canvas height, save the largest header canvas height for use later
+            //   Explicit header heights are based off of the max we are calculating here. We never want to base the max on something we're setting explicitly
+            if (!container.explicitHeaderCanvasHeight && headerCanvasHeight > maxHeaderCanvasHeight) {
               maxHeaderCanvasHeight = headerCanvasHeight;
             }
           }
@@ -1904,12 +1906,27 @@ angular.module('ui.grid')
         for (i = 0; i < containerHeadersToRecalc.length; i++) {
           container = containerHeadersToRecalc[i];
 
-          // If this header's height is less than another header's height, then explicitly set it so they're the same and one isn't all offset and weird looking
-          if (maxHeaderHeight > 0 && typeof(container.headerHeight) !== 'undefined' && container.headerHeight !== null && container.headerHeight < maxHeaderHeight) {
+          /* If:
+              1. We have a max header height
+              2. This container has a header height defined
+              3. And either this container has an explicit header height set, OR its header height is less than the max
+
+              then:
+
+              Give this container's header an explicit height so it will line up with the tallest header
+          */
+          if (
+            maxHeaderHeight > 0 && typeof(container.headerHeight) !== 'undefined' && container.headerHeight !== null &&
+            (container.explicitHeaderHeight || container.headerHeight < maxHeaderHeight)
+          ) {
             container.explicitHeaderHeight = maxHeaderHeight;
           }
 
-          if (typeof(container.headerCanvasHeight) !== 'undefined' && container.headerCanvasHeight !== null && maxHeaderCanvasHeight > 0 && container.headerCanvasHeight < maxHeaderCanvasHeight) {
+          // Do the same as above except for the header canvas
+          if (
+            maxHeaderCanvasHeight > 0 && typeof(container.headerCanvasHeight) !== 'undefined' && container.headerCanvasHeight !== null &&
+            (container.explicitHeaderCanvasHeight || container.headerCanvasHeight < maxHeaderCanvasHeight)
+          ) {
             container.explicitHeaderCanvasHeight = maxHeaderCanvasHeight;
           }
         }