From b8f9be0384d1f082308f5b1d84847eb7ae391203 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Sat, 30 Jul 2016 16:58:35 +0200 Subject: [PATCH 1/2] feat(input[range]): support step Step support works like min / max, but with the following caveat. Currently, only Firefox fully implements the spec. Other browsers (Chrome, Safari, Edge) have issues when the step value changes after the input has been changed. They do not adjust the input value to a valid value, but instead set the stepMismatch validity state. Angular will take this validity state, and forward it as the ngModel "step" error. Adjusting the error ourselves would add too much code, as the logic is quite involved. --- src/ng/directive/input.js | 94 +++++++++++++++------ test/ng/directive/inputSpec.js | 145 +++++++++++++++++++++++++++++++++ 2 files changed, 212 insertions(+), 27 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 94978da4e7e9..dcc9567b1303 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1037,13 +1037,19 @@ var inputType = { * The model for the range input must always be a `Number`. * * IE9 and other browsers that do not support the `range` type fall back - * to a text input. Model binding, validation and number parsing are nevertheless supported. + * to a text input without any default values for `min`, `max` and `step`. Model binding, + * validation and number parsing are nevertheless supported. * * Browsers that support range (latest Chrome, Safari, Firefox, Edge) treat `input[range]` * in a way that never allows the input to hold an invalid value. That means: * - any non-numerical value is set to `(max + min) / 2`. * - any numerical value that is less than the current min val, or greater than the current max val * is set to the min / max val respectively. + * - additionally, the current `step` is respected, so the nearest value that satisfies a step + * is used. + * + * See the [HTML Spec on input[type=range]](https://www.w3.org/TR/html5/forms.html#range-state-(type=range)) + * for more info. * * This has the following consequences for Angular: * @@ -1056,16 +1062,21 @@ var inputType = { * That means the model for range will immediately be set to `50` after `ngModel` has been * initialized. It also means a range input can never have the required error. * - * This does not only affect changes to the model value, but also to the values of the `min` and - * `max` attributes. When these change in a way that will cause the browser to modify the input value, - * Angular will also update the model value. + * This does not only affect changes to the model value, but also to the values of the `min`, + * `max`, and `step` attributes. When these change in a way that will cause the browser to modify + * the input value, 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. * - * 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. + * However, `step` is currently only fully implemented by Firefox. Other browsers have problems + * when the step value changes dynamically - they do not adjust the element value correctly, but + * instead may set the `stepMismatch` error. If that's the case, the Angular will set the `step` + * error on the input, and set the model to `undefined`. + * + * Note that `input[range]` is not compatible with`ngMax`, `ngMin`, and `ngStep`, 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, max = 100, and step = 1. * * @param {string} ngModel Assignable angular expression to data-bind to. * @param {string=} name Property name of the form under which the control is published. @@ -1073,6 +1084,8 @@ 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=} step Sets the `step` validation to ensure that the value entered matches the `step` + * Can be interpolated. * @param {string=} ngChange Angular expression to be executed when the ngModel value changes due * to user interaction with the input element. * @@ -1499,6 +1512,13 @@ function numberFormatterParser(ctrl) { }); } +function parseNumberAttrVal(val) { + if (isDefined(val) && !isNumber(val)) { + val = parseFloat(val); + } + return isNumber(val) && !isNaN(val) ? val : undefined; +} + function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { badInputChecker(scope, element, attr, ctrl); numberFormatterParser(ctrl); @@ -1511,10 +1531,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { }; attr.$observe('min', function(val) { - if (isDefined(val) && !isNumber(val)) { - val = parseFloat(val); - } - minVal = isNumber(val) && !isNaN(val) ? val : undefined; + minVal = parseNumberAttrVal(val); // TODO(matsko): implement validateLater to reduce number of validations ctrl.$validate(); }); @@ -1527,10 +1544,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { }; attr.$observe('max', function(val) { - if (isDefined(val) && !isNumber(val)) { - val = parseFloat(val); - } - maxVal = isNumber(val) && !isNaN(val) ? val : undefined; + maxVal = parseNumberAttrVal(val); // TODO(matsko): implement validateLater to reduce number of validations ctrl.$validate(); }); @@ -1545,9 +1559,11 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) { var supportsRange = ctrl.$$hasNativeValidators && element[0].type === 'range', minVal = supportsRange ? 0 : undefined, maxVal = supportsRange ? 100 : undefined, + stepVal = supportsRange ? 1 : undefined, validity = element[0].validity, hasMinAttr = isDefined(attr.min), - hasMaxAttr = isDefined(attr.max); + hasMaxAttr = isDefined(attr.max), + hasStepAttr = isDefined(attr.step); var originalRender = ctrl.$render; @@ -1564,7 +1580,7 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) { 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 + // non-support browsers validate the min val function minValidator(modelValue, viewValue) { return ctrl.$isEmpty(viewValue) || isUndefined(minVal) || viewValue >= minVal; }; @@ -1576,7 +1592,7 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) { 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 + // non-support browsers validate the max val function maxValidator(modelValue, viewValue) { return ctrl.$isEmpty(viewValue) || isUndefined(maxVal) || viewValue <= maxVal; }; @@ -1584,20 +1600,32 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) { setInitialValueAndObserver('max', maxChange); } + if (hasStepAttr) { + ctrl.$validators.step = supportsRange ? + function nativeStepValidator() { + // Currently, only FF implements the spec on step change correctly (i.e. adjusting the + // input element value to a valid value). It's possible that other browsers set the stepMismatch + // validity error instead, so we can at least report an error in that case. + return !validity.stepMismatch; + } : + // ngStep doesn't set the setp attr, so the browser doesn't adjust the input value as setting step would + function stepValidator(modelValue, viewValue) { + return ctrl.$isEmpty(viewValue) || isUndefined(stepVal) || viewValue % stepVal === 0; + }; + + setInitialValueAndObserver('step', stepChange); + } + 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); - } - minVal = isNumber(val) && !isNaN(val) ? val : undefined; + minVal = parseNumberAttrVal(val); // ignore changes before model is initialized if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) { return; @@ -1618,10 +1646,7 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) { } function maxChange(val) { - if (isDefined(val) && !isNumber(val)) { - val = parseFloat(val); - } - maxVal = isNumber(val) && !isNaN(val) ? val : undefined; + maxVal = parseNumberAttrVal(val); // ignore changes before model is initialized if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) { return; @@ -1642,6 +1667,21 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) { } } + function stepChange(val) { + stepVal = parseNumberAttrVal(val); + // ignore changes before model is initialized + if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) { + return; + } + + // Some browsers don't adjust the input value correctly, but set the stepMismatch error + if (supportsRange && ctrl.$viewValue !== element.val()) { + ctrl.$setViewValue(element.val()); + } else { + // TODO(matsko): implement validateLater to reduce number of validations + ctrl.$validate(); + } + } } function urlInputType(scope, element, attr, ctrl, $sniffer, $browser) { diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index db804043a1e5..ff9506245e2a 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -3290,6 +3290,151 @@ describe('input', function() { } + + describe('step', function() { + + if (supportsRange) { + // Browsers that implement range will never allow you to set a value that doesn't match the step value + // However, currently only Firefox fully inplements the spec when setting the value after the step value changes. + // Other browsers fail in various edge cases, which is why they are not tested here. + it('should round the input value to the nearest step on user input', function() { + var inputElm = helper.compileInput(''); + + helper.changeInputValueTo('5'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(5); + expect(scope.form.alias.$error.step).toBeFalsy(); + + helper.changeInputValueTo('10'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + expect(scope.form.alias.$error.step).toBeFalsy(); + + helper.changeInputValueTo('9'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + expect(scope.form.alias.$error.step).toBeFalsy(); + + helper.changeInputValueTo('7'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(5); + expect(scope.form.alias.$error.step).toBeFalsy(); + + helper.changeInputValueTo('7.5'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + expect(scope.form.alias.$error.step).toBeFalsy(); + }); + + it('should round the input value to the nearest step when setting the model', function() { + var inputElm = helper.compileInput(''); + + scope.$apply('value = 10'); + expect(inputElm.val()).toBe('10'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + expect(scope.form.alias.$error.step).toBeFalsy(); + + scope.$apply('value = 5'); + expect(inputElm.val()).toBe('5'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(5); + expect(scope.form.alias.$error.step).toBeFalsy(); + + scope.$apply('value = 7.5'); + expect(inputElm.val()).toBe('10'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + expect(scope.form.alias.$error.step).toBeFalsy(); + + scope.$apply('value = 7'); + expect(inputElm.val()).toBe('5'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(5); + expect(scope.form.alias.$error.step).toBeFalsy(); + + scope.$apply('value = 9'); + expect(inputElm.val()).toBe('10'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + expect(scope.form.alias.$error.step).toBeFalsy(); + }); + + } else { + it('should validate if "range" is not implemented', function() { + scope.step = 10; + scope.value = 20; + var inputElm = helper.compileInput(''); + + expect(inputElm.val()).toBe('20'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(20); + expect(scope.form.alias.$error.step).toBeFalsy(); + + helper.changeInputValueTo('18'); + expect(inputElm).toBeInvalid(); + expect(inputElm.val()).toBe('18'); + expect(scope.value).toBeUndefined(); + expect(scope.form.alias.$error.step).toBeTruthy(); + + helper.changeInputValueTo('10'); + expect(inputElm).toBeValid(); + expect(inputElm.val()).toBe('10'); + expect(scope.value).toBe(10); + expect(scope.form.alias.$error.step).toBeFalsy(); + + scope.$apply('value = 12'); + expect(inputElm).toBeInvalid(); + expect(inputElm.val()).toBe('12'); + expect(scope.value).toBe(12); + expect(scope.form.alias.$error.step).toBeTruthy(); + }); + + it('should validate even if the step value changes on-the-fly', function() { + scope.step = 10; + var inputElm = helper.compileInput(''); + + helper.changeInputValueTo('10'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + + // Step changes, but value matches + scope.$apply('step = 5'); + expect(inputElm.val()).toBe('10'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + expect(scope.form.alias.$error.step).toBeFalsy(); + + // Step changes, value does not match + scope.$apply('step = 6'); + expect(inputElm).toBeInvalid(); + expect(scope.value).toBeUndefined(); + expect(inputElm.val()).toBe('10'); + expect(scope.form.alias.$error.step).toBeTruthy(); + + // null = valid + scope.$apply('step = null'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + expect(inputElm.val()).toBe('10'); + expect(scope.form.alias.$error.step).toBeFalsy(); + + // Step val as string + scope.$apply('step = "7"'); + expect(inputElm).toBeInvalid(); + expect(scope.value).toBeUndefined(); + expect(inputElm.val()).toBe('10'); + expect(scope.form.alias.$error.step).toBeTruthy(); + + // unparsable string is ignored + scope.$apply('step = "abc"'); + expect(inputElm).toBeValid(); + expect(scope.value).toBe(10); + expect(inputElm.val()).toBe('10'); + expect(scope.form.alias.$error.step).toBeFalsy(); + }); + } + }); }); describe('email', function() { From 132698ae888f4182661ccd4d43a0c97582f98784 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Mon, 15 Aug 2016 23:01:53 +0200 Subject: [PATCH 2/2] feat(input[number]): support step input[number] will now set the step error if the input value (ngModel $viewValue) does not fit the step constraint set in the step / ngStep attribute. Fixes #10597 --- src/jqLite.js | 3 +- src/ng/directive/input.js | 23 +++++ test/ng/directive/inputSpec.js | 151 +++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+), 1 deletion(-) diff --git a/src/jqLite.js b/src/jqLite.js index 802fdb574a13..e4ad42f86932 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -576,7 +576,8 @@ var ALIASED_ATTR = { 'ngMaxlength': 'maxlength', 'ngMin': 'min', 'ngMax': 'max', - 'ngPattern': 'pattern' + 'ngPattern': 'pattern', + 'ngStep': 'step' }; function getBooleanAttrName(element, name) { diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index dcc9567b1303..13990b4c0b68 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -679,7 +679,17 @@ var inputType = { * @param {string} ngModel Assignable angular expression to data-bind to. * @param {string=} name Property name of the form under which the control is published. * @param {string=} min Sets the `min` validation error key if the value entered is less than `min`. + * Can be interpolated. * @param {string=} max Sets the `max` validation error key if the value entered is greater than `max`. + * Can be interpolated. + * @param {string=} ngMin Like `min`, sets the `min` validation error key if the value entered is less than `ngMin`, + * but does not trigger HTML5 native validation. Takes an expression. + * @param {string=} ngMax Like `max`, sets the `max` validation error key if the value entered is greater than `ngMax`, + * but does not trigger HTML5 native validation. Takes an expression. + * @param {string=} step Sets the `step` validation error key if the value entered does not fit the `step` constraint. + * Can be interpolated. + * @param {string=} ngStep Like `step`, sets the `max` validation error key if the value entered does not fit the `ngStep` constraint, + * but does not trigger HTML5 native validation. Takes an expression. * @param {string=} required Sets `required` validation error key if the value is not entered. * @param {string=} ngRequired Adds `required` attribute and `required` validation constraint to * the element when the ngRequired expression evaluates to true. Use `ngRequired` instead of @@ -1549,6 +1559,19 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) { ctrl.$validate(); }); } + + if (isDefined(attr.step) || attr.ngStep) { + var stepVal; + ctrl.$validators.step = function(modelValue, viewValue) { + return ctrl.$isEmpty(viewValue) || isUndefined(stepVal) || viewValue % stepVal === 0; + }; + + attr.$observe('step', function(val) { + stepVal = parseNumberAttrVal(val); + // TODO(matsko): implement validateLater to reduce number of validations + ctrl.$validate(); + }); + } } function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) { diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index ff9506245e2a..d61860df0a8d 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -2621,6 +2621,157 @@ describe('input', function() { }); }); + describe('step', function() { + it('should validate', function() { + $rootScope.step = 10; + $rootScope.value = 20; + var inputElm = helper.compileInput(''); + + expect(inputElm.val()).toBe('20'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(20); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + + helper.changeInputValueTo('18'); + expect(inputElm).toBeInvalid(); + expect(inputElm.val()).toBe('18'); + expect($rootScope.value).toBeUndefined(); + expect($rootScope.form.alias.$error.step).toBeTruthy(); + + helper.changeInputValueTo('10'); + expect(inputElm).toBeValid(); + expect(inputElm.val()).toBe('10'); + expect($rootScope.value).toBe(10); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + + $rootScope.$apply('value = 12'); + expect(inputElm).toBeInvalid(); + expect(inputElm.val()).toBe('12'); + expect($rootScope.value).toBe(12); + expect($rootScope.form.alias.$error.step).toBeTruthy(); + }); + + it('should validate even if the step value changes on-the-fly', function() { + $rootScope.step = 10; + var inputElm = helper.compileInput(''); + + helper.changeInputValueTo('10'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(10); + + // Step changes, but value matches + $rootScope.$apply('step = 5'); + expect(inputElm.val()).toBe('10'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(10); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + + // Step changes, value does not match + $rootScope.$apply('step = 6'); + expect(inputElm).toBeInvalid(); + expect($rootScope.value).toBeUndefined(); + expect(inputElm.val()).toBe('10'); + expect($rootScope.form.alias.$error.step).toBeTruthy(); + + // null = valid + $rootScope.$apply('step = null'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(10); + expect(inputElm.val()).toBe('10'); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + + // Step val as string + $rootScope.$apply('step = "7"'); + expect(inputElm).toBeInvalid(); + expect($rootScope.value).toBeUndefined(); + expect(inputElm.val()).toBe('10'); + expect($rootScope.form.alias.$error.step).toBeTruthy(); + + // unparsable string is ignored + $rootScope.$apply('step = "abc"'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(10); + expect(inputElm.val()).toBe('10'); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + }); + }); + + + describe('ngStep', function() { + it('should validate', function() { + $rootScope.step = 10; + $rootScope.value = 20; + var inputElm = helper.compileInput(''); + + expect(inputElm.val()).toBe('20'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(20); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + + helper.changeInputValueTo('18'); + expect(inputElm).toBeInvalid(); + expect(inputElm.val()).toBe('18'); + expect($rootScope.value).toBeUndefined(); + expect($rootScope.form.alias.$error.step).toBeTruthy(); + + helper.changeInputValueTo('10'); + expect(inputElm).toBeValid(); + expect(inputElm.val()).toBe('10'); + expect($rootScope.value).toBe(10); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + + $rootScope.$apply('value = 12'); + expect(inputElm).toBeInvalid(); + expect(inputElm.val()).toBe('12'); + expect($rootScope.value).toBe(12); + expect($rootScope.form.alias.$error.step).toBeTruthy(); + }); + + it('should validate even if the step value changes on-the-fly', function() { + $rootScope.step = 10; + var inputElm = helper.compileInput(''); + + helper.changeInputValueTo('10'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(10); + + // Step changes, but value matches + $rootScope.$apply('step = 5'); + expect(inputElm.val()).toBe('10'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(10); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + + // Step changes, value does not match + $rootScope.$apply('step = 6'); + expect(inputElm).toBeInvalid(); + expect($rootScope.value).toBeUndefined(); + expect(inputElm.val()).toBe('10'); + expect($rootScope.form.alias.$error.step).toBeTruthy(); + + // null = valid + $rootScope.$apply('step = null'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(10); + expect(inputElm.val()).toBe('10'); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + + // Step val as string + $rootScope.$apply('step = "7"'); + expect(inputElm).toBeInvalid(); + expect($rootScope.value).toBeUndefined(); + expect(inputElm.val()).toBe('10'); + expect($rootScope.form.alias.$error.step).toBeTruthy(); + + // unparsable string is ignored + $rootScope.$apply('step = "abc"'); + expect(inputElm).toBeValid(); + expect($rootScope.value).toBe(10); + expect(inputElm.val()).toBe('10'); + expect($rootScope.form.alias.$error.step).toBeFalsy(); + }); + }); + describe('required', function() {