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.
angular-ui · Oct 3, 2014 · e8e31ce · e8e31ce
1 parent 7454408
commit e8e31ce
Show file tree

Hide file tree

Showing 14 changed files with 96 additions and 98 deletions.
diff --git 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
@@ -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
@@ -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
@@ -1,5 +1,5 @@
 <div>
     <form name="inputForm">
-        <input type="{{inputType}}" ng-class="'colt' + col.index" ui-grid-editor ng-model="MODEL_COL_FIELD" />
+        <input type="{{inputType}}" ng-class="'colt' + col.uid" ui-grid-editor ng-model="MODEL_COL_FIELD" />
     </form>
 </div>
diff --git a/src/features/edit/templates/dropdownEditor.html b/src/features/edit/templates/dropdownEditor.html
@@ -1,5 +1,5 @@
 <div>
   <form name="inputForm">
-    <select ng-class="'colt' + col.index" ui-grid-edit-dropdown ng-model="MODEL_COL_FIELD" ng-options="field[editDropdownIdLabel] as field[editDropdownValueLabel] CUSTOM_FILTERS for field in editDropdownOptionsArray"></select>
+    <select ng-class="'colt' + col.uid" ui-grid-edit-dropdown ng-model="MODEL_COL_FIELD" ng-options="field[editDropdownIdLabel] as field[editDropdownValueLabel] CUSTOM_FILTERS for field in editDropdownOptionsArray"></select>
   </form>
 </div>
diff --git 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
@@ -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,15 +101,15 @@ 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 
 
       firstResizer.trigger('dblclick');
 
       $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
@@ -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
@@ -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
@@ -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
@@ -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,38 +363,43 @@ 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<self.columns.length; i++ ){
+      if (!self.getColDef(self.columns[i].name)) {
+        self.columns.splice(i, 1);
+        i--;
       }
-    });
-
+    }
 
     //add row header columns to the grid columns array _after_ columns without columnDefs have been removed
     self.rowHeaderColumns.forEach(function (rowHeaderColumn) {
       self.columns.unshift(rowHeaderColumn);
     });
 
 
+    // look at each column def, and update column properties to match.  If the column def
+    // doesn't have a column, then splice in a new gridCol
     self.options.columnDefs.forEach(function (colDef, index) {
       self.preprocessColDef(colDef);
       var col = self.getColumn(colDef.name);
 
       if (!col) {
-        col = new GridColumn(colDef, index, self);
+        col = new GridColumn(colDef, gridUtil.nextUid(), self);
         self.columns.splice(index + headerOffset, 0, col);
       }
       else {
-        col.updateColumnDef(colDef, col.index);
+        col.updateColumnDef(colDef);
       }
 
       self.columnBuilders.forEach(function (builder) {
         builderPromises.push(builder.call(self, colDef, col, self.options));
       });
     });
-
+    
     return $q.all(builderPromises);
   };