From 767e0228006b7ddcc9685b502c25733db1d6ff94 Mon Sep 17 00:00:00 2001 From: "Portugal, Marcelo" Date: Fri, 15 Jun 2018 12:53:15 -0400 Subject: [PATCH] fix(ui-grid.core): Address resizing issues. - Ensure displayNames can be empty strings - Ensure scrolling is not lost on grid resize - Ensure rows are refreshed when auto-resize is triggered --- src/features/auto-resize/js/auto-resize.js | 2 +- src/js/core/factories/Grid.js | 19 +++--- src/js/core/factories/GridColumn.js | 4 +- test/unit/core/factories/Grid.spec.js | 71 ++++++++++++++++++++- test/unit/core/factories/GridColumn.spec.js | 48 ++++++++------ 5 files changed, 113 insertions(+), 31 deletions(-) diff --git a/src/features/auto-resize/js/auto-resize.js b/src/features/auto-resize/js/auto-resize.js index 8f213040bf..0ceb852cb5 100644 --- a/src/features/auto-resize/js/auto-resize.js +++ b/src/features/auto-resize/js/auto-resize.js @@ -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); }); diff --git a/src/js/core/factories/Grid.js b/src/js/core/factories/Grid.js index b5cc38d9ba..9efa9830d1 100644 --- a/src/js/core/factories/Grid.js +++ b/src/js/core/factories/Grid.js @@ -2289,6 +2289,9 @@ angular.module('ui.grid') return p.promise; }; + function getPrevScrollValue(rowsAdded, prevScrollVal) { + return rowsAdded || prevScrollVal > 0 ? prevScrollVal : null; + } /** * @ngdoc function @@ -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); } }; diff --git a/src/js/core/factories/GridColumn.js b/src/js/core/factories/GridColumn.js index 4285d08488..447e407009 100644 --- a/src/js/core/factories/GridColumn.js +++ b/src/js/core/factories/GridColumn.js @@ -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; } /** diff --git a/test/unit/core/factories/Grid.spec.js b/test/unit/core/factories/Grid.spec.js index f897b55f3e..144af6ef1f 100644 --- a/test/unit/core/factories/Grid.spec.js +++ b/test/unit/core/factories/Grid.spec.js @@ -309,8 +309,6 @@ describe('Grid factory', function () { grid.createRightContainer(); expect(grid.renderContainers.right).toBe(right); }); - - }); describe('buildColumns', function() { @@ -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() { diff --git a/test/unit/core/factories/GridColumn.spec.js b/test/unit/core/factories/GridColumn.spec.js index 433b9fa6b9..c89f3a331b 100644 --- a/test/unit/core/factories/GridColumn.spec.js +++ b/test/unit/core/factories/GridColumn.spec.js @@ -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')); @@ -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); }); @@ -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); }); @@ -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); }); @@ -103,7 +100,6 @@ describe('GridColumn factory', function () { }); - it('should obey columnDef sort spec', function () { // ... TODO(c0bra) }); @@ -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'); @@ -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'); @@ -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;