fix(ui5-step-input): remove value rounding, apply value-state (#8293)

we decided to remove the rounding of the value if `valuePrecision` property is provided. To inform users about the correctness of their input based on the `valuePrecision` property, we now set a `valueState` on the component. This shows whether the input value meets the required precision. *Note: The `valueState` changing could be easily prevented by preventing the `value-state-change` event.
SAP · May 9, 2024 · 0c0aa1d · 0c0aa1d
1 parent 8a5ec75
commit 0c0aa1d
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 17 deletions.
diff --git a/packages/main/src/StepInput.hbs b/packages/main/src/StepInput.hbs
@@ -35,7 +35,7 @@
 			class="ui5-step-input-input"
 			placeholder="{{placeholder}}"
 			type="{{type}}"
-			value="{{_valuePrecisioned}}"
+			value="{{_displayValue}}"
 			?disabled="{{disabled}}"
 			?required="{{required}}"
 			?readonly="{{readonly}}"

diff --git a/packages/main/src/StepInput.ts b/packages/main/src/StepInput.ts
@@ -343,8 +343,16 @@ class StepInput extends UI5Element implements IFormElement {
 		return this.focused;
 	}
 
-	get _valuePrecisioned() {
-		return this.value.toFixed(this.valuePrecision);
+	get _displayValue() {
+		if ((this.value === 0) || (Number.isInteger(this.value))) {
+			return this.value.toFixed(this.valuePrecision);
+		}
+
+		if (this.value === Number(this.input.value)) { // For the cases where the number is fractional and is ending with 0s.
+			return this.input.value;
+		}
+
+		return this.value.toString();
 	}
 
 	get accInfo() {
@@ -380,6 +388,10 @@ class StepInput extends UI5Element implements IFormElement {
 		return this.shadowRoot!.querySelector<Input>("[ui5-input]")!;
 	}
 
+	get innerInput(): HTMLInputElement {
+		return this.input.shadowRoot!.querySelector<HTMLInputElement>("input")!;
+	}
+
 	get inputOuter() {
 		return this.shadowRoot!.querySelector(".ui5-step-input-input")!;
 	}
@@ -418,12 +430,18 @@ class StepInput extends UI5Element implements IFormElement {
 	}
 
 	_updateValueState() {
-		const valid = !((this.min !== undefined && this.value < this.min) || (this.max !== undefined && this.value > this.max));
+		const isWithinRange = (this.min === undefined || Number(this.input.value) >= this.min)
+							  && (this.max === undefined || Number(this.input.value) <= this.max);
+		const isValueWithCorrectPrecision = this._isValueWithCorrectPrecision;
 		const previousValueState = this.valueState;
+		const isValid = isWithinRange && isValueWithCorrectPrecision;
 
-		this.valueState = valid ? ValueState.None : ValueState.Negative;
+		this.valueState = isValid ? ValueState.None : ValueState.Negative;
 
-		const eventPrevented = !this.fireEvent<StepInputValueStateChangeEventDetail>("value-state-change", { valueState: this.valueState, valid }, true);
+		const eventPrevented = !this.fireEvent<StepInputValueStateChangeEventDetail>("value-state-change", {
+			valueState: this.valueState,
+			valid: isValid,
+		}, true);
 
 		if (eventPrevented) {
 			this.valueState = previousValueState;
@@ -451,7 +469,6 @@ class StepInput extends UI5Element implements IFormElement {
 	 */
 	_modifyValue(modifier: number, fireChangeEvent = false) {
 		let value;
-		this.value = this._preciseValue(parseFloat(this.input.value));
 		value = this.value + modifier;
 		if (this.min !== undefined && value < this.min) {
 			value = this.min;
@@ -462,6 +479,7 @@ class StepInput extends UI5Element implements IFormElement {
 		value = this._preciseValue(value);
 		if (value !== this.value) {
 			this.value = value;
+			this.input.value = value.toFixed(this.valuePrecision);
 			this._validate();
 			this._setButtonState();
 			this.focused = true;
@@ -488,19 +506,52 @@ class StepInput extends UI5Element implements IFormElement {
 		}
 	}
 
+	get _isValueWithCorrectPrecision() {
+		// gets either "." or "," as delimiter which is based on locale, and splits the number by it
+		const delimiter = this.input.value.includes(".") ? "." : ",";
+		const numberParts = this.input.value.split(delimiter);
+		const decimalPartLength = numberParts.length > 1 ? numberParts[1].length : 0;
+
+		return decimalPartLength === this.valuePrecision;
+	}
+
 	_onInputChange() {
-		if (this.input.value === "") {
-			this.input.value = (this.min || 0) as unknown as string;
+		this._setDefaultInputValueIfNeeded();
+
+		const inputValue = Number(this.input.value);
+		if (this._isValueChanged(inputValue)) {
+			this._updateValueAndValidate(inputValue);
 		}
-		const inputValue = this._preciseValue(parseFloat(this.input.value));
-		if (this.value !== this._previousValue || this.value !== inputValue) {
-			this.value = inputValue;
-			this._validate();
-			this._setButtonState();
-			this._fireChangeEvent();
+	}
+
+	_setDefaultInputValueIfNeeded() {
+		if (this.input.value === "") {
+			const defaultValue = (this.min || 0).toFixed(this.valuePrecision);
+			this.input.value = defaultValue;
+			this.innerInput.value = defaultValue; // we need to update inner input value as well, to avoid empty input scenario
 		}
 	}
 
+	_isValueChanged(inputValue: number) {
+		const isValueWithCorrectPrecision = this._isValueWithCorrectPrecision;
+		// Treat values as distinct when modified to match a specific precision (e.g., from 3.4000 to 3.40),
+		// even if JavaScript sees them as equal, to correctly update valueState based on expected valuePrecision.
+		const isPrecisionCorrectButValueStateError = isValueWithCorrectPrecision && this.valueState === ValueState.Negative;
+
+		return this.value !== this._previousValue
+			|| this.value !== inputValue
+			|| inputValue === 0
+			|| !isValueWithCorrectPrecision
+			|| isPrecisionCorrectButValueStateError;
+	}
+
+	_updateValueAndValidate(inputValue: number) {
+		this.value = inputValue;
+		this._validate();
+		this._setButtonState();
+		this._fireChangeEvent();
+	}
+
 	_onfocusin() {
 		this.focused = true;
 	}

diff --git a/packages/main/test/specs/StepInput.spec.js b/packages/main/test/specs/StepInput.spec.js
@@ -457,7 +457,6 @@ describe("'change' event firing", () => {
 	});
 
 	it("Value state is not changed, when value-state-change is prevented", async () => {
-		await browser.url(`test/pages/StepInput.html`);
 		const input = await browser.$("#stepInputValueStateChange").shadow$("ui5-input").shadow$("input");
 
 		const valueState = await input.getProperty("valueState");
@@ -468,6 +467,19 @@ describe("'change' event firing", () => {
 		assert.strictEqual(await input.getProperty("valueState"), valueState, "value state is not changed");
 	});
 
+	it("Value is not rounding when valuePrecision is set, and Value State is set to 'Negative' when the value is not compliant", async () => {
+		const input = await browser.$("#stepInputPrecision").shadow$("ui5-input");
+		const innerInput = await input.shadow$("input");
+
+		await innerInput.click();
+		await innerInput.setValue("0.100");
+		await browser.keys("Enter");
+
+
+		assert.strictEqual(await input.getProperty("valueState"), "Negative", "value state is set to Error");
+		assert.strictEqual(await input.getProperty("value"), "0.100", "value is not rounded");
+	})
+
 });
 
 describe("Accessibility related parameters", async () => {
@@ -492,4 +504,4 @@ describe("Accessibility related parameters", async () => {
 		assert.strictEqual(await siInner.getAttribute("aria-label"), "test-aria-label", "'aria-label' attribute exists and has correct value 'test-aria-label'");
 	});
 
-});
+});