fix(core): fix angular-ui#4180 by adding validity check for minWidth and maxWidth.

- String and number are both allowed.

- Valid String value will parse into number properly.

- Invalid String value will throw an error.

Author: merlinchen

src/js/core/factories/GridColumn.js

@@ -464,8 +464,22 @@ angular.module('ui.grid')
       }
     }
 
-    self.minWidth = !colDef.minWidth ? 30 : colDef.minWidth;
-    self.maxWidth = !colDef.maxWidth ? 9000 : colDef.maxWidth;
+    ['minWidth', 'maxWidth'].forEach(function (name) {
+      var minOrMaxWidth = colDef[name];
+      var parseErrorMsg = "Cannot parse column " + name + " '" + minOrMaxWidth + "' for column named '" + colDef.name + "'";
+
+      if (!angular.isString(minOrMaxWidth) && !angular.isNumber(minOrMaxWidth)) {
+        self[name] = ((name === 'minWidth') ? 30 : 9000);
+      } else if (angular.isString(minOrMaxWidth)) {
+        if (minOrMaxWidth.match(/^(\d+)$/)) {
+          self[name] = parseInt(minOrMaxWidth.match(/^(\d+)$/)[1], 10);
+        } else {
+          throw new Error(parseErrorMsg);
+        }
+      } else {
+        self[name] = minOrMaxWidth;
+      }
+    });
 
     //use field if it is defined; name if it is not
     self.field = (colDef.field === undefined) ? colDef.name : colDef.field;

test/unit/core/factories/GridColumn.spec.js

@@ -30,7 +30,7 @@ describe('GridColumn factory', function () {
 
     buildCols();
   }));
-  
+
   describe('buildColumns', function () {
     it('should not remove existing sort details on a column', function () {
       var sort = { priority: 0, direction: 'asc' };
@@ -192,7 +192,7 @@ describe('GridColumn factory', function () {
       expect(col.colDef.visible).toBe(false);
     });
   });
-  
+
   describe('aggregation', function() {
     beforeEach( function() {
       grid.options.data = [
@@ -202,19 +202,19 @@ describe('GridColumn factory', function () {
         { name: 'matthew', value: 4 },
         { name: 'murray', value: 5 }
       ];
-      grid.options.columnDefs = [ {name: 'name'}, {name: 'value'}];  
+      grid.options.columnDefs = [ {name: 'name'}, {name: 'value'}];
     });
-    
+
     it('count, with label', function() {
       grid.options.columnDefs[0].aggregationType = uiGridConstants.aggregationTypes.count;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[0].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[0].getAggregationValue()).toEqual(5);
       expect(grid.columns[0].getAggregationText()).toEqual('total rows: ');
       deregFn();
@@ -223,29 +223,29 @@ describe('GridColumn factory', function () {
     it('count, without label', function() {
       grid.options.columnDefs[0].aggregationType = uiGridConstants.aggregationTypes.count;
       grid.options.columnDefs[0].aggregationHideLabel = true;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[0].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[0].getAggregationValue()).toEqual(5);
       expect(grid.columns[0].getAggregationText()).toEqual('');
       deregFn();
     });
 
     it('sum, with label', function() {
       grid.options.columnDefs[1].aggregationType = uiGridConstants.aggregationTypes.sum;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[1].getAggregationValue()).toEqual(15);
       expect(grid.columns[1].getAggregationText()).toEqual('total: ');
       deregFn();
@@ -254,29 +254,29 @@ describe('GridColumn factory', function () {
     it('sum, without label', function() {
       grid.options.columnDefs[1].aggregationType = uiGridConstants.aggregationTypes.sum;
       grid.options.columnDefs[1].aggregationHideLabel = true;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[1].getAggregationValue()).toEqual(15);
       expect(grid.columns[1].getAggregationText()).toEqual('');
       deregFn();
     });
 
     it('avg, with label', function() {
       grid.options.columnDefs[1].aggregationType = uiGridConstants.aggregationTypes.avg;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[1].getAggregationValue()).toEqual(3);
       expect(grid.columns[1].getAggregationText()).toEqual('avg: ');
       deregFn();
@@ -285,29 +285,29 @@ describe('GridColumn factory', function () {
     it('avg, without label', function() {
       grid.options.columnDefs[1].aggregationType = uiGridConstants.aggregationTypes.avg;
       grid.options.columnDefs[1].aggregationHideLabel = true;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[1].getAggregationValue()).toEqual(3);
       expect(grid.columns[1].getAggregationText()).toEqual('');
       deregFn();
-    });    
+    });
 
     it('min, with label', function() {
       grid.options.columnDefs[1].aggregationType = uiGridConstants.aggregationTypes.min;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[1].getAggregationValue()).toEqual(1);
       expect(grid.columns[1].getAggregationText()).toEqual('min: ');
       deregFn();
@@ -316,29 +316,29 @@ describe('GridColumn factory', function () {
     it('min, without label', function() {
       grid.options.columnDefs[1].aggregationType = uiGridConstants.aggregationTypes.min;
       grid.options.columnDefs[1].aggregationHideLabel = true;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[1].getAggregationValue()).toEqual(1);
       expect(grid.columns[1].getAggregationText()).toEqual('');
       deregFn();
     });
 
     it('max, with label', function() {
       grid.options.columnDefs[1].aggregationType = uiGridConstants.aggregationTypes.max;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[1].getAggregationValue()).toEqual(5);
       expect(grid.columns[1].getAggregationText()).toEqual('max: ');
       deregFn();
@@ -347,14 +347,14 @@ describe('GridColumn factory', function () {
     it('max, without label', function() {
       grid.options.columnDefs[1].aggregationType = uiGridConstants.aggregationTypes.max;
       grid.options.columnDefs[1].aggregationHideLabel = true;
-      
+
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
-      
+
       expect(grid.columns[1].getAggregationValue()).toEqual(5);
       expect(grid.columns[1].getAggregationText()).toEqual('');
       deregFn();
@@ -370,7 +370,7 @@ describe('GridColumn factory', function () {
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
 
@@ -389,7 +389,7 @@ describe('GridColumn factory', function () {
       buildCols();
       grid.modifyRows(grid.options.data);
 
-      // this would be called by the footer cell if it were rendered 
+      // this would be called by the footer cell if it were rendered
       var deregFn = grid.api.core.on.rowsRendered(null, grid.columns[1].updateAggregationValue);
       grid.setVisibleRows(grid.rows);
 
@@ -408,13 +408,13 @@ describe('GridColumn factory', function () {
         { name: 'matthew', value: 4 },
         { name: 'murray', value: 5 }
       ];
-      grid.options.columnDefs = [ {name: 'name'}, {name: 'value'}];  
+      grid.options.columnDefs = [ {name: 'name'}, {name: 'value'}];
     });
-    
+
     it('raise event', function() {
       var sortChanged = false;
       grid.api.core.on.sortChanged( $scope, function(){ sortChanged = true; });
-      
+
       grid.columns[0].unsort();
       expect( sortChanged ).toEqual(true);
     });
@@ -560,6 +560,52 @@ describe('GridColumn factory', function () {
       expect(updateCol(colDef.width)).toThrow();
     });
 
+    it ('should set the value of minWidth to 30 when colDef.minWidth is undefined', invalidMinOrMaxWidthDef(undefined, 'minWidth'));
+    it ('should set the value of minWidth to 30 when colDef.minWidth is null', invalidMinOrMaxWidthDef(null, 'minWidth'));
+    it ('should set the value of minWidth to 30 when colDef.minWidth is an object', invalidMinOrMaxWidthDef({}, 'minWidth'));
+
+    it ('should set the value of minWidth to the parsed integer colDef.minWidth when it is a string', function () {
+      colDef.minWidth = '90';
+      col.updateColumnDef(colDef);
+      expect(col.minWidth).toBe(90);
+    });
+
+    it ('should set the value of minWidth to colDef.minWidth when it is a number', function () {
+      colDef.minWidth = 90;
+      col.updateColumnDef(colDef);
+      expect(col.minWidth).toBe(90);
+    });
+
+    it ('should throw when colDef.minWidth is an invalid string', function () {
+      colDef.minWidth = 'e1%';
+      expect(col.updateColumnDef(colDef)).toThrow();
+      colDef.minWidth = '#FFF';
+      expect(col.updateColumnDef(colDef)).toThrow();
+    });
+
+    it ('should set the value of maxWidth to 9000 when colDef.maxWidth is undefined', invalidMinOrMaxWidthDef(undefined, 'maxWidth'));
+    it ('should set the value of maxWidth to 9000 when colDef.maxWidth is null', invalidMinOrMaxWidthDef(null, 'maxWidth'));
+    it ('should set the value of maxWidth to 9000 when colDef.maxWidth is an object', invalidMinOrMaxWidthDef({}, 'maxWidth'));
+
+    it ('should set the value of maxWidth to the parsed integer colDef.maxWidth when it is a string', function () {
+      colDef.maxWidth = '200';
+      col.updateColumnDef(colDef);
+      expect(col.maxWidth).toBe(200);
+    });
+
+    it ('should set the value of maxWidth to colDef.maxWidth when it is a number', function () {
+      colDef.maxWidth = 200;
+      col.updateColumnDef(colDef);
+      expect(col.maxWidth).toBe(200);
+    });
+
+    it ('should throw when colDef.maxWidth is an invalid string', function () {
+      colDef.maxWidth = 'e1%';
+      expect(col.updateColumnDef(colDef)).toThrow();
+      colDef.maxWidth = '#FFF';
+      expect(col.updateColumnDef(colDef)).toThrow();
+    });
+
     function widthEqualsColDefWidth(expected) {
       return function () {
         colDef.width = expected;
@@ -581,5 +627,13 @@ describe('GridColumn factory', function () {
         col.updateColumnDef(colDef);
       };
     }
+
+    function invalidMinOrMaxWidthDef(width, minOrMax) {
+      return function () {
+        colDef[minOrMax] = width;
+        col.updateColumnDef(colDef);
+        expect(col[minOrMax]).toBe(minOrMax === 'minWidth' ? 30 : 9000);
+      };
+    }
   });
-});
+});