Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: improve input value handling #10935

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/base/src/thirdparty/preact/preact.module.js

Large diffs are not rendered by default.

16 changes: 16 additions & 0 deletions packages/main/src/ColorPicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ class ColorPicker extends UI5Element implements IFormInputElement {
@property({ type: Number })
_alpha = 1;

/**
* this is the alpha value in the input only while editing, since it can container invalid/empty values temporarily
* @private
*/
@property()
_alphaTemp?: string;

/**
* @private
*/
Expand Down Expand Up @@ -301,6 +308,7 @@ class ColorPicker extends UI5Element implements IFormInputElement {

_handleAlphaInput(e: UI5CustomEvent<Input, "input"> | UI5CustomEvent<Slider, "input">) {
const aphaInputValue = String(e.currentTarget.value);
this._alphaTemp = aphaInputValue;
this._alpha = parseFloat(aphaInputValue);
if (Number.isNaN(this._alpha)) {
this._alpha = 1;
Expand Down Expand Up @@ -433,6 +441,14 @@ class ColorPicker extends UI5Element implements IFormInputElement {
}

_handleAlphaChange() {
// parse the input value if valid or fallback to default
this._alpha = this._alphaTemp ? parseFloat(this._alphaTemp) : 1;
if (Number.isNaN(this._alpha)) {
this._alpha = 1;
}
// reset input value so _alpha is rendered
this._alphaTemp = undefined;
// normalize range
this._alpha = this._alpha < 0 ? 0 : this._alpha;
this._alpha = this._alpha > 1 ? 1 : this._alpha;

Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/ColorPickerTemplate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export default function ColorPickerTemplate(this: ColorPicker) {
id="alpha"
disabled={this.inputsDisabled}
class="ui5-color-channel-input"
value={String(this._alpha)}
value={this._alphaTemp ?? String(this._alpha)}
accessibleName={this.alphaInputLabel}
onChange={this._handleAlphaChange}
onInput={this._handleAlphaInput}
Expand Down
62 changes: 3 additions & 59 deletions packages/main/src/Input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,16 +373,6 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement
@property()
value = "";

/**
* Defines the inner stored value of the component.
*
* **Note:** The property is updated upon typing. In some special cases the old value is kept (e.g. deleting the value after the dot in a float)
* @default ""
* @private
*/
@property({ noAttribute: true })
_innerValue = "";

/**
* Defines the value state of the component.
* @default "None"
Expand Down Expand Up @@ -568,7 +558,6 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement
lastConfirmedValue: string
isTyping: boolean
_handleResizeBound: ResizeObserverCallback;
_keepInnerValue: boolean;
_shouldAutocomplete?: boolean;
_enterKeyDown?: boolean;
_isKeyNavigation?: boolean;
Expand Down Expand Up @@ -643,7 +632,6 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement

this._handleResizeBound = this._handleResize.bind(this);

this._keepInnerValue = false;
this._focusedAfterClear = false;
}

Expand All @@ -666,10 +654,6 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement
}

onBeforeRendering() {
if (!this._keepInnerValue) {
this._innerValue = this.value === null ? "" : this.value;
}

if (this.showSuggestions) {
this.enableSuggestions();

Expand Down Expand Up @@ -737,9 +721,9 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement
if (this._performTextSelection) {
// this is required to syncronize lit-html input's value and user's input
// lit-html does not sync its stored value for the value property when the user is typing
if (innerInput.value !== this._innerValue) {
innerInput.value = this._innerValue;
}
// if (innerInput.value !== this._innerValue) {
// innerInput.value = this._innerValue;
// }

if (this.typedInValue.length && this.value.length) {
innerInput.setSelectionRange(this.typedInValue.length, this.value.length);
Expand Down Expand Up @@ -967,7 +951,6 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement
return;
}

this._keepInnerValue = false;
this.focused = false; // invalidating property
this._isChangeTriggeredBySuggestion = false;
if (this.showClearIcon && !this._effectiveShowClearIcon) {
Expand Down Expand Up @@ -1080,9 +1063,6 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement

_input(e: CustomEvent<InputEventDetail> | InputEvent, eventType: string) {
const inputDomRef = this.getInputDOMRefSync();
const emptyValueFiredOnNumberInput = this.value && this.isTypeNumber && !inputDomRef!.value;

this._keepInnerValue = false;

const allowedEventTypes = [
"deleteWordBackward",
Expand All @@ -1102,41 +1082,6 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement

this._shouldAutocomplete = !allowedEventTypes.includes(eventType) && !this.noTypeahead;

if (e instanceof InputEvent) {
// ---- Special cases of numeric Input ----
// ---------------- Start -----------------

// When the last character after the delimiter is removed.
// In such cases, we want to skip the re-rendering of the
// component as this leads to cursor repositioning and causes user experience issues.

// There are few scenarios:
// Example: type "123.4" and press BACKSPACE - the native input is firing event with the whole part as value (123).
// Pressing BACKSPACE again will remove the delimiter and the native input will fire event with the whole part as value again (123).
// Example: type "123.456", select/mark "456" and press BACKSPACE - the native input is firing event with the whole part as value (123).
// Example: type "123.456", select/mark "123.456" and press BACKSPACE - the native input is firing event with empty value.
const delimiterCase = this.isTypeNumber
&& (e.inputType === "deleteContentForward" || e.inputType === "deleteContentBackward")
&& !(e.target as HTMLInputElement).value.includes(".")
&& this.value.includes(".");

// Handle special numeric notation with "e", example "12.5e12"
const eNotationCase = emptyValueFiredOnNumberInput && e.data === "e";

// Handle special numeric notation with "-", example "-3"
// When pressing BACKSPACE, the native input fires event with empty value
const minusRemovalCase = emptyValueFiredOnNumberInput
&& this.value.startsWith("-")
&& this.value.length === 2
&& (e.inputType === "deleteContentForward" || e.inputType === "deleteContentBackward");

if (delimiterCase || eNotationCase || minusRemovalCase) {
this.value = (e.target as HTMLInputElement).value;
this._keepInnerValue = true;
}
// ----------------- End ------------------
}

if (e.target === inputDomRef) {
this.focused = true;

Expand Down Expand Up @@ -1183,7 +1128,6 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement
_handleTypeAhead(item: IInputSuggestionItemSelectable) {
const value = item.text ? item.text : "";

this._innerValue = value;
this.value = value;
this._performTextSelection = true;

Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/InputTemplate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default function InputTemplate(this: Input, hooks?: { preContent: Templat
inner-input-with-icon={this.icon.length}
disabled={this.disabled}
readonly={this._readonly}
value={this._innerValue}
value={this.value}
placeholder={this._placeholder}
maxlength={this.maxlength}
role={this.accInfo.role}
Expand Down
19 changes: 19 additions & 0 deletions packages/main/src/RangeSlider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {

// Styles
import rangeSliderStyles from "./generated/themes/RangeSlider.css.js";
import type { UI5CustomEvent } from "@ui5/webcomponents-base/dist/index.js";

type AriaHandlesText = {
startHandleText?: string,
Expand Down Expand Up @@ -104,6 +105,9 @@ class RangeSlider extends SliderBase implements IFormInputElement {
@property({ type: Number })
startValue = 0;

@property()
startValueTemp?: string;

/**
* Defines end point of a selection - position of a second handle on the slider.
* @default 100
Expand All @@ -114,6 +118,9 @@ class RangeSlider extends SliderBase implements IFormInputElement {
@property({ type: Number })
endValue = 100;

@property()
endValueTemp?: string;

@property({ type: Boolean })
rangePressed = false;

Expand Down Expand Up @@ -312,7 +319,19 @@ class RangeSlider extends SliderBase implements IFormInputElement {
}
}

_onStartInputInput(e: UI5CustomEvent<Input, "input">) {
super._onInputInput();
this.startValueTemp = e.currentTarget.value;
}

_onEndInputInput(e: UI5CustomEvent<Input, "input">) {
super._onInputInput();
this.endValueTemp = e.currentTarget.value;
}

_onInputFocusOut(e: FocusEvent) {
this.startValueTemp = undefined;
this.endValueTemp = undefined;
const tooltipInput = e.target as Input;
const oppositeTooltipInput: Input = tooltipInput.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("[ui5-input][data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("[ui5-input][data-sap-ui-start-value]")!;
const relatedTarget = e.relatedTarget as HTMLElement;
Expand Down
10 changes: 6 additions & 4 deletions packages/main/src/RangeSliderTemplate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,13 @@ export function handles(this: RangeSlider) {
{this.editableTooltip ?
<Input
type="Number"
value={this.startValue.toString()}
value={this.startValueTemp ?? this.startValue.toString()}
accessibleNameRef="ui5-slider-InputLabel"
onFocusIn={() => { this.startValueTemp = this.startValue.toString(); }}
onFocusOut={this._onInputFocusOut}
onKeyDown={this._onInputKeydown}
onChange={this._onInputChange}
onInput={this._onInputInput}
onInput={this._onStartInputInput}
data-sap-ui-start-value
tabIndex={-1}
></Input>
Expand Down Expand Up @@ -115,12 +116,13 @@ export function handles(this: RangeSlider) {
{this.editableTooltip ?
<Input
type="Number"
value={this.endValue.toString()}
value={this.endValueTemp ?? this.endValue.toString()}
accessibleNameRef="ui5-slider-InputLabel"
onFocusIn={() => { this.endValueTemp = this.endValue.toString(); }}
onFocusOut={this._onInputFocusOut}
onKeyDown={this._onInputKeydown}
onChange={this._onInputChange}
onInput={this._onInputInput}
onInput={this._onEndInputInput}
data-sap-ui-end-value
tabIndex={-1}
></Input>
Expand Down
42 changes: 42 additions & 0 deletions packages/main/test/pages/Input_old_value.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<!DOCTYPE html>
<html>

<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
<title>ui5-input</title>


<script src="%VITE_BUNDLE_PATH%" type="module"></script>


<script>// delete Document.prototype.adoptedStyleSheets;</script>

</head>

<body class="input1auto">
<input id="inp1"/>
<ui5-input id="inp2" type="Number"></ui5-input>
<script type="module">
let value = "";

let internalValue = "";

inp1.addEventListener("input", (e) => {
if (e.target.value.length <= 3) {
value = e.target.value;
}
inp1.value = value;
inp2.value = value;
});
inp2.addEventListener("input", (e) => {
if (e.target.value.length <= 3) {
value = e.target.value;
}
inp1.value = value;
inp2.value = value;
});
</script>
</body>

</html>
16 changes: 10 additions & 6 deletions packages/main/test/specs/RangeSlider.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,17 +273,17 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => {

await browser.keys("ArrowUp");
assert.strictEqual(await rangeSlider.getProperty("startValue"), 1, "The start value is not changed on arrow up");

await browser.keys("ArrowDown");
assert.strictEqual(await rangeSlider.getProperty("startValue"), 1, "The start value is not changed on arrow down");

await rangeSlider.setProperty("endValue", 10);
await rangeSliderEndHandle.click();
await rangeSliderEndTooltipInput.click();

await browser.keys("ArrowUp");
assert.strictEqual(await rangeSlider.getProperty("endValue"), 10, "The end value is not changed on arrow up");

await browser.keys("ArrowDown");
assert.strictEqual(await rangeSlider.getProperty("endValue"), 10, "The end value is not changed on arrow down");
});
Expand Down Expand Up @@ -377,7 +377,7 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => {
assert.strictEqual(await rangeSlider.getProperty("startValue"), 1, "The end value is now start value");
assert.strictEqual(await rangeSliderStartTooltipInput.getProperty("value"), "1", "The end input value is now start value");
});

it("Invalid tooltip value should not be changed on 'Enter'", async () => {
const rangeSlider = await browser.$("#range-slider-tickmarks-labels");
const rangeSliderTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--end ui5-input");
Expand All @@ -387,8 +387,12 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => {

await rangeSliderHandle.click();
await rangeSliderTooltipInput.click();
await rangeSliderTooltipInput.setProperty("value", "60");

// await rangeSliderTooltipInput.setProperty("value", "60");
await browser.keys("ArrowRight");
await browser.keys("ArrowRight");
await browser.keys("Backspace");
await browser.keys("Backspace");
await browser.keys("60");
await browser.keys("Enter");

assert.strictEqual(await rangeSlider.getProperty("endValue"), 12, "The slider's value is not changed when invalid");
Expand Down
Loading