From 7c5cdca1f471a0a3c1ef340fe65af268df68cae3 Mon Sep 17 00:00:00 2001 From: Brian Hann Date: Mon, 30 Mar 2015 14:17:34 -0500 Subject: [PATCH] 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 --- .../test/uiGridSelectionDirective.spec.js | 72 ++++++++++++++----- src/js/core/factories/Grid.js | 31 ++++++-- 2 files changed, 78 insertions(+), 25 deletions(-) diff --git a/src/features/selection/test/uiGridSelectionDirective.spec.js b/src/features/selection/test/uiGridSelectionDirective.spec.js index 84b87d1b40..6f16a2e134 100644 --- 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 = '
'; - - 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('
'); + 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 index 746a685546..bd79f254b1 100644 --- 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; } }