From e8e31cee1fcccf9626919b325d95b6db8af3a3cb Mon Sep 17 00:00:00 2001 From: Paul Lambert Date: Fri, 3 Oct 2014 16:09:23 +1000 Subject: [PATCH] Fix #1685: Remove col.index from most code, add col.uid Tidies up much of the column behaviour. Removed col.index everywhere it appears, except for in the cell content templates. For the cell content, replace with col.uid, which is now uniquely generated when the column is created, and never changed again. Should also fix #1694, but need to push to be sure. --- .../303_customizing_column_menu.ngdoc | 3 +- src/features/cellnav/js/cellnav.js | 39 ++++++++++------- .../cellnav/test/uiGridCellNavService.spec.js | 6 +-- src/features/edit/templates/cellEditor.html | 2 +- .../edit/templates/dropdownEditor.html | 2 +- .../js/ui-grid-column-resizer.js | 7 ++- .../resize-columns/test/resizeColumns.spec.js | 19 ++++---- src/js/core/directives/ui-grid-column-menu.js | 4 +- src/js/core/directives/ui-grid-header-cell.js | 7 --- src/js/core/directives/ui-grid-header.js | 13 ------ src/js/core/factories/Grid.js | 26 ++++++----- src/js/core/factories/GridColumn.js | 43 +++++++++++++++---- src/js/core/factories/GridRenderContainer.js | 14 ------ src/js/core/factories/GridRow.js | 9 ---- 14 files changed, 96 insertions(+), 98 deletions(-) diff --git a/misc/tutorial/303_customizing_column_menu.ngdoc b/misc/tutorial/303_customizing_column_menu.ngdoc index 2f1cd12ea9..5853446467 100644 --- a/misc/tutorial/303_customizing_column_menu.ngdoc +++ b/misc/tutorial/303_customizing_column_menu.ngdoc @@ -12,7 +12,8 @@ The column menu will add the column's `GridColumn` object to the context as `thi You can then show/hide the the menu item based on conditions that use the grid and column. You could also use a custom column builder to add some item to the every column definition. -You can remove the column hide option using the `disableHiding` columnDef option. You can disable +You can remove the column hide option using the `disableHiding` columnDef option, which will also prevent +this column being hidden in the gridMenu (once it is finished and merged). You can disable the column menu entirely using the `disableColumnMenu` columnDef option. You can supply an icon class with the `icon` property. diff --git a/src/features/cellnav/js/cellnav.js b/src/features/cellnav/js/cellnav.js index 0174457479..40cea094f8 100644 --- a/src/features/cellnav/js/cellnav.js +++ b/src/features/cellnav/js/cellnav.js @@ -354,11 +354,11 @@ var args = {}; if (gridRow !== null) { - args.y = { percentage: gridRow.index / grid.renderContainers.body.visibleRowCache.length }; + args.y = { percentage: grid.renderContainers.body.visibleRowCache.indexOf(gridRow) / grid.renderContainers.body.visibleRowCache.length }; } if (gridCol !== null) { - args.x = { percentage: this.getLeftWidth(grid, gridCol.index) / this.getLeftWidth(grid, grid.renderContainers.body.visibleColumnCache.length - 1) }; + args.x = { percentage: this.getLeftWidth(grid, gridCol) / this.getLeftWidth(grid, grid.renderContainers.body.visibleColumnCache[grid.renderContainers.body.visibleColumnCache.length - 1] ) }; } if (args.y || args.x) { @@ -371,26 +371,37 @@ * @methodOf ui.grid.cellNav.service:uiGridCellNavService * @name getLeftWidth * @description Get the current drawn width of the columns in the - * grid up to and including the numbered column + * grid up to the numbered column, and add an apportionment for the + * column that we're on. So if we are on column 0, we want to scroll + * 0% (i.e. exclude this column from calc). If we're on the last column + * we want to scroll to 100% (i.e. include this column in the calc). So + * we include (thisColIndex / totalNumberCols) % of this column width * @param {Grid} grid the grid you'd like to act upon, usually available * from gridApi.grid - * @param {object} colIndex the column to total up to and including + * @param {gridCol} upToCol the column to total up to and including */ - getLeftWidth: function (grid, colIndex) { + getLeftWidth: function (grid, upToCol) { var width = 0; - if (!colIndex) { - return; + if (!upToCol) { + return width; } - - for (var i = 0; i <= colIndex; i++) { - if (grid.renderContainers.body.visibleColumnCache[i].drawnWidth) { - width += grid.renderContainers.body.visibleColumnCache[i].drawnWidth; + + var lastIndex = grid.renderContainers.body.visibleColumnCache.indexOf( upToCol ); + + // total column widths up-to but not including the passed in column + grid.renderContainers.body.visibleColumnCache.forEach( function( col, index ) { + if ( index < lastIndex ){ + width += col.drawnWidth; } - } + }); + + // pro-rata the final column based on % of total columns. + var percentage = lastIndex === 0 ? 0 : (lastIndex + 1) / grid.renderContainers.body.visibleColumnCache.length; + width += upToCol.drawnWidth * percentage; + return width; } - }; return service; @@ -516,7 +527,6 @@ var rowCol = $scope.colContainer.cellNav.getNextRowCol(direction, $scope.row, $scope.col); - //$log.debug('next row ' + rowCol.row.index + ' next Col ' + rowCol.col.colDef.name); uiGridCtrl.cellNav.broadcastCellNav(rowCol); setTabEnabled(); @@ -534,7 +544,6 @@ $scope.$on(uiGridCellNavConstants.CELL_NAV_EVENT, function (evt, rowCol) { if (rowCol.row === $scope.row && rowCol.col === $scope.col) { - // $log.debug('Setting focus on Row ' + rowCol.row.index + ' Col ' + rowCol.col.colDef.name); setFocused(); } }); diff --git a/src/features/cellnav/test/uiGridCellNavService.spec.js b/src/features/cellnav/test/uiGridCellNavService.spec.js index ff93d23ce7..e6d6edb38d 100644 --- a/src/features/cellnav/test/uiGridCellNavService.spec.js +++ b/src/features/cellnav/test/uiGridCellNavService.spec.js @@ -182,7 +182,7 @@ describe('ui.grid.edit uiGridCellNavService', function () { it('should request scroll to row and column', function () { uiGridCellNavService.scrollTo( grid, $scope, grid.options.data[2], grid.columns[1].colDef); - expect(args).toEqual( { y : { percentage : 2/3 }, x : { percentage : 300/600 } }); + expect(args).toEqual( { y : { percentage : 2/3 }, x : { percentage : (100 + 2/3 * 200)/600 } }); }); it('should request scroll to row only', function () { @@ -192,9 +192,9 @@ describe('ui.grid.edit uiGridCellNavService', function () { }); it('should request scroll to column only', function () { - uiGridCellNavService.scrollTo( grid, $scope, null, grid.columns[1].colDef); + uiGridCellNavService.scrollTo( grid, $scope, null, grid.columns[0].colDef); - expect(args).toEqual( { x : { percentage : 300/600 } }); + expect(args).toEqual( { x : { percentage : 0 } }); }); it('should request no scroll as no row or column', function () { diff --git a/src/features/edit/templates/cellEditor.html b/src/features/edit/templates/cellEditor.html index 4192f82db1..8276227deb 100644 --- a/src/features/edit/templates/cellEditor.html +++ b/src/features/edit/templates/cellEditor.html @@ -1,5 +1,5 @@
- +
diff --git a/src/features/edit/templates/dropdownEditor.html b/src/features/edit/templates/dropdownEditor.html index 2ab3853cdb..d7d6745d73 100644 --- a/src/features/edit/templates/dropdownEditor.html +++ b/src/features/edit/templates/dropdownEditor.html @@ -1,5 +1,5 @@
- +
diff --git a/src/features/resize-columns/js/ui-grid-column-resizer.js b/src/features/resize-columns/js/ui-grid-column-resizer.js index 51b1a20a7c..93ea6810dd 100644 --- a/src/features/resize-columns/js/ui-grid-column-resizer.js +++ b/src/features/resize-columns/js/ui-grid-column-resizer.js @@ -167,12 +167,11 @@ var otherCol = renderContainer.renderedColumns[$scope.renderIndex - 1]; // Don't append the left resizer if this is the first column or the column to the left of this one has resizing disabled - if (otherCol && $scope.col.index !== 0 && otherCol.colDef.enableColumnResizing !== false) { + if (otherCol && renderContainer.visibleColumnCache.indexOf($scope.col) !== 0 && otherCol.colDef.enableColumnResizing !== false) { $elm.prepend(resizerLeft); } // Don't append the right resizer if this column has resizing disabled - //if ($scope.col.index !== $scope.grid.renderedColumns.length - 1 && $scope.col.colDef.enableColumnResizing !== false) { if ($scope.col.colDef.enableColumnResizing !== false) { $elm.append(resizerRight); } @@ -259,7 +258,7 @@ renderContainer.visibleColumnCache.forEach(function (column) { // Skip the column we just resized - if (column.index === col.index) { return; } + if (column === col) { return; } var colDef = column.colDef; if (!colDef.width || (angular.isString(colDef.width) && (colDef.width.indexOf('*') !== -1 || colDef.width.indexOf('%') !== -1))) { @@ -446,7 +445,7 @@ var renderContainerElm = gridUtil.closestElm($elm, '.ui-grid-render-container'); // Get the cell contents so we measure correctly. For the header cell we have to account for the sort icon and the menu buttons, if present - var cells = renderContainerElm.querySelectorAll('.' + uiGridConstants.COL_CLASS_PREFIX + col.index + ' .ui-grid-cell-contents'); + var cells = renderContainerElm.querySelectorAll('.' + uiGridConstants.COL_CLASS_PREFIX + col.uid + ' .ui-grid-cell-contents'); Array.prototype.forEach.call(cells, function (cell) { // Get the cell width // $log.debug('width', gridUtil.elementWidth(cell)); diff --git a/src/features/resize-columns/test/resizeColumns.spec.js b/src/features/resize-columns/test/resizeColumns.spec.js index 45bd0b2430..62fb0e81de 100644 --- a/src/features/resize-columns/test/resizeColumns.spec.js +++ b/src/features/resize-columns/test/resizeColumns.spec.js @@ -18,7 +18,8 @@ describe('ui.grid.resizeColumns', function () { $scope.gridOpts = { enableColumnResizing: true, - data: data + data: data, + onRegisterApi: function(gridApi){ $scope.gridApi = gridApi; } }; recompile = function () { @@ -100,7 +101,7 @@ describe('ui.grid.resizeColumns', function () { xit('should resize the column to the maximum width of the rendered columns', function (done) { var firstResizer = $(grid).find('[ui-grid-column-resizer]').first(); - var colWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + '0').first().width(); + var colWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + $scope.gridApi.grid.columns[0].uid).first().width(); expect(colWidth === 166 || colWidth === 167).toBe(true); // allow for column widths that don't equally divide @@ -108,7 +109,7 @@ describe('ui.grid.resizeColumns', function () { $scope.$digest(); - var newColWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + '0').first().width(); + var newColWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + $scope.gridApi.grid.columns[0].uid).first().width(); // Can't really tell how big the columns SHOULD be, we'll just expect them to be different in width now expect(newColWidth).not.toEqual(colWidth); @@ -137,7 +138,7 @@ describe('ui.grid.resizeColumns', function () { var firstResizer = $(grid).find('[ui-grid-column-resizer]').first(); // Get the initial width of the column - initialWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + '0').first().width(); + initialWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + $scope.gridApi.grid.columns[0].uid).first().width(); initialX = firstResizer.position().left; @@ -176,7 +177,7 @@ describe('ui.grid.resizeColumns', function () { }); it('should cause the column to resize by the amount change in the X axis', function () { - var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + '0').first().width(); + var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + $scope.gridApi.grid.columns[0].uid ).first().width(); expect(newWidth - initialWidth).toEqual(xDiff); }); @@ -212,7 +213,7 @@ describe('ui.grid.resizeColumns', function () { $(firstResizer).simulate('dblclick'); $scope.$digest(); - var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + '0').first().width(); + var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + $scope.gridApi.grid.columns[0].uid).first().width(); expect(newWidth >= minWidth).toEqual(true); }); @@ -233,7 +234,7 @@ describe('ui.grid.resizeColumns', function () { }); it('should not go below the minWidth', function () { - var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + '0').first().width(); + var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + $scope.gridApi.grid.columns[0].uid).first().width(); expect(newWidth >= minWidth).toEqual(true); }); @@ -262,7 +263,7 @@ describe('ui.grid.resizeColumns', function () { $(firstResizer).simulate('dblclick'); $scope.$digest(); - var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + '0').first().width(); + var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + $scope.gridApi.grid.columns[0].uid).first().width(); expect(newWidth <= maxWidth).toEqual(true); }); @@ -283,7 +284,7 @@ describe('ui.grid.resizeColumns', function () { }); it('should not go above the maxWidth', function () { - var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + '0').first().width(); + var newWidth = $(grid).find('.' + uiGridConstants.COL_CLASS_PREFIX + $scope.gridApi.grid.columns[0].uid).first().width(); expect(newWidth <= maxWidth).toEqual(true); }); diff --git a/src/js/core/directives/ui-grid-column-menu.js b/src/js/core/directives/ui-grid-column-menu.js index 874ef54175..2e934fbac8 100644 --- a/src/js/core/directives/ui-grid-column-menu.js +++ b/src/js/core/directives/ui-grid-column-menu.js @@ -89,8 +89,8 @@ angular.module('ui.grid').directive('uiGridColumnMenu', ['$log', '$timeout', '$w * @ngdoc boolean * @name disableHiding * @propertyOf ui.grid.class:GridOptions.columnDef - * @description (optional) True by default. When enabled, this setting allows a user to hide the column - * using the column menu. + * @description (optional) False by default. When enabled, this setting prevents a user from hiding the column + * using the column menu or the grid menu. */ $scope.hideable = function() { if (typeof($scope.col) !== 'undefined' && $scope.col && $scope.col.colDef && $scope.col.colDef.disableHiding) { diff --git a/src/js/core/directives/ui-grid-header-cell.js b/src/js/core/directives/ui-grid-header-cell.js index 95ccc916ce..5458ed8421 100644 --- a/src/js/core/directives/ui-grid-header-cell.js +++ b/src/js/core/directives/ui-grid-header-cell.js @@ -41,13 +41,6 @@ $elm.addClass($scope.col.getColClass(false)); - // shane - No need for watch now that we trackby col name - // $scope.$watch('col.index', function (newValue, oldValue) { - // if (newValue === oldValue) { return; } - // var className = $elm.attr('class'); - // className = className.replace(uiGridConstants.COL_CLASS_PREFIX + oldValue, uiGridConstants.COL_CLASS_PREFIX + newValue); - // $elm.attr('class', className); - // }); // Hide the menu by default $scope.menuShown = false; diff --git a/src/js/core/directives/ui-grid-header.js b/src/js/core/directives/ui-grid-header.js index ecb9be274b..57e34402b5 100644 --- a/src/js/core/directives/ui-grid-header.js +++ b/src/js/core/directives/ui-grid-header.js @@ -129,7 +129,6 @@ column.drawnWidth = column.width; - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + column.width + 'px; }'; } }); @@ -155,8 +154,6 @@ canvasWidth += colWidth; column.drawnWidth = colWidth; - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; - // Remove this element from the percent array so it's not processed below percentArray.splice(i, 1); } @@ -168,8 +165,6 @@ canvasWidth += colWidth; column.drawnWidth = colWidth; - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; - // Remove this element from the percent array so it's not processed below percentArray.splice(i, 1); } @@ -182,8 +177,6 @@ canvasWidth += colWidth; column.drawnWidth = colWidth; - - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; }); } @@ -205,8 +198,6 @@ canvasWidth += colWidth; column.drawnWidth = colWidth; - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; - lastColumn = column; // Remove this element from the percent array so it's not processed below @@ -221,8 +212,6 @@ canvasWidth += colWidth; column.drawnWidth = colWidth; - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; - // Remove this element from the percent array so it's not processed below asterisksArray.splice(i, 1); } @@ -237,8 +226,6 @@ canvasWidth += colWidth; column.drawnWidth = colWidth; - - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; }); } diff --git a/src/js/core/factories/Grid.js b/src/js/core/factories/Grid.js index 2247b0314a..78e198a89e 100644 --- a/src/js/core/factories/Grid.js +++ b/src/js/core/factories/Grid.js @@ -340,6 +340,7 @@ angular.module('ui.grid') .then(function(){ rowHeaderCol.enableFiltering = false; rowHeaderCol.enableSorting = false; + rowHeaderCol.disableHiding = true; self.rowHeaderColumns.push(rowHeaderCol); self.buildColumns() .then( function() { @@ -362,14 +363,17 @@ angular.module('ui.grid') var self = this; var builderPromises = []; var headerOffset = self.rowHeaderColumns.length; - - // Synchronize self.columns with self.options.columnDefs so that columns can also be removed. - self.columns.forEach(function (column, index) { - if (!self.getColDef(column.name)) { - self.columns.splice(index, 1); + var i; + + // Remove any columns for which a columnDef cannot be found + // Deliberately don't use forEach, as it doesn't like splice being called in the middle + // Also don't cache columns.length, as it will change during this operation + for ( i=0; i * */ - GridColumn.prototype.updateColumnDef = function(colDef, index) { + + /** + * @ngdoc method + * @methodOf ui.grid.class:GridColumn + * @name updateColumnDef + * @description Moves settings from the columnDef down onto the column, + * and sets properties as appropriate + * @param {ColumnDef} colDef the column def to look in for the property value + */ + GridColumn.prototype.updateColumnDef = function(colDef) { var self = this; self.colDef = colDef; - //position of column - self.index = (typeof(index) === 'undefined') ? colDef.index : index; - if (colDef.name === undefined) { - throw new Error('colDef.name is required for column at index ' + self.index); + throw new Error('colDef.name is required for column at index ' + self.grid.options.columnDefs.indexOf(colDef)); } var parseErrorMsg = "Cannot parse column width '" + colDef.width + "' for column named '" + colDef.name + "'"; @@ -500,7 +525,7 @@ angular.module('ui.grid') * @param {bool} prefixDot if true, will return .className instead of className */ GridColumn.prototype.getColClass = function (prefixDot) { - var cls = uiGridConstants.COL_CLASS_PREFIX + this.index; + var cls = uiGridConstants.COL_CLASS_PREFIX + this.uid; return prefixDot ? '.' + cls : cls; }; diff --git a/src/js/core/factories/GridRenderContainer.js b/src/js/core/factories/GridRenderContainer.js index 29df1b366b..167c8fcce7 100644 --- a/src/js/core/factories/GridRenderContainer.js +++ b/src/js/core/factories/GridRenderContainer.js @@ -498,8 +498,6 @@ angular.module('ui.grid') canvasWidth = parseInt(canvasWidth, 10) + parseInt(column.width, 10); column.drawnWidth = column.width; - - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + column.width + 'px; }'; } }); @@ -525,8 +523,6 @@ angular.module('ui.grid') canvasWidth += colWidth; column.drawnWidth = colWidth; - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; - // Remove this element from the percent array so it's not processed below percentArray.splice(i, 1); } @@ -538,8 +534,6 @@ angular.module('ui.grid') canvasWidth += colWidth; column.drawnWidth = colWidth; - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; - // Remove this element from the percent array so it's not processed below percentArray.splice(i, 1); } @@ -552,8 +546,6 @@ angular.module('ui.grid') canvasWidth += colWidth; column.drawnWidth = colWidth; - - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; }); } @@ -575,8 +567,6 @@ angular.module('ui.grid') canvasWidth += colWidth; column.drawnWidth = colWidth; - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; - lastColumn = column; // Remove this element from the percent array so it's not processed below @@ -591,8 +581,6 @@ angular.module('ui.grid') canvasWidth += colWidth; column.drawnWidth = colWidth; - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; - // Remove this element from the percent array so it's not processed below asterisksArray.splice(i, 1); } @@ -607,8 +595,6 @@ angular.module('ui.grid') canvasWidth += colWidth; column.drawnWidth = colWidth; - - // ret = ret + ' .grid' + uiGridCtrl.grid.id + ' .col' + column.index + ' { width: ' + colWidth + 'px; }'; }); } diff --git a/src/js/core/factories/GridRow.js b/src/js/core/factories/GridRow.js index 54a31079e7..7d8e3e097c 100644 --- a/src/js/core/factories/GridRow.js +++ b/src/js/core/factories/GridRow.js @@ -30,15 +30,6 @@ angular.module('ui.grid') */ this.entity = entity; - /** - * @ngdoc object - * @name index - * @propertyOf ui.grid.class:GridRow - * @description the index of the GridRow. It should always be unique and immutable - */ - this.index = index; - - /** * @ngdoc object * @name uid