Skip to content

Commit

Permalink
fix(ui-grid.core): Address resizing issues.
Browse files Browse the repository at this point in the history 
- Ensure displayNames can be empty strings
- Ensure scrolling is not lost on grid resize
- Ensure rows are refreshed when auto-resize is triggered
  • Loading branch information
@mportuga
Portugal, Marcelo authored and mportuga committed Jun 15, 2018
1 parent 5b584f9 commit 767e022
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/features/auto-resize/js/auto-resize.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
if ($elm[0].offsetParent !== null) {
uiGridCtrl.grid.gridWidth = width;
uiGridCtrl.grid.gridHeight = height;
uiGridCtrl.grid.queueRefresh()
uiGridCtrl.grid.queueGridRefresh()
.then(function() {
uiGridCtrl.grid.api.core.raise.gridDimensionChanged(prevHeight, prevWidth, height, width);
});
Expand Down
19 changes: 10 additions & 9 deletions src/js/core/factories/Grid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2289,6 +2289,9 @@ angular.module('ui.grid')
return p.promise;
};

function getPrevScrollValue(rowsAdded, prevScrollVal) {
return rowsAdded || prevScrollVal > 0 ? prevScrollVal : null;
}

/**
* @ngdoc function
Expand All @@ -2304,18 +2307,16 @@ angular.module('ui.grid')
var self = this;

for (var i in self.renderContainers) {
var container = self.renderContainers[i];
var container = self.renderContainers[i],
prevScrollTop = getPrevScrollValue(rowsAdded, container.prevScrollTop),
prevScrollLeft = getPrevScrollValue(rowsAdded, container.prevScrollLeft),
prevScrolltopPercentage = rowsAdded || prevScrollTop > 0 ? null : container.prevScrolltopPercentage,
prevScrollleftPercentage = rowsAdded || prevScrollLeft > 0 ? null : container.prevScrollleftPercentage;

// gridUtil.logDebug('redrawing container', i);

if (rowsAdded) {
container.adjustRows(container.prevScrollTop, null);
container.adjustColumns(container.prevScrollLeft, null);
}
else {
container.adjustRows(null, container.prevScrolltopPercentage);
container.adjustColumns(null, container.prevScrollleftPercentage);
}
container.adjustRows(prevScrollTop, prevScrolltopPercentage);
container.adjustColumns(prevScrollLeft, prevScrollleftPercentage);
}
};

Expand Down
4 changes: 3 additions & 1 deletion src/js/core/factories/GridColumn.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,9 @@ angular.module('ui.grid')

// Use colDef.displayName as long as it's not undefined, otherwise default to the field name
function getDisplayName(colDef) {
return colDef.displayName || gridUtil.readableColumnName(colDef.name);
return (colDef.displayName === undefined)
? gridUtil.readableColumnName(colDef.name)
: colDef.displayName;
}

/**
Expand Down
71 changes: 69 additions & 2 deletions test/unit/core/factories/Grid.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,6 @@ describe('Grid factory', function () {
grid.createRightContainer();
expect(grid.renderContainers.right).toBe(right);
});


});

describe('buildColumns', function() {
Expand Down Expand Up @@ -1147,6 +1145,75 @@ describe('Grid factory', function () {
});
});

describe('redrawInPlace', function() {
beforeEach(function() {
grid.renderContainers = {
body: {
prevScrollTop: 0,
prevScrollLeft: 0,
prevScrolltopPercentage: 0,
prevScrollleftPercentage: 0,
adjustRows: jasmine.createSpy('adjustRows'),
adjustColumns: jasmine.createSpy('adjustColumns')
}
};
});
describe('when rows have been added', function() {
beforeEach(function() {
grid.renderContainers.body.prevScrollTop = 10;
grid.renderContainers.body.prevScrollLeft = 20;
grid.redrawInPlace(true);
});
it('should call adjust rows with null for a scroll percentage and the prevScrollTop value', function() {
expect(grid.renderContainers.body.adjustRows).toHaveBeenCalledWith(
grid.renderContainers.body.prevScrollTop,
null
);
});
it('should call adjust columns with null for a scroll percentage and the prevScrollLeft value', function() {
expect(grid.renderContainers.body.adjustColumns).toHaveBeenCalledWith(
grid.renderContainers.body.prevScrollLeft,
null
);
});
});
describe('when rows have not been added', function() {
it('should call adjust rows with null for a scroll percentage and the prevScrollTop value if prevScrollTop is greater than 0', function() {
grid.renderContainers.body.prevScrollTop = 10;
grid.redrawInPlace(false);
expect(grid.renderContainers.body.adjustRows).toHaveBeenCalledWith(
grid.renderContainers.body.prevScrollTop,
null
);
});
it('should call adjust columns with null for a scroll percentage and the prevScrollLeft value if prevScrollLeft is greater than 0', function() {
grid.renderContainers.body.prevScrollLeft = 20;
grid.redrawInPlace(false);
expect(grid.renderContainers.body.adjustColumns).toHaveBeenCalledWith(
grid.renderContainers.body.prevScrollLeft,
null
);
});
it('should call adjust rows with null for a scroll top and the scroll percentage value if prevScrollTop is less or equal to 0', function() {
grid.renderContainers.body.prevScrollTop = 0;
grid.renderContainers.body.prevScrolltopPercentage = 30;
grid.redrawInPlace(false);
expect(grid.renderContainers.body.adjustRows).toHaveBeenCalledWith(
null,
grid.renderContainers.body.prevScrolltopPercentage
);
});
it('should call adjust columns with null for a scroll left and the scroll percentage value if prevScrollLeft is less or equal to 0', function() {
grid.renderContainers.body.prevScrollLeft = 0;
grid.renderContainers.body.prevScrollleftPercentage = 40;
grid.redrawInPlace(false);
expect(grid.renderContainers.body.adjustColumns).toHaveBeenCalledWith(
null,
grid.renderContainers.body.prevScrollleftPercentage
);
});
});
});

describe( 'data change callbacks', function() {
it('register then deregister data change callback', function() {
Expand Down
48 changes: 30 additions & 18 deletions test/unit/core/factories/GridColumn.spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
describe('GridColumn factory', function () {
var $q, $scope, cols, grid, gridCol, Grid, GridColumn, gridClassFactory, GridRenderContainer, uiGridConstants, $httpBackend;
var $q, $scope, cols, grid, Grid, GridColumn, gridClassFactory, GridRenderContainer, uiGridConstants, $httpBackend;

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

Expand Down Expand Up @@ -65,8 +65,7 @@ describe('GridColumn factory', function () {

describe('should not update sort when updating a column, but visible flag does update', function () {
beforeEach(function(complete){
var sort = { priority: 0, direction: 'asc' };
grid.options.columnDefs[0].sort = sort;
grid.options.columnDefs[0].sort = { priority: 0, direction: 'asc' };
grid.options.columnDefs[0].visible = false;
buildCols(complete);
});
Expand All @@ -79,8 +78,7 @@ describe('GridColumn factory', function () {

describe('should update everything but term when updating filters', function () {
beforeEach(function(complete) {
var filter = { term: 'x', placeholder: 'placeholder', type: uiGridConstants.filter.SELECT, selectOptions: [ { value: 1, label: "male" } ] };
grid.options.columnDefs[0].filter = filter;
grid.options.columnDefs[0].filter = { term: 'x', placeholder: 'placeholder', type: uiGridConstants.filter.SELECT, selectOptions: [ { value: 1, label: "male" } ] };
buildCols(complete);
});

Expand All @@ -92,8 +90,7 @@ describe('GridColumn factory', function () {

describe('should update everything but term when updating filters', function () {
beforeEach(function(complete) {
var filters = [{ term: 'x', placeholder: 'placeholder', type: uiGridConstants.filter.SELECT, selectOptions: [ { value: 1, label: "male" } ] }];
grid.options.columnDefs[0].filters = filters;
grid.options.columnDefs[0].filters = [{ term: 'x', placeholder: 'placeholder', type: uiGridConstants.filter.SELECT, selectOptions: [ { value: 1, label: "male" } ] }];
buildCols(complete);
});

Expand All @@ -103,7 +100,6 @@ describe('GridColumn factory', function () {
});



it('should obey columnDef sort spec', function () {
// ... TODO(c0bra)
});
Expand All @@ -115,15 +111,13 @@ describe('GridColumn factory', function () {
});

it('should add an incrementing number to column names when they have the same field and no name', function () {
var cols = [
grid.options.columnDefs = [
{ field: 'age' },
{ field: 'name' },
{ field: 'name' },
{ field: 'name' }
];

grid.options.columnDefs = cols;

buildCols();

expect(grid.columns[0].name).toEqual('age');
Expand All @@ -133,34 +127,29 @@ describe('GridColumn factory', function () {
});

it('should not change the displayNames if they are provided', function () {
var cols = [
grid.options.columnDefs = [
{ field: 'age' },
{ field: 'name', displayName:'First Name' },
{ field: 'name', displayName:'First Name' },
{ field: 'name', displayName:'First Name' }
];

grid.options.columnDefs = cols;

buildCols();

expect(grid.columns[0].displayName).toEqual('Age');
expect(grid.columns[1].displayName).toEqual('First Name');
expect(grid.columns[2].displayName).toEqual('First Name');
expect(grid.columns[3].displayName).toEqual('First Name');

});

it('should account for existing incremented names', function () {
var cols = [
grid.options.columnDefs = [
{ field: 'age' },
{ field: 'name' },
{ field: 'name', name: 'name3' },
{ field: 'name' }
];

grid.options.columnDefs = cols;

buildCols();

expect(grid.columns[0].name).toEqual('age');
Expand Down Expand Up @@ -622,6 +611,29 @@ describe('GridColumn factory', function () {
expect(updateCol(col, colDef)).toThrow();
});

describe('displayName', function() {
it('should allow the user to set the displayName to an empty string or null value', function() {
colDef.displayName = '';
col.updateColumnDef(colDef);
expect(col.displayName).toEqual('');

colDef.displayName = null;
col.updateColumnDef(colDef);
expect(col.displayName).toBeNull();
});
it('should allow the user to enter whatever displayName they desire', function() {
colDef.displayName = 'Test';
col.updateColumnDef(colDef);
expect(col.displayName).toEqual(colDef.displayName);
});
it('should generate a displayName based on the name attribute if the user does not define it', function() {
colDef.displayName = undefined;
colDef.name = 'mockName';
col.updateColumnDef(colDef);
expect(col.displayName).toEqual('Mock Name');
});
});

describe('cellTooltip', function() {
it('should be set to false when it is undefined', function() {
colDef.cellTooltip = undefined;
Expand Down

0 comments on commit 767e022

Please sign in to comment.