fix: avoid using initial step in validation (#421)

setting the min or max property should not trigger step-based validation fix #420
vaadin · Oct 28, 2019 · 6d5f374 · 6d5f374
1 parent cc5143d
commit 6d5f374
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 4 deletions.
diff --git a/src/vaadin-number-field.html b/src/vaadin-number-field.html
@@ -120,7 +120,6 @@
              */
             step: {
               type: Number,
-              reflectToAttribute: true,
               value: 1,
               observer: '_stepChanged'
             }
@@ -303,7 +302,12 @@
         }
 
         _stepChanged(step) {
-          this.inputElement.step = step;
+          // Avoid using initial value in validation
+          this.__validateByStep = this.__stepChangedCalled || this.getAttribute('step') !== null;
+          this.inputElement.step = this.__validateByStep ? step : 'any';
+
+          this.__stepChangedCalled = true;
+          this.setAttribute('step', step);
         }
 
         _minChanged(min) {
@@ -342,7 +346,7 @@
 
         checkValidity() {
           // text-field mixin does not check against `min`, `max` and `step`
-          if (this.min !== undefined || this.max !== undefined || this.step !== 1) {
+          if (this.min !== undefined || this.max !== undefined || this.__validateByStep) {
             this.invalid = !this.inputElement.checkValidity();
           }
           return super.checkValidity();

diff --git a/test/integer-field.html b/test/integer-field.html
@@ -215,7 +215,7 @@
 
         it('should allow setting positive integer as string', () => {
           integerField.step = '5';
-          expect(integerField.step).to.eql('5');
+          expect(integerField.step).to.eql(5);
         });
 
         ['foo', '-1', -1, '1.2', 1.2, '+1', '1e1', undefined, null, {}].forEach(invalidStep => {

diff --git a/test/number-field.html b/test/number-field.html
@@ -17,6 +17,16 @@
       <vaadin-number-field></vaadin-number-field>
     </template>
   </test-fixture>
+  <test-fixture id="step-as-attribute">
+    <template>
+      <vaadin-number-field step="1.5"></vaadin-number-field>
+    </template>
+  </test-fixture>
+  <test-fixture id="default-step-as-attribute">
+    <template>
+      <vaadin-number-field step="1"></vaadin-number-field>
+    </template>
+  </test-fixture>
 
   <script>
     describe('number-field', () => {
@@ -760,6 +770,58 @@
           expect(numberField.validate(), 'value should not be greater than max').to.be.false;
         });
 
+        it('should validate by step when defined by user', () => {
+          numberField.step = 1.5;
+
+          [-6, -1.5, 0, 1.5, 4.5].forEach(validValue => {
+            numberField.value = validValue;
+            expect(numberField.validate()).to.be.true;
+          });
+
+          [-3.5, -1, 2, 2.5].forEach(invalidValue => {
+            numberField.value = invalidValue;
+            expect(numberField.validate()).to.be.false;
+          });
+        });
+
+        it('should validate by step when defined as attribute', () => {
+          const numberField = fixture('step-as-attribute');
+          numberField.value = 1;
+          expect(numberField.validate()).to.be.false;
+          numberField.value = 1.5;
+          expect(numberField.validate()).to.be.true;
+        });
+
+        it('should validate by step when default value defined as attribute', () => {
+          const numberField = fixture('default-step-as-attribute');
+          numberField.value = 1.5;
+          expect(numberField.validate()).to.be.false;
+          numberField.value = 1;
+          expect(numberField.validate()).to.be.true;
+        });
+
+        it('should use min as step basis in validation when both are defined', () => {
+          numberField.min = 1;
+          numberField.step = 1.5;
+
+          [1, 2.5, 4, 5.5].forEach(validValue => {
+            numberField.value = validValue;
+            expect(numberField.validate()).to.be.true;
+          });
+
+          [1.5, 3, 5].forEach(invalidValue => {
+            numberField.value = invalidValue;
+            expect(numberField.validate()).to.be.false;
+          });
+        });
+
+        it('should not validate by step when only min and max are set', () => {
+          numberField.min = 1;
+          numberField.max = 5;
+          numberField.value = 1.5; // would be invalid by default step=1
+          expect(numberField.validate()).to.be.true;
+        });
+
         describe('removing validation constraints', () => {
           it('should update "invalid" state when "min" is removed', () => {
             numberField.value = '42';