Merge pull request #1705 from PaulL1/1685_replace_col_index

PaulL1 · PaulL1 · commit 5d590d8d2fd2 · 2014-10-04T10:02:13.000+10:00
Fix #1685: Remove col.index from most code, add col.uid
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);
   };
 
diff --git a/src/js/core/factories/GridColumn.js b/src/js/core/factories/GridColumn.js
diff --git a/src/js/core/factories/GridRenderContainer.js b/src/js/core/factories/GridRenderContainer.js
diff --git a/src/js/core/factories/GridRow.js b/src/js/core/factories/GridRow.js
