Skip to content

Commit c29b9ef

Browse files
authored
fix(ui5-input): fire change on the initial tab press after suggestion is selected (#3514)
* Fix change firing on the initial tab press * Minor error fixes * Code refactoring * Minor linting fix * Retrigger tests * Fix firing of change after shift+tab is used for focus inwq * Retrigger tests * Retrigger tests * Avoid double firing of the change event * Add test
1 parent 32337a9 commit c29b9ef

File tree

4 files changed

+78
-4
lines changed

4 files changed

+78
-4
lines changed

packages/main/src/Input.js

+23-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
isEnter,
1313
isBackSpace,
1414
isEscape,
15+
isTabNext,
1516
} from "@ui5/webcomponents-base/dist/Keys.js";
1617
import Integer from "@ui5/webcomponents-base/dist/types/Integer.js";
1718
import { fetchI18nBundle, getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js";
@@ -505,6 +506,9 @@ class Input extends UI5Element {
505506
// Indicates if the user selection has been canceled with [ESC].
506507
this.suggestionSelectionCanceled = false;
507508

509+
// Indicates if the change event has already been fired
510+
this._changeFired = false;
511+
508512
// tracks the value between focus in and focus out to detect that change event should be fired.
509513
this.previousValue = undefined;
510514

@@ -594,6 +598,10 @@ class Input extends UI5Element {
594598
return this._handleSpace(event);
595599
}
596600

601+
if (isTabNext(event)) {
602+
return this._handleTab(event);
603+
}
604+
597605
if (isEnter(event)) {
598606
return this._handleEnter(event);
599607
}
@@ -638,6 +646,12 @@ class Input extends UI5Element {
638646
}
639647
}
640648

649+
_handleTab(event) {
650+
if (this.Suggestions && (this.previousValue !== this.value)) {
651+
this.Suggestions.onTab(event);
652+
}
653+
}
654+
641655
_handleEnter(event) {
642656
const itemPressed = !!(this.Suggestions && this.Suggestions.onEnter(event));
643657
if (!itemPressed) {
@@ -697,7 +711,12 @@ class Input extends UI5Element {
697711
}
698712

699713
_handleChange(event) {
700-
this.fireEvent(this.EVENT_CHANGE);
714+
if (!this._changeFired) {
715+
this.fireEvent(this.EVENT_CHANGE);
716+
}
717+
718+
// Set event as no longer marked
719+
this._changeFired = false;
701720
}
702721

703722
_scroll(event) {
@@ -866,6 +885,9 @@ class Input extends UI5Element {
866885
this.valueBeforeItemSelection = itemText;
867886
this.fireEvent(this.EVENT_INPUT);
868887
this.fireEvent(this.EVENT_CHANGE);
888+
889+
// Mark the change event to avoid double firing
890+
this._changeFired = true;
869891
}
870892

871893
this.valueBeforeItemPreview = "";

packages/main/src/features/InputSuggestions.js

+8
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,14 @@ class Suggestions {
104104
return false;
105105
}
106106

107+
onTab(event) {
108+
if (this._isItemOnTarget()) {
109+
this.onItemSelected(null, true);
110+
return true;
111+
}
112+
return false;
113+
}
114+
107115
toggle(bToggle, { preventFocusRestore }) {
108116
const toggle = bToggle !== undefined ? bToggle : !this.isOpened();
109117

packages/main/test/pages/Input.html

+17-3
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,19 @@ <h3> Test 'input' event</h3>
128128
<h3> 'input' test result</h3>
129129
<ui5-input id="inputLiveChangeResult"></ui5-input>
130130

131+
<h3> Input test change with suggestions</h3>
132+
<ui5-input id="inputChange-Suggestions" show-suggestions>
133+
<ui5-icon slot="icon" name="message-warning"></ui5-icon>
134+
<ui5-suggestion-item text="Project"></ui5-suggestion-item>
135+
<ui5-suggestion-item text="Fellowship"></ui5-suggestion-item>
136+
<ui5-suggestion-item text="Vocational Training"></ui5-suggestion-item>
137+
</ui5-input>
138+
131139
<h3> Input test change</h3>
132-
<ui5-input id="inputChange"> </ui5-input>
133-
<h3> Input test change result</h3>
134-
<ui5-input id="inputChangeResult"></ui5-input>
140+
<ui5-input id="inputChange"></ui5-input>
135141

142+
<h3> Input test change result</h3>
143+
<ui5-input id="inputChangeResult"></ui5-input>
136144
<h3>Input test ESC</h3>
137145
<ui5-input id="myInputEsc" show-suggestions style="width: 100%">
138146
<ui5-suggestion-item text="Chromium"></ui5-suggestion-item>
@@ -360,6 +368,7 @@ <h3>Test Backspace</h3>
360368
var sap_database_entries = [{ key: "A", text: "A" }, { key: "Afg", text: "Afghanistan" }, { key: "Arg", text: "Argentina" }, { key: "Alb", text: "Albania" }, { key: "Arm", text: "Armenia" }, { key: "Alg", text: "Algeria" }, { key: "And", text: "Andorra" }, { key: "Ang", text: "Angola" }, { key: "Ast", text: "Austria" }, { key: "Aus", text: "Australia" }, { key: "Aze", text: "Azerbaijan" }, { key: "Aruba", text: "Aruba" }, { key: "Antigua", text: "Antigua and Barbuda" }, { key: "B", text: "B" }, { key: "Bel", text: "Belarus" }, { key: "Bel", text: "Belgium" }, { key: "Bg", text: "Bulgaria" }, { key: "Bra", text: "Brazil" }, { key: "C", text: "C" }, { key: "Ch", text: "China" }, { key: "Cub", text: "Cuba" }, { key: "Chil", text: "Chili" }, { key: "L", text: "L" }, { key: "Lat", text: "Latvia" }, { key: "Lit", text: "Litva" }, { key: "P", text: "P" }, { key: "Prt", text: "Portugal" }, { key: "S", text: "S" }, { key: "Sen", text: "Senegal" }, { key: "Ser", text: "Serbia" }, { key: "Sey", text: "Seychelles" }, { key: "Sierra", text: "Sierra Leone" }, { key: "Sgp", text: "Singapore" }, { key: "Sint", text: "Sint Maarten" }, { key: "Slv", text: "Slovakia" }, { key: "Slo", text: "Slovenia" }];
361369

362370
var input = document.getElementById('myInput');
371+
var inputChangeWithSuggestions = document.getElementById('inputChange-Suggestions');
363372
var inputGrouping = document.getElementById('myInputGrouping');
364373
var myInput2 = document.getElementById('myInput2');
365374
var inputError = document.getElementById("inputError");
@@ -460,6 +469,11 @@ <h3>Test Backspace</h3>
460469
inputChangeResult.value = ++inputChangeResultCounter;
461470
});
462471

472+
inputChangeWithSuggestions.addEventListener("ui5-change", function (event) {
473+
inputChangeResult.value = ++inputChangeResultCounter;
474+
});
475+
476+
463477
scrollInput.addEventListener("ui5-suggestion-scroll", function (event) {
464478
scrollResult.value = event.detail.scrollTop;
465479
console.log("scroll", { scrolltop: event.detail.scrollTop, container: event.detail.scrollContainer });

packages/main/test/specs/Input.spec.js

+30
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,37 @@ describe("Input general interaction", () => {
9898
assert.strictEqual(inputResult.getValue(), "2", "change is called twice");
9999
});
100100

101+
it("fires change on tab", () => {
102+
browser.url(`http://localhost:${PORT}/test-resources/pages/Input.html`);
103+
104+
const input = $("#inputChange-Suggestions").shadow$("input");
105+
const inputResult = $("#inputChangeResult").shadow$("input");
106+
107+
input.click();
108+
input.keys("ArrowDown");
109+
input.keys("Tab");
110+
111+
assert.strictEqual(inputResult.getValue(), "1", "change is called twice");
112+
});
113+
114+
it("fires change only once when there was already a value on focus in", () => {
115+
const input = $("#inputChange-Suggestions").shadow$("input");
116+
const inputResult = $("#inputChangeResult").shadow$("input");
117+
browser.keys(["Shift", "Tab"]);
118+
input.keys("Backspace");
119+
120+
input.keys("ArrowDown");
121+
input.keys("ArrowDown");
122+
123+
124+
input.keys("Tab");
125+
126+
assert.strictEqual(inputResult.getValue(), "2", "change is called once");
127+
});
128+
101129
it("fires input", () => {
130+
browser.url(`http://localhost:${PORT}/test-resources/pages/Input.html`);
131+
102132
const input2 = $("#input2").shadow$("input");
103133
const inputLiveChangeResult = $("#inputLiveChangeResult").shadow$("input");
104134

0 commit comments

Comments
 (0)