invisi/angular.js
1
0
Fork 0
Code Issues Pull Requests Actions Packages Releases Wiki Activity

fix(input[range]): correctly handle min/max; remove ngMin/ngMax support

Browse Source 
This commit fixes the handling of min/max, and removes support for ngMin/ngMax:

min/max:
Previously, interpolated min/max values on input range were not set when the first $render happens,
because the interpolation directive only sets the actual element attribute value after
a digest passes. That means that the browser would not adjust
the input value according to min/max and the range input and model would
not be initialzed as expected. 
With this change, input range will set the actual element attribute value during its own
linking phase, as it is already available on the attrs argument passed to the link fn.

ngMin/ngMax
Since ng prefixed attributes do not set their corresponding element attribute, the range input would always have min = 0, and max = 100 (in supported browsers), regardless of the value
in ngMin/ngMax. This is confusing and not very useful, so it's better to not support these attributes at all.

The commit also fixes a test which used an interpolation inside an attribute that expects an expression.

Fixes #14982 
PR (#14996)
This commit is contained in:
Martin Staffa
2016-08-12 19:58:08 +02:00
committed by Martin Staffa
parent af78d45dc0
commit b78539bfc0
3 changed files with 165 additions and 149 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ng/directive/input.js
+49 -51
View File
@@ -1061,8 +1061,11 @@ var inputType = {
* Angular will also update the model value.
*
* Automatic value adjustment also means that a range input element can never have the `required`,
* `min`, or `max` errors, except when using `ngMax` and `ngMin`, which are not affected by automatic
* value adjustment, because they do not set the `min` and `max` attributes.
* `min`, or `max` errors.
*
* Note that `input[range]` is not compatible with`ngMax` and `ngMin`, because they do not set the
* `min` and `max` attributes, which means that the browser won't automatically adjust the input
* value based on their values, and will always assume min = 0 and max = 100.
*
* @param {string} ngModel Assignable angular expression to data-bind to.
* @param {string=} name Property name of the form under which the control is published.
@@ -1070,14 +1073,6 @@ var inputType = {
* than `min`. Can be interpolated.
* @param {string=} max Sets the `max` validation to ensure that the value entered is less than `max`.
* Can be interpolated.
* @param {string=} ngMin Takes an expression. Sets the `min` validation to ensure that the value
* entered is greater than `min`. Does not set the `min` attribute and therefore
* adds no native HTML5 validation. It also means the browser won't adjust the
* element value in case `min` is greater than the current value.
* @param {string=} ngMax Takes an expression. Sets the `max` validation to ensure that the value
* entered is less than `max`. Does not set the `max` attribute and therefore
* adds no native HTML5 validation. It also means the browser won't adjust the
* element value in case `max` is less than the current value.
* @param {string=} ngChange Angular expression to be executed when the ngModel value changes due
* to user interaction with the input element.
*
@@ -1547,10 +1542,12 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
numberFormatterParser(ctrl);
baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
var minVal = 0,
maxVal = 100,
supportsRange = ctrl.$$hasNativeValidators && element[0].type === 'range',
validity = element[0].validity;
var supportsRange = ctrl.$$hasNativeValidators && element[0].type === 'range',
minVal = supportsRange ? 0 : undefined,
maxVal = supportsRange ? 100 : undefined,
validity = element[0].validity,
hasMinAttr = isDefined(attr.min),
hasMaxAttr = isDefined(attr.max);
var originalRender = ctrl.$render;
@@ -1563,6 +1560,39 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
} :
originalRender;
if (hasMinAttr) {
ctrl.$validators.min = supportsRange ?
// Since all browsers set the input to a valid value, we don't need to check validity
function noopMinValidator() { return true; } :
// non-support browsers validate the range
function minValidator(modelValue, viewValue) {
return ctrl.$isEmpty(viewValue) || isUndefined(minVal) || viewValue >= minVal;
};
setInitialValueAndObserver('min', minChange);
}
if (hasMaxAttr) {
ctrl.$validators.max = supportsRange ?
// Since all browsers set the input to a valid value, we don't need to check validity
function noopMaxValidator() { return true; } :
// ngMax doesn't set the max attr, so the browser doesn't adjust the input value as setting max would
function maxValidator(modelValue, viewValue) {
return ctrl.$isEmpty(viewValue) || isUndefined(maxVal) || viewValue <= maxVal;
};
setInitialValueAndObserver('max', maxChange);
}
function setInitialValueAndObserver(htmlAttrName, changeFn) {
// interpolated attributes set the attribute value only after a digest, but we need the
// attribute value when the input is first rendered, so that the browser can adjust the
// input value based on the min/max value
element.attr(htmlAttrName, attr[htmlAttrName]);
attr.$observe(htmlAttrName, changeFn);
}
function minChange(val) {
if (isDefined(val) && !isNumber(val)) {
val = parseFloat(val);
@@ -1573,12 +1603,12 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
return;
}
if (supportsRange && minAttrType === 'min') {
if (supportsRange) {
var elVal = element.val();
// IE11 doesn't set the el val correctly if the minVal is greater than the element value
if (minVal > elVal) {
element.val(minVal);
elVal = minVal;
element.val(elVal);
}
ctrl.$setViewValue(elVal);
} else {
@@ -1587,23 +1617,6 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
}
}
var minAttrType = isDefined(attr.ngMin) ? 'ngMin' : isDefined(attr.min) ? 'min' : false;
if (minAttrType) {
ctrl.$validators.min = isDefined(attr.min) && supportsRange ?
function noopMinValidator(value) {
// Since all browsers set the input to a valid value, we don't need to check validity
return true;
} :
// ngMin doesn't set the min attr, so the browser doesn't adjust the input value as setting min would
function minValidator(modelValue, viewValue) {
return ctrl.$isEmpty(viewValue) || isUndefined(minVal) || viewValue >= minVal;
};
// Assign minVal when the directive is linked. This won't run the validators as the model isn't ready yet
minChange(attr.min);
attr.$observe('min', minChange);
}
function maxChange(val) {
if (isDefined(val) && !isNumber(val)) {
val = parseFloat(val);
@@ -1614,12 +1627,13 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
return;
}
if (supportsRange && maxAttrType === 'max') {
if (supportsRange) {
var elVal = element.val();
// IE11 doesn't set the el val correctly if the maxVal is less than the element value
if (maxVal < elVal) {
element.val(maxVal);
elVal = minVal;
// IE11 and Chrome don't set the value to the minVal when max < min
elVal = maxVal < minVal ? minVal : maxVal;
}
ctrl.$setViewValue(elVal);
} else {
@@ -1627,22 +1641,6 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
ctrl.$validate();
}
}
var maxAttrType = isDefined(attr.max) ? 'max' : attr.ngMax ? 'ngMax' : false;
if (maxAttrType) {
ctrl.$validators.max = isDefined(attr.max) && supportsRange ?
function noopMaxValidator() {
// Since all browsers set the input to a valid value, we don't need to check validity
return true;
} :
// ngMax doesn't set the max attr, so the browser doesn't adjust the input value as setting max would
function maxValidator(modelValue, viewValue) {
return ctrl.$isEmpty(viewValue) || isUndefined(maxVal) || viewValue <= maxVal;
};
// Assign maxVal when the directive is linked. This won't run the validators as the model isn't ready yet
maxChange(attr.max);
attr.$observe('max', maxChange);
}
}
test/ng/directive/inputSpec.js
+115 -97
View File
@@ -2819,7 +2819,7 @@ describe('input', function() {
expect(inputElm.val()).toEqual('50');
});
it('should set model to 50 when no value specified', function() {
it('should set model to 50 when no value specified and default min/max', function() {
var inputElm = helper.compileInput('<input type="range" ng-model="age" />');
expect(inputElm.val()).toBe('50');
@@ -2829,7 +2829,7 @@ describe('input', function() {
expect(scope.age).toBe(50);
});
it('should parse non-number values to 50', function() {
it('should parse non-number values to 50 when default min/max', function() {
var inputElm = helper.compileInput('<input type="range" ng-model="age" />');
scope.$apply('age = 10');
@@ -2891,8 +2891,20 @@ describe('input', function() {
describe('min', function() {
if (supportsRange) {
it('should initialize correctly with non-default model and min value', function() {
scope.value = -3;
scope.min = -5;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" min="{{min}}" />');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('-3');
expect(scope.value).toBe(-3);
expect(scope.form.alias.$error.min).toBeFalsy();
});
// Browsers that implement range will never allow you to set the value < min values
it('should validate', function() {
it('should adjust invalid input values', function() {
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" min="10" />');
helper.changeInputValueTo('5');
@@ -2906,6 +2918,22 @@ describe('input', function() {
expect(scope.form.alias.$error.min).toBeFalsy();
});
it('should set the model to the min val if it is less than the min val', function() {
scope.value = -10;
// Default min is 0
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" min="{{min}}" />');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('0');
expect(scope.value).toBe(0);
scope.$apply('value = 5; min = 10');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('10');
expect(scope.value).toBe(10);
});
it('should adjust the element and model value when the min value changes on-the-fly', function() {
scope.min = 10;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" min="{{min}}" />');
@@ -2939,8 +2967,9 @@ describe('input', function() {
});
} else {
// input[type=range] will become type=text in browsers that don't support it
it('should validate if "range" is not implemented', function() {
// This will become type=text in browsers that don't support it
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" min="10" />');
helper.changeInputValueTo('5');
@@ -2954,6 +2983,34 @@ describe('input', function() {
expect(scope.form.alias.$error.min).toBeFalsy();
});
it('should not assume a min val of 0 if the min interpolates to a non-number', function() {
scope.value = -10;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" min="{{min}}" />');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('-10');
expect(scope.value).toBe(-10);
expect(scope.form.alias.$error.min).toBeFalsy();
helper.changeInputValueTo('-5');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('-5');
expect(scope.value).toBe(-5);
expect(scope.form.alias.$error.min).toBeFalsy();
scope.$apply('max = "null"');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('-5');
expect(scope.value).toBe(-5);
expect(scope.form.alias.$error.max).toBeFalsy();
scope.$apply('max = "asdf"');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('-5');
expect(scope.value).toBe(-5);
expect(scope.form.alias.$error.max).toBeFalsy();
});
it('should validate even if the min value changes on-the-fly', function() {
scope.min = 10;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" min="{{min}}" />');
@@ -2990,52 +3047,21 @@ describe('input', function() {
}
});
describe('ngMin', function() {
it('should validate', function() {
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" ng-min="50" />');
helper.changeInputValueTo('1');
expect(inputElm).toBeInvalid();
expect(scope.value).toBeFalsy();
expect(scope.form.alias.$error.min).toBeTruthy();
helper.changeInputValueTo('100');
expect(inputElm).toBeValid();
expect(scope.value).toBe(100);
expect(scope.form.alias.$error.min).toBeFalsy();
});
it('should validate even if the ngMin value changes on-the-fly', function() {
scope.min = 10;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" ng-min="min" />');
helper.changeInputValueTo('15');
expect(inputElm).toBeValid();
scope.min = 20;
scope.$digest();
expect(inputElm).toBeInvalid();
scope.min = null;
scope.$digest();
expect(inputElm).toBeValid();
scope.min = '20';
scope.$digest();
expect(inputElm).toBeInvalid();
scope.min = 'abc';
scope.$digest();
expect(inputElm).toBeValid();
});
});
describe('max', function() {
if (supportsRange) {
// Browsers that implement range will never allow you to set the value > max value
it('should initialize correctly with non-default model and max value', function() {
scope.value = 130;
scope.max = 150;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" max="{{max}}" />');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('130');
expect(scope.value).toBe(130);
expect(scope.form.alias.$error.max).toBeFalsy();
});
it('should validate', function() {
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" max="10" />');
@@ -3050,9 +3076,16 @@ describe('input', function() {
expect(scope.form.alias.$error.max).toBeFalsy();
});
it('should set the model to the max val if it is more than the max val', function() {
scope.value = 90;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" max="10" />');
it('should set the model to the max val if it is greater than the max val', function() {
scope.value = 110;
// Default max is 100
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" max="{{max}}" />');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('100');
expect(scope.value).toBe(100);
scope.$apply('value = 90; max = 10');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('10');
@@ -3106,6 +3139,34 @@ describe('input', function() {
expect(scope.form.alias.$error.max).toBeFalsy();
});
it('should not assume a max val of 100 if the max attribute interpolates to a non-number', function() {
scope.value = 120;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" max="{{max}}" />');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('120');
expect(scope.value).toBe(120);
expect(scope.form.alias.$error.max).toBeFalsy();
helper.changeInputValueTo('140');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('140');
expect(scope.value).toBe(140);
expect(scope.form.alias.$error.max).toBeFalsy();
scope.$apply('max = null');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('140');
expect(scope.value).toBe(140);
expect(scope.form.alias.$error.max).toBeFalsy();
scope.$apply('max = "asdf"');
expect(inputElm).toBeValid();
expect(inputElm.val()).toBe('140');
expect(scope.value).toBe(140);
expect(scope.form.alias.$error.max).toBeFalsy();
});
it('should validate even if the max value changes on-the-fly', function() {
scope.max = 10;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" max="{{max}}" />');
@@ -3141,68 +3202,25 @@ describe('input', function() {
}
});
describe('ngMax', function() {
it('should validate', function() {
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" ng-max="5" />');
helper.changeInputValueTo('20');
expect(inputElm).toBeInvalid();
expect(scope.value).toBeUndefined();
expect(scope.form.alias.$error.max).toBeTruthy();
helper.changeInputValueTo('0');
expect(inputElm).toBeValid();
expect(scope.value).toBe(0);
expect(scope.form.alias.$error.max).toBeFalsy();
});
it('should validate even if the ngMax value changes on-the-fly', function() {
scope.max = 10;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" ng-max="max" />');
helper.changeInputValueTo('5');
expect(inputElm).toBeValid();
scope.max = 0;
scope.$digest();
expect(inputElm).toBeInvalid();
scope.max = null;
scope.$digest();
expect(inputElm).toBeValid();
scope.max = '4';
scope.$digest();
expect(inputElm).toBeInvalid();
scope.max = 'abc';
scope.$digest();
expect(inputElm).toBeValid();
});
});
if (supportsRange) {
describe('min and max', function() {
it('should keep the initial default value when min and max are specified', function() {
it('should set the correct initial value when min and max are specified', function() {
scope.max = 80;
scope.min = 40;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" max="{{max}}" min="{{min}}" />');
expect(inputElm.val()).toBe('50');
expect(scope.value).toBe(50);
expect(inputElm.val()).toBe('60');
expect(scope.value).toBe(60);
});
it('should set element and model value to min if max is less than min', function() {
scope.min = 40;
var inputElm = helper.compileInput('<input type="range" ng-model="value" name="alias" max="{{max}}" min="{{min}}" />');
expect(inputElm.val()).toBe('50');
expect(scope.value).toBe(50);
expect(inputElm.val()).toBe('70');
expect(scope.value).toBe(70);
scope.max = 20;
scope.$digest();
test/ng/directive/validatorsSpec.js
+1 -1
View File
@@ -351,7 +351,7 @@ describe('validators', function() {
it('should accept values of any length when maxlength is non-numeric', function() {
var inputElm = helper.compileInput('<input type="text" ng-model="value" ng-maxlength="{{maxlength}}" />');
var inputElm = helper.compileInput('<input type="text" ng-model="value" ng-maxlength="maxlength" />');
helper.changeInputValueTo('aaaaaaaaaa');
$rootScope.$apply('maxlength = "5"');