fix: number-field improved value handling and event management (#4881)

* Change files * Improved value updating * Updating to HTMLInputElement for proxy.
microsoft · Jul 20, 2021 · 6e53b28 · 6e53b28
1 parent 2382e1c
commit 6e53b28
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 33 deletions.
diff --git a/change/@microsoft-fast-components-8b0fff16-adfe-49a9-94ec-5226d4384f61.json b/change/@microsoft-fast-components-8b0fff16-adfe-49a9-94ec-5226d4384f61.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Hide step-up and step-down for number-field when readonly",
+  "packageName": "@microsoft/fast-components",
+  "email": "robarb@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@microsoft-fast-foundation-edf67bb2-0ceb-4be1-9b84-142421961233.json b/change/@microsoft-fast-foundation-edf67bb2-0ceb-4be1-9b84-142421961233.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Hide step-up and step-down for number-field when readonly",
+  "packageName": "@microsoft/fast-foundation",
+  "email": "robarb@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/web-components/fast-components/src/number-field/fixtures/number-field.html b/packages/web-components/fast-components/src/number-field/fixtures/number-field.html
@@ -33,8 +33,8 @@ <h2>Disabled</h2>
 
 <!-- Read only -->
 <h2>Read only</h2>
-<fast-number-field readonly value="Readonly"></fast-number-field>
-<fast-number-field readonly value="Readonly">label</fast-number-field>
+<fast-number-field readonly value="10"></fast-number-field>
+<fast-number-field readonly value="20">label</fast-number-field>
 
 <!-- Autofocus -->
 <h2>Autofocus</h2>

diff --git a/packages/web-components/fast-foundation/src/number-field/number-field.spec.ts b/packages/web-components/fast-foundation/src/number-field/number-field.spec.ts
@@ -654,6 +654,24 @@ describe("NumberField", () => {
             await disconnect();
         });
 
+        it("should set value to max if the max changes to a value less than the value", async () => {
+            const { element, connect, disconnect } = await setup();
+            const max = 10;
+            const value = 10 + max;
+
+            element.setAttribute("value", `${value}`);
+
+            await connect();
+            expect(element.value).to.equal(value.toString());
+
+            element.setAttribute("max", max.toString());
+            await DOM.nextUpdate();
+
+            expect(element.value).to.equal(max.toString());
+
+            await disconnect();
+        });
+
         it("should set value to min when value is less than min", async () => {
             const { element, connect, disconnect } = await setup();
             const min = 10;
@@ -667,6 +685,24 @@ describe("NumberField", () => {
             await disconnect();
         });
 
+        it("should set value to min when value is less than min", async () => {
+            const { element, connect, disconnect } = await setup();
+            const min = 10;
+            const value = min - 10;
+
+            element.setAttribute("value", `${value}`);
+
+            await connect();
+            expect(element.value).to.equal(value.toString());
+
+            element.setAttribute("min", min.toString());
+            await DOM.nextUpdate();
+
+            expect(element.value).to.equal(min.toString());
+
+            await disconnect();
+        });
+
         it("should set max to highest when min is greater than max", async () => {
             const { element, connect, disconnect } = await setup();
             const min = 10;
@@ -762,7 +798,37 @@ describe("NumberField", () => {
             await disconnect();
         });
 
+        it("should update the proxy value when incrementing the value", async () => {
+            const { element, connect, disconnect } = await setup();
+            const step = 2;
+            const value = 5;
+
+            element.step = step;
+            element.value = `${value}`;
+            element.stepUp();
 
+            await connect();
+            expect(element.value).to.equal(`${value + step}`);
+            expect(element.proxy.value).to.equal(`${value + step}`);
+
+            await disconnect();
+        });
+
+        it("should update the proxy value when decrementing the value", async () => {
+            const { element, connect, disconnect } = await setup();
+            const step = 2;
+            const value = 5;
+
+            element.step = step;
+            element.value = `${value}`;
+            element.stepDown();
+
+            await connect();
+            expect(element.value).to.equal(`${value - step}`);
+            expect(element.proxy.value).to.equal(`${value - step}`);
+
+            await disconnect();
+        });
     });
 
     describe("hide step", () => {
@@ -783,4 +849,23 @@ describe("NumberField", () => {
             await disconnect();
         });
     });
+
+    describe("readonly", () => {
+        it("should not render step controls when `readonly` attribute is present", async () => {
+            const { element, connect, disconnect } = await setup();
+
+            await connect();
+
+            expect(element.shadowRoot?.querySelector(".controls")).not.to.equal(null);
+
+            element.setAttribute("readonly", "");
+
+            await DOM.nextUpdate();
+
+            expect(
+                element.shadowRoot?.querySelector(".controls")).to.equal(null);
+
+            await disconnect();
+        });
+    });
 });
diff --git a/packages/web-components/fast-foundation/src/number-field/number-field.template.ts b/packages/web-components/fast-foundation/src/number-field/number-field.template.ts
@@ -71,7 +71,7 @@ export const numberFieldTemplate: (
                 ${ref("control")}
             />
             ${when(
-                x => !x.hideStep,
+                x => !x.hideStep && !x.readOnly && !x.disabled,
                 html`
                     <div class="controls" part="controls">
                         <div class="step-up" part="step-up" @click="${x => x.stepUp()}">

diff --git a/packages/web-components/fast-foundation/src/number-field/number-field.ts b/packages/web-components/fast-foundation/src/number-field/number-field.ts
@@ -127,6 +127,7 @@ export class NumberField extends FormAssociatedNumberField {
                 this.max = numb;
             }
         }
+        this.updateValue(this.value);
     }
 
     /**
@@ -147,6 +148,7 @@ export class NumberField extends FormAssociatedNumberField {
                 this.min = numb;
             }
         }
+        this.updateValue(this.value);
     }
 
     /**
@@ -169,38 +171,38 @@ export class NumberField extends FormAssociatedNumberField {
     public valueChanged(previousValue, nextValue): void {
         super.valueChanged(previousValue, nextValue);
 
-        const numb = parseFloat(nextValue);
-        let out: number | string = numb == nextValue ? nextValue : numb;
-
-        if (nextValue === "" || isNaN(numb)) {
-            out = "";
-        } else {
-            out = this.getValidValue(numb);
-        }
-
-        this.value = out;
-
-        if (this.proxy instanceof HTMLElement) {
-            this.proxy.value = this.value;
-        }
+        this.updateValue(nextValue);
     }
 
     /**
-     * Ensures that the value is between the min and max values
-     *
-     * @param value - number to evaluate
-     * @returns - a string repesentation
+     * Updates the value. Validates that it's a number, between the min
+     *  and max, updates the proxy and emits events.
      *
+     * @param value - value to be validated
      * @internal
      */
-    private getValidValue(value: number): string {
-        if (this.min !== undefined && value < this.min) {
-            value = this.min;
-        } else if (this.max !== undefined && value > this.max) {
-            value = this.max;
+    private updateValue(value): void {
+        if (value === "" || isNaN(parseFloat(value))) {
+            value = "";
+        } else {
+            value = parseFloat(value);
+            if (this.min !== undefined && value < this.min) {
+                value = this.min;
+            } else if (this.max !== undefined && value > this.max) {
+                value = this.max;
+            }
+
+            value = parseFloat(value.toPrecision(12)).toString();
         }
 
-        return parseFloat(value.toPrecision(12)).toString();
+        if (value != this.value) {
+            this.value = value;
+            if (this.proxy instanceof HTMLInputElement) {
+                this.proxy.value = this.value;
+            }
+            this.$emit("input");
+            this.$emit("change");
+        }
     }
 
     /**
@@ -210,9 +212,7 @@ export class NumberField extends FormAssociatedNumberField {
      */
     public stepUp(): void {
         const stepUpValue = this.step + (parseFloat(this.value) || 0);
-        this.value = this.getValidValue(stepUpValue);
-
-        this.$emit("input");
+        this.updateValue(stepUpValue);
     }
 
     /**
@@ -222,9 +222,7 @@ export class NumberField extends FormAssociatedNumberField {
      */
     public stepDown(): void {
         const stepDownValue = (parseFloat(this.value) || 0) - this.step;
-        this.value = this.getValidValue(stepDownValue);
-
-        this.$emit("input");
+        this.updateValue(stepDownValue);
     }
 
     /**