Skip to content

Commit

Permalink
fix(uiGridHeader): Fix height calculations
Browse files Browse the repository at this point in the history
Header heights were wrong in the "row header", as well as kinda weird when
multiple filters were on one column and another only had one: the border
on the second one would be too short.

This change adds handling for updating the headers' heights to keep them
consistent. Any header cell shorter than the largest header gets explicitly set to the
right height.

Alternatively every header cell will get set to the "inner" height of its
header so a shorter header cell will still have the right border size as a
larger one.

Fixes #1639, fixes #1613
  • Loading branch information
c0bra committed Oct 6, 2014
1 parent c4ecd01 commit cfc2444
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 21 deletions.
13 changes: 10 additions & 3 deletions src/js/core/directives/ui-grid-header-cell.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,24 @@
row: '=',
renderIndex: '='
},
require: '?^uiGrid',
require: ['?^uiGrid', '^uiGridRenderContainer'],
replace: true,
compile: function() {
return {
pre: function ($scope, $elm, $attrs, uiGridCtrl) {
pre: function ($scope, $elm, $attrs) {
var cellHeader = $compile($scope.col.headerCellTemplate)($scope);
$elm.append(cellHeader);
},

post: function ($scope, $elm, $attrs, uiGridCtrl) {
post: function ($scope, $elm, $attrs, controllers) {
var uiGridCtrl = controllers[0];
var renderContainerCtrl = controllers[1];

$scope.grid = uiGridCtrl.grid;

$log.debug('id', renderContainerCtrl.containerId);

$scope.renderContainer = uiGridCtrl.grid.renderContainers[renderContainerCtrl.containerId];

$elm.addClass($scope.col.getColClass(false));

Expand Down
9 changes: 8 additions & 1 deletion src/js/core/directives/ui-grid-header.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,14 @@
var template = angular.element(contents);

var newElm = $compile(template)($scope);
$elm.append(newElm);
$elm.replaceWith(newElm);

// Replace the reference to the container's header element with this new element
containerCtrl.header = newElm;
containerCtrl.colContainer.header = newElm;

// And update $elm to be the new element
$elm = newElm;

if (containerCtrl) {
// Inject a reference to the header viewport (if it exists) into the grid controller for use in the horizontal scroll handler below
Expand Down
12 changes: 11 additions & 1 deletion src/js/core/directives/ui-grid-render-container.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
var rowContainer = containerCtrl.rowContainer;
var colContainer = containerCtrl.colContainer;

var renderContainer = grid.renderContainers[$scope.containerId];

// Put the container name on this element as a class
$elm.addClass('ui-grid-render-container-' + $scope.containerId);

Expand Down Expand Up @@ -358,7 +360,15 @@

ret += '\n .grid' + uiGridCtrl.grid.id + ' .ui-grid-render-container-' + $scope.containerId + ' .ui-grid-footer-canvas { width: ' + canvasWidth + 'px; }';
ret += '\n .grid' + uiGridCtrl.grid.id + ' .ui-grid-render-container-' + $scope.containerId + ' .ui-grid-footer-viewport { width: ' + footerViewportWidth + 'px; }';
// Update

// If the render container has an "explicit" header height (such as in the case that its header is smaller than the other headers and needs to be explicitly set to be the same, ue thae)
if (renderContainer.explicitHeaderHeight !== undefined && renderContainer.explicitHeaderHeight !== null && renderContainer.explicitHeaderHeight > 0) {
ret += '\n .grid' + uiGridCtrl.grid.id + ' .ui-grid-render-container-' + $scope.containerId + ' .ui-grid-header-cell { height: ' + renderContainer.explicitHeaderHeight + 'px; }';
}
// Otherwise if the render container has an INNER header height, use that on the header cells (so that all the header cells are the same height and those that have less elements don't have undersized borders)
else if (renderContainer.innerHeaderHeight !== undefined && renderContainer.innerHeaderHeight !== null && renderContainer.innerHeaderHeight > 0) {
ret += '\n .grid' + uiGridCtrl.grid.id + ' .ui-grid-render-container-' + $scope.containerId + ' .ui-grid-header-cell { height: ' + renderContainer.innerHeaderHeight + 'px; }';
}

return ret;
}
Expand Down
29 changes: 27 additions & 2 deletions src/js/core/factories/Grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -1512,17 +1512,42 @@ angular.module('ui.grid')
var rebuildStyles = false;

// Get all the header heights
for (var i = 0; i < containerHeadersToRecalc.length; i++) {
var container = containerHeadersToRecalc[i];
var maxHeight = 0;
var i, container;
for (i = 0; i < containerHeadersToRecalc.length; i++) {
container = containerHeadersToRecalc[i];

if (container.header) {
var oldHeaderHeight = container.headerHeight;
var headerHeight = gridUtil.outerElementHeight(container.header);

container.headerHeight = headerHeight;

if (oldHeaderHeight !== headerHeight) {
rebuildStyles = true;
}

// Get the "inner" header height, that is the height minus the top and bottom borders, if present. We'll use it to make sure all the headers have a consistent height
var topBorder = gridUtil.getBorderSize(container.header, 'top');
var bottomBorder = gridUtil.getBorderSize(container.header, 'bottom');
var innerHeaderHeight = headerHeight - topBorder - bottomBorder;

container.innerHeaderHeight = innerHeaderHeight;

// Save the largest header height for use later
if (innerHeaderHeight > maxHeight) {
maxHeight = innerHeaderHeight;
}
}
}

// Go through all the headers
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 (container.headerHeight < maxHeight) {
container.explicitHeaderHeight = maxHeight;
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/js/core/services/ui-grid-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -748,8 +748,9 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC

var styles = getStyles(elem);

// If a specific border is supplied, like 'top', read the 'borderTop' style property
if (borderType) {
borderType = 'border-' + borderType;
borderType = 'border' + borderType.charAt(0).toUpperCase() + borderType.slice(1);
}
else {
borderType = 'border';
Expand Down
9 changes: 6 additions & 3 deletions src/less/header.less
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
}

@topPanelRadius: @gridBorderRadius - @gridBorderWidth;

.ui-grid-header {
border-bottom: 1px solid @borderColor;
}

.ui-grid-top-panel {
position: relative;
// z-index: 1;
// background-color: @darkGray; // #EAEAEA
border-bottom: 1px solid @borderColor; // #D4D4D4
// border-bottom: 1px solid @borderColor; // #D4D4D4

overflow: hidden; // Disable so menus show up
font-weight: bold;
Expand Down
16 changes: 9 additions & 7 deletions src/templates/ui-grid/ui-grid-header.html
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
<div class="ui-grid-top-panel">
<div ui-grid-group-panel ng-show="grid.options.showGroupPanel"></div>
<div class="ui-grid-header ui-grid-header-viewport">
<div class="ui-grid-header-canvas">
<div class="ui-grid-header-cell clearfix" ng-repeat="col in colContainer.renderedColumns track by col.colDef.name" ui-grid-header-cell col="col" render-index="$index" ng-style="$index === 0 && colContainer.columnStyle($index)">
</div>
<div class="ui-grid-header">
<div class="ui-grid-top-panel">
<div ui-grid-group-panel ng-show="grid.options.showGroupPanel"></div>
<div class="ui-grid-header-viewport">
<div class="ui-grid-header-canvas">
<div class="ui-grid-header-cell clearfix" ng-repeat="col in colContainer.renderedColumns track by col.colDef.name" ui-grid-header-cell col="col" render-index="$index" ng-style="$index === 0 && colContainer.columnStyle($index)">
</div>
</div>
</div>
<div ui-grid-menu></div>
</div>
<div ui-grid-menu></div>
</div>
4 changes: 2 additions & 2 deletions src/templates/ui-grid/uiGridHeaderCell.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div ng-class="{ 'sortable': sortable }">
<div ng-class="{ 'sortable': sortable }" ng-style="{ 'height': renderContainer.explicitHeaderHeight + 'px' }">
<div class="ui-grid-vertical-bar">&nbsp;</div>
<div class="ui-grid-cell-contents" col-index="renderIndex">
{{ col.displayName CUSTOM_FILTERS }}
Expand All @@ -9,7 +9,7 @@
</div>

<div class="ui-grid-column-menu-button" ng-if="grid.options.enableColumnMenu && !col.isRowHeader && !col.colDef.disableColumnMenu" class="ui-grid-column-menu-button" ng-click="toggleMenu($event)">
<i class="ui-grid-icon-angle-down">&nbsp;<i>
<i class="ui-grid-icon-angle-down">&nbsp;</i>
</div>

<div ng-if="filterable" class="ui-grid-filter-container" ng-repeat="colFilter in col.filters">
Expand Down
2 changes: 1 addition & 1 deletion src/templates/ui-grid/uiGridViewport.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div class="ui-grid-viewport">
<div class="ui-grid-canvas">
<div ng-repeat="(rowRenderIndex, row) in rowContainer.renderedRows track by row.uid" class="ui-grid-row" ng-style="containerCtrl.rowStyle(rowRenderIndex)">
<div ng-repeat="(rowRenderIndex, row) in rowContainer.renderedRows track by $index" class="ui-grid-row" ng-style="containerCtrl.rowStyle(rowRenderIndex)">
<div ui-grid-row="row" row-render-index="rowRenderIndex"></div>
</div>
</div>
Expand Down

4 comments on commit cfc2444

@c0bra
Copy link
Contributor Author

@c0bra c0bra commented on cfc2444 Oct 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops I left an ng-style directive in here on accident

@c0bra
Copy link
Contributor Author

@c0bra c0bra commented on cfc2444 Oct 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AND I accidentally changed the row directive to track by $index. Crap.

@PaulL1
Copy link
Contributor

@PaulL1 PaulL1 commented on cfc2444 Oct 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed these two items, as I had to fix the second in order to run e2e tests.

@c0bra
Copy link
Contributor Author

@c0bra c0bra commented on cfc2444 Oct 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Please sign in to comment.