Skip to content

Commit

Permalink
fix(enableColumnResizing): enableColumnResizing accumulates watchers …
Browse files Browse the repository at this point in the history
…with each table $digest cycle (#5933)

* Fix for column resizer watchers accumulating.

* Added declarations for resizerScopes and resizers array.

* Added unit test to ensure watchers get destroyed.
  • Loading branch information
jedancona authored and mportuga committed Mar 3, 2017
1 parent e23a2af commit 954beba
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 12 deletions.
36 changes: 24 additions & 12 deletions src/features/resize-columns/js/ui-grid-column-resizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,9 @@
compile: function() {
return {
post: function ($scope, $elm, $attrs, uiGridCtrl) {
var grid = uiGridCtrl.grid;
var grid = uiGridCtrl.grid,
resizerScopes = [],
resizers = [];

if (grid.options.enableColumnResizing) {
var columnResizerElm = $templateCache.get('ui-grid/columnResizer');
Expand All @@ -211,13 +213,9 @@
rtlMultiplier = -1;
}

var displayResizers = function(){
var displayResizers = function () {

// remove any existing resizers.
var resizers = $elm[0].getElementsByClassName('ui-grid-column-resizer');
for ( var i = 0; i < resizers.length; i++ ){
angular.element(resizers[i]).remove();
}
removeResizers();

// get the target column for the left resizer
var otherCol = uiGridResizeColumnsService.findTargetCol($scope.col, 'left', rtlMultiplier);
Expand All @@ -227,18 +225,29 @@
if (otherCol && renderContainer.visibleColumnCache.indexOf($scope.col) !== 0 && otherCol.colDef.enableColumnResizing !== false) {
var resizerLeft = angular.element(columnResizerElm).clone();
resizerLeft.attr('position', 'left');

$elm.prepend(resizerLeft);
$compile(resizerLeft)($scope);
$compile(resizerLeft)(resizerScopes[resizerScopes.push($scope.$new()) - 1]);
}

// Don't append the right resizer if this column has resizing disabled
if ($scope.col.colDef.enableColumnResizing !== false) {
var resizerRight = angular.element(columnResizerElm).clone();
resizerRight.attr('position', 'right');

$elm.append(resizerRight);
$compile(resizerRight)($scope);
$compile(resizerRight)(resizerScopes[resizerScopes.push($scope.$new()) - 1]);
}
};

var removeResizers = function() {
// remove any existing resizer scopes.
for (var x = resizerScopes.length - 1; x >= 0; x--) {
resizerScopes[x].$destroy();
resizerScopes.splice(x);
}
// remove any existing resizer elements.
resizers = $elm[0].getElementsByClassName('ui-grid-column-resizer');
for (var i = resizers.length - 1; i >= 0; i--) {
angular.element(resizers[i]).remove();
}
};

Expand All @@ -250,7 +259,10 @@

var dataChangeDereg = grid.registerDataChangeCallback( waitDisplay, [uiGridConstants.dataChange.COLUMN] );

$scope.$on( '$destroy', dataChangeDereg );
$scope.$on( '$destroy',function() {
dataChangeDereg();
removeResizers();
});
}
}
};
Expand Down
10 changes: 10 additions & 0 deletions src/features/resize-columns/test/resizeColumns.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ describe('ui.grid.resizeColumns', function () {
});
});

describe('destroying header cell',function(){
it('should have 0 watchers after destroy',function(){
var secondCol = $(grid).find('[ui-grid-header-cell]:nth-child(1)');
secondCol.parent().scope().$destroy();
var numWatchers = secondCol.scope().$$watchers.length;

expect(numWatchers).toEqual(0);
});
});

describe('setting enableColumnResizing to false', function () {
it('should result in no resizer elements being attached to the column', function () {
$scope.gridOpts.enableColumnResizing = false;
Expand Down

0 comments on commit 954beba

Please sign in to comment.