Skip to content

fix(5007): Reduced the amount of digest cycles initiated by the grid. #6072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 44 additions & 37 deletions misc/tutorial/401_AllFeatures.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -15,44 +15,52 @@ All features are enabled to get an idea of performance

app.controller('MainCtrl', ['$scope', '$http', '$timeout', '$interval', 'uiGridConstants', 'uiGridGroupingConstants',
function ($scope, $http, $timeout, $interval, uiGridConstants, uiGridGroupingConstants) {

$scope.gridOptions = {};
$scope.gridOptions.data = 'myData';
$scope.gridOptions.enableCellEditOnFocus = true;
$scope.gridOptions.enableColumnResizing = true;
$scope.gridOptions.enableFiltering = true;
$scope.gridOptions.enableGridMenu = true;
$scope.gridOptions.showGridFooter = true;
$scope.gridOptions.showColumnFooter = true;
$scope.gridOptions.fastWatch = true;

$scope.gridOptions.rowIdentity = function(row) {
return row.id;
var gridApi;

$scope.gridOptions = {
Copy link
Member Author

Choose a reason for hiding this comment

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

I decided to combine the multiple calls to $scope.gridOptions.[something] into one object definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

good idea

data: 'myData',
enableCellEditOnFocus: true,
enableColumnResizing: true,
enableFiltering: true,
enableGridMenu: true,
showGridFooter: true,
showColumnFooter: true,
fastWatch: true,
rowIdentity: getRowId,
getRowIdentity: getRowId,
columnDefs: [
{ name:'id', width:50 },
{ name:'name', width:100 },
{ name:'age', width:100, enableCellEdit: true, aggregationType:uiGridConstants.aggregationTypes.avg, treeAggregationType: uiGridGroupingConstants.aggregation.AVG },
{ name:'address.street', width:150, enableCellEdit: true },
{ name:'address.city', width:150, enableCellEdit: true },
Copy link
Member Author

Choose a reason for hiding this comment

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

This moved to line 31

{ name:'address.state', width:50, enableCellEdit: true },
{ name:'address.zip', width:50, enableCellEdit: true },
{ name:'company', width:100, enableCellEdit: true },
{ name:'email', width:100, enableCellEdit: true },
{ name:'phone', width:200, enableCellEdit: true },
{ name:'about', width:300, enableCellEdit: true },
{ name:'friends[0].name', displayName:'1st friend', width:150, enableCellEdit: true },
{ name:'friends[1].name', displayName:'2nd friend', width:150, enableCellEdit: true },
{ name:'friends[2].name', displayName:'3rd friend', width:150, enableCellEdit: true },
{ name:'agetemplate',field:'age', width:150, cellTemplate: '<div class="ui-grid-cell-contents"><span>Age 2:{{COL_FIELD}}</span></div>' },
{ name:'Is Active',field:'isActive', width:150, type:'boolean' },
{ name:'Join Date',field:'registered', cellFilter:'date', width:150, type:'date', enableFiltering:false },
{ name:'Month Joined',field:'registered', cellFilter: 'date:"MMMM"', filterCellFiltered:true, sortCellFiltered:true, width:150, type:'date' }
],
onRegisterApi: function onRegisterApi(registeredApi) {
gridApi = registeredApi;
}
};
$scope.gridOptions.getRowIdentity = function(row) {

function getRowId(row) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This function was repeated for getRowIndentity and rowIdentity, so I combined them.

return row.id;
};
}

$scope.gridOptions.columnDefs = [
{ name:'id', width:50 },
{ name:'name', width:100 },
{ name:'age', width:100, enableCellEdit: true, aggregationType:uiGridConstants.aggregationTypes.avg, treeAggregationType: uiGridGroupingConstants.aggregation.AVG },
{ name:'address.street', width:150, enableCellEdit: true },
{ name:'address.city', width:150, enableCellEdit: true },
{ name:'address.state', width:50, enableCellEdit: true },
{ name:'address.zip', width:50, enableCellEdit: true },
{ name:'company', width:100, enableCellEdit: true },
{ name:'email', width:100, enableCellEdit: true },
{ name:'phone', width:200, enableCellEdit: true },
{ name:'about', width:300, enableCellEdit: true },
{ name:'friends[0].name', displayName:'1st friend', width:150, enableCellEdit: true },
{ name:'friends[1].name', displayName:'2nd friend', width:150, enableCellEdit: true },
{ name:'friends[2].name', displayName:'3rd friend', width:150, enableCellEdit: true },
{ name:'agetemplate',field:'age', width:150, cellTemplate: '<div class="ui-grid-cell-contents"><span>Age 2:{{COL_FIELD}}</span></div>' },
{ name:'Is Active',field:'isActive', width:150, type:'boolean' },
{ name:'Join Date',field:'registered', cellFilter:'date', width:150, type:'date', enableFiltering:false },
{ name:'Month Joined',field:'registered', cellFilter: 'date:"MMMM"', filterCellFiltered:true, sortCellFiltered:true, width:150, type:'date' }
];
$scope.toggleFilterRow = function() {
$scope.gridOptions.enableFiltering = !$scope.gridOptions.enableFiltering;
gridApi.core.notifyDataChange(uiGridConstants.dataChange.COLUMN);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this used? Is it a new button designed into the grid? Why do you need to notifyDataChange?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is a new button. And the grid will not update unless the event is fired.


$scope.callsPending = 0;

Expand Down Expand Up @@ -90,13 +98,12 @@ All features are enabled to get an idea of performance
$timeout.cancel(timeout);
$interval.cancel(sec);
});

};

}]);
</file>
<file name="index.html">
<div ng-controller="MainCtrl">
<button id="filterToggle" type="button" class="btn btn-success" ng-click="toggleFilterRow()">Toggle Filter</button>
Copy link
Member Author

Choose a reason for hiding this comment

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

I find myself needing to test this a lot and I frequently see question about how to implement something like this, so I added it to this demo.

<button id="refreshButton" type="button" class="btn btn-success" ng-click="refreshData()">Refresh Data</button> <strong>Calls Pending:</strong> <span ng-bind="callsPending"></span>
<br>
<br>
Expand Down
2 changes: 1 addition & 1 deletion src/features/selection/js/selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@
allowCellFocus: true
};

uiGridCtrl.grid.addRowHeaderColumn(selectionRowHeaderDef, 0);
uiGridCtrl.grid.addRowHeaderColumn(selectionRowHeaderDef, 0, true);
Copy link
Member Author

Choose a reason for hiding this comment

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

I added a new attribute to prevent build columns from being triggered multiple times in a row. ColumnDefs is now responsible for building the columns. See line 763 of Grid.js

}

var processorSet = false;
Expand Down
100 changes: 64 additions & 36 deletions src/features/selection/test/uiGridSelectionDirective.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,39 +5,56 @@ describe('ui.grid.selection uiGridSelectionDirective', function() {
gridCtrl,
$compile,
$rootScope,
$timeout,
uiGridConstants;

beforeEach(module('ui.grid.selection'));
/*
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.
*/
function compileUiGridSelectionDirective(parentScope) {
var 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();

beforeEach(inject(function(_$rootScope_, _$compile_, _uiGridConstants_) {
$compile = _$compile_;
$rootScope = _$rootScope_;
uiGridConstants = _uiGridConstants_;
return elm;
}

parentScope = $rootScope.$new();
beforeEach(function() {
module('ui.grid.selection');

parentScope.options = {
columnDefs : [{field: 'id'}]
};
inject(function(_$compile_, _$rootScope_, _$timeout_, _uiGridConstants_) {
$compile = _$compile_;
$rootScope = _$rootScope_;
$timeout = _$timeout_;
uiGridConstants = _uiGridConstants_;
});

parentScope.options.isRowSelectable = function(gridRow) {
return gridRow.entity.id % 2 === 0;
parentScope = $rootScope.$new();
parentScope.options = {
columnDefs : [{field: 'id'}],
data: [],
isRowSelectable: function(gridRow) {
return gridRow.entity.id % 2 === 0;
}
};

parentScope.options.data = [];
for (var i = 0; i < 10; i++) {
parentScope.options.data.push({id: i});
}

var tpl = '<div ui-grid="options" ui-grid-selection options="options"></div>';
elm = $compile(tpl)(parentScope);

parentScope.$digest();
elm = compileUiGridSelectionDirective(parentScope);
scope = elm.scope();

gridCtrl = elm.controller('uiGrid');
});

}));
it('should add the row header selection buttons', function() {
expect($(elm).find('.ui-grid-header .ui-grid-selection-row-header-buttons').length).toEqual(1);
});

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++) {
Expand All @@ -56,25 +73,10 @@ 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_;

beforeEach(function () {
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();
}));
elm = compileUiGridSelectionDirective(parentScope);
});

afterEach(function () {
$(elm).remove();
Expand All @@ -95,4 +97,30 @@ describe('ui.grid.selection uiGridSelectionDirective', function() {
expect(noFilteringHeight < filteringHeight).toBe(true);
});
});

describe('when row header selection is turned off', function() {
beforeEach(function () {
parentScope.options.enableRowHeaderSelection = false;
elm = compileUiGridSelectionDirective(parentScope);
});

it('should not add the row header selection buttons', function() {
expect($(elm).find('.ui-grid-header .ui-grid-selection-row-header-buttons').length).toEqual(0);
});
});

describe('when isRowSelectable is not defined', function() {
beforeEach(function () {
delete parentScope.options.isRowSelectable;
elm = compileUiGridSelectionDirective(parentScope);
gridCtrl = elm.controller('uiGrid');
});

it('should not define enableSelection', function() {
for (var i = 0; i < gridCtrl.grid.rows.length; i++) {
var currentRow = gridCtrl.grid.rows[i];
expect(currentRow.enableSelection).toBeUndefined();
}
});
});
});
2 changes: 1 addition & 1 deletion src/features/tree-base/js/tree-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@
};

rowHeaderColumnDef.visible = grid.options.treeRowHeaderAlwaysVisible;
grid.addRowHeaderColumn( rowHeaderColumnDef, -100 );
grid.addRowHeaderColumn(rowHeaderColumnDef, -100, true);
Copy link
Member Author

Choose a reason for hiding this comment

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

I added a new attribute to prevent build columns from being triggered multiple times in a row. ColumnDefs is now responsible for building the columns. See line 763 of Grid.js

},


Expand Down
63 changes: 28 additions & 35 deletions src/features/tree-base/test/tree-base.spec.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
describe('ui.grid.treeBase uiGridTreeBaseService', function () {
var uiGridTreeBaseService;
var uiGridTreeBaseConstants;
var gridClassFactory;
var grid;
var $rootScope;
var $scope;
var GridRow;
var gridUtil;
var uiGridConstants;

beforeEach(module('ui.grid.treeBase'));

beforeEach(inject(function (_uiGridTreeBaseService_,_gridClassFactory_, $templateCache, _uiGridTreeBaseConstants_,
_$rootScope_, _GridRow_, _gridUtil_, _uiGridConstants_) {
uiGridTreeBaseService = _uiGridTreeBaseService_;
uiGridTreeBaseConstants = _uiGridTreeBaseConstants_;
gridClassFactory = _gridClassFactory_;
$rootScope = _$rootScope_;
var uiGridTreeBaseService,
uiGridTreeBaseConstants,
gridClassFactory,
grid,
$rootScope,
$scope,
GridRow,
gridUtil,
uiGridConstants;

beforeEach(function() {
module('ui.grid.treeBase');

inject(function (_uiGridTreeBaseService_,_gridClassFactory_, $templateCache, _uiGridTreeBaseConstants_,
_$rootScope_, _GridRow_, _gridUtil_, _uiGridConstants_) {
uiGridTreeBaseService = _uiGridTreeBaseService_;
uiGridTreeBaseConstants = _uiGridTreeBaseConstants_;
gridClassFactory = _gridClassFactory_;
$rootScope = _$rootScope_;
GridRow = _GridRow_;
gridUtil = _gridUtil_;
uiGridConstants = _uiGridConstants_;
});
$scope = $rootScope.$new();
GridRow = _GridRow_;
gridUtil = _gridUtil_;
uiGridConstants = _uiGridConstants_;

grid = gridClassFactory.createGrid({});
grid.options.columnDefs = [
Expand All @@ -30,6 +32,7 @@ describe('ui.grid.treeBase uiGridTreeBaseService', function () {
{field: 'col3'}
];

spyOn(grid, 'addRowHeaderColumn').and.callThrough();
uiGridTreeBaseService.initializeGrid(grid, $scope);
$scope.$apply();

Expand All @@ -44,24 +47,11 @@ describe('ui.grid.treeBase uiGridTreeBaseService', function () {
data[7].$$treeLevel = 0;
data[9].$$treeLevel = 1;

// data = [
// { col0: 'a_0', col1: 0, col2: 'c_0', col3: 0, '$$treeLevel': 0 } ,
// { col0: 'a_0', col1: 0, col2: 'c_1', col3: 1, '$$treeLevel': 1 },
// { col0: 'a_0', col1: 1, col2: 'c_2', col3: 2 },
// { col0: 'a_0', col1: 1, col2: 'c_3', col3: 3, '$$treeLevel': 1 },
// { col0: 'a_1', col1: 2, col2: 'c_4', col3: 4, '$$treeLevel': 2 },
// { col0: 'a_1', col1: 2, col2: 'c_5', col3: 5 },
// { col0: 'a_1', col1: 3, col2: 'c_6', col3: 6 },
// { col0: 'a_1', col1: 3, col2: 'c_7', col3: 7, '$$treeLevel': 0 },
// { col0: 'a_2', col1: 4, col2: 'c_8', col3: 8 },
// { col0: 'a_2', col1: 4, col2: 'c_9', col3: 9, '$$treeLevel': 1 }
// ];

grid.options.data = data;

grid.buildColumns();
grid.modifyRows(grid.options.data);
}));
});


describe( 'initializeGrid and defaultGridOptions', function() {
Expand Down Expand Up @@ -90,6 +80,9 @@ describe('ui.grid.treeBase uiGridTreeBaseService', function () {
enableExpandAll: true
});
});
it('should call addRowHeaderColumn', function() {
expect(grid.addRowHeaderColumn).toHaveBeenCalledWith(jasmine.any(Object), -100, true);
});
});


Expand Down
Loading