From 5c8d2da28aa3dc0fd003d63b731cec3081c03232 Mon Sep 17 00:00:00 2001
From: Nii Yeboah <nii@vaadin.com>
Date: Thu, 7 Mar 2019 16:15:23 +0200
Subject: [PATCH] Fix number-field value and step issues and align behaviour
 with native element - Allow programmatically setting any value regardless of
 step - Change clicking + or - to increment or decrement value to the next
 multiple of step (offset by the min) - Change checkValidity to check when
 value is not multiple of step (offset by the min) - Change value to always
 return a string

---
 src/vaadin-number-field.html | 26 ++++++++++++++++++++------
 test/number-field.html       | 34 +++++++++++++++++++++++++---------
 2 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/src/vaadin-number-field.html b/src/vaadin-number-field.html
index 6edfc310..c6232321 100644
--- a/src/vaadin-number-field.html
+++ b/src/vaadin-number-field.html
@@ -208,7 +208,18 @@
         }
 
         _getValue(incr) {
-          return (incr + (incr * Math.floor((parseFloat(this.value || 0) / incr).toFixed(1)))).toFixed(this.__decimals);
+          let result = this.min || incr;
+          if (this.value) {
+            const number = parseFloat(this.value);
+            let nextMultiple = parseFloat((Math.ceil(number / incr) * incr).toFixed(this.__decimals));
+            if (this.min) {
+              let minOffset = this.min % this.step;
+              minOffset = incr < 0 ? -minOffset : minOffset;
+              nextMultiple = this.min ? nextMultiple + minOffset : nextMultiple;
+            }
+            result = (nextMultiple === number ? nextMultiple + incr : nextMultiple).toFixed(this.__decimals);
+          }
+          return result;
         }
 
         _getAllowedIncrementSign(sign) {
@@ -262,17 +273,19 @@
           // Validate value to be numeric
           if (newVal && isNaN(parseFloat(newVal).toFixed(this.__decimals))) {
             this.value = '';
-          } else if (!isNaN(parseFloat(this.value)) &&
+          } else if (!isNaN(parseFloat(this.value)) && this.__decimals &&
             parseFloat(this.value) !== parseFloat(parseFloat(this.value).toFixed(this.__decimals))) {
             // Validate correct decimals
-            this.value = parseFloat(parseFloat(this.value).toFixed(this.__decimals));
+            this.value = parseFloat(this.value).toFixed(this.__decimals);
+          } else if (typeof this.value !== 'string') {
+            this.value = String(this.value);
           }
 
           super._valueChanged(this.value, oldVal);
         }
 
         __onInputChange() {
-          this.checkValidity() && this.__adjustDecimals();
+          this.checkValidity();
         }
 
         __adjustDecimals() {
@@ -289,8 +302,9 @@
 
         checkValidity() {
           // text-field mixin does not check against `min` and `max`
-          if (this.min !== undefined || this.max !== undefined) {
-            this.invalid = !this.inputElement.checkValidity();
+          if (this.value && (this.min !== undefined || this.max !== undefined || this.step)) {
+            const stepBase = this.min || 0;
+            this.invalid = this.value < this.min || this.value > this.max || (this.value - stepBase).toFixed(this.__decimals) % this.step !== 0;
           }
           return super.checkValidity();
         }
diff --git a/test/number-field.html b/test/number-field.html
index 6c3fc001..b65f6244 100644
--- a/test/number-field.html
+++ b/test/number-field.html
@@ -49,6 +49,22 @@
           });
         });
 
+        it('should set value with correct decimal places regardless of step', () => {
+          numberField.step = 2;
+          numberField.value = 9.99;
+
+          expect(numberField.value).equal('9.99');
+        });
+
+        it('should increment value to next multiple of step offset by the min', () => {
+          numberField.step = 3;
+          numberField.min = 4;
+          numberField.value = 4;
+
+          increaseButton.click();
+
+          expect(numberField.value).equal('7');
+        });
       });
 
       describe('value control buttons', () => {
@@ -145,7 +161,7 @@
 
           increaseButton.click();
 
-          expect(numberField.value).to.be.equal(0);
+          expect(numberField.value).to.be.equal('0');
         });
 
         it('should not decrease value when decreaseButton is clicked and min value is reached', () => {
@@ -154,7 +170,7 @@
 
           decreaseButton.click();
 
-          expect(numberField.value).to.be.equal(0);
+          expect(numberField.value).to.be.equal('0');
         });
 
         it('should not disable buttons if there are no limits set', () => {
@@ -181,10 +197,10 @@
           numberField.value = 0;
 
           increaseButton.click();
-          expect(numberField.value).to.be.equal(0);
+          expect(numberField.value).to.be.equal('0');
 
           decreaseButton.click();
-          expect(numberField.value).to.be.equal(0);
+          expect(numberField.value).to.be.equal('0');
         });
 
         it('should prevent touchend event on value control buttons', () => {
@@ -240,7 +256,7 @@
 
           decreaseButton.click();
 
-          expect(numberField.value).to.be.equal(numberField.max);
+          expect(numberField.value).to.be.equal(String(numberField.max));
         });
 
         it('should increase value to min value when value is under min and increaseButton is clicked', () => {
@@ -249,7 +265,7 @@
 
           increaseButton.click();
 
-          expect(numberField.value).to.be.equal(numberField.min);
+          expect(numberField.value).to.be.equal(String(numberField.min));
         });
       });
 
@@ -268,15 +284,15 @@
           expect(numberField.validate()).to.be.true;
         });
 
-        it('should prevent setting decimals', () => {
+        it('should allow setting decimals', () => {
           numberField.value = 7.6;
-          expect(numberField.value).to.be.equal(8);
+          expect(numberField.value).to.be.equal('7.6');
         });
 
         it('should prevent setting too many decimals', () => {
           numberField.step = 0.1;
           numberField.value = 7.686;
-          expect(numberField.value).to.be.equal(7.7);
+          expect(numberField.value).to.be.equal('7.7');
         });
 
         it('should validate when setting limits', () => {