From 55fa533b738396b67f1776c643573607339d3b23 Mon Sep 17 00:00:00 2001 From: ilhan orhan Date: Wed, 13 Mar 2019 17:21:41 +0200 Subject: [PATCH] fix(ui5-input): fire change in sync with the native input (#168) Bind for the native input change to fire the ui5-input semantic one. Make sure change event is fired upon suggestion item selection. Still, for IE, a manually work is needed to detect if change should be fired on Enter as the native input does not react. Fixes: https://github.com/SAP/ui5-webcomponents/issues/154 --- packages/main/src/Input.hbs | 1 + packages/main/src/Input.js | 42 +++++++------------ .../main/pages/DatePicker_test_page.html | 17 +++++--- .../ui/webcomponents/main/pages/Input.html | 36 +++++++++++----- .../webcomponents/main/qunit/Input.qunit.js | 35 ---------------- packages/main/test/specs/DatePicker.spec.js | 29 ++++++++++++- packages/main/test/specs/Input.spec.js | 40 ++++++++++++++---- 7 files changed, 115 insertions(+), 85 deletions(-) diff --git a/packages/main/src/Input.hbs b/packages/main/src/Input.hbs index be885ed439d7..cd8a1d3c6d01 100644 --- a/packages/main/src/Input.hbs +++ b/packages/main/src/Input.hbs @@ -15,6 +15,7 @@ .value="{{ctr.value}}" placeholder="{{ctr.placeholder}}" @input="{{ctr._input.onInput}}" + @change="{{ctr._input.change}}" data-sap-no-tab-ref data-sap-focus-ref /> diff --git a/packages/main/src/Input.js b/packages/main/src/Input.js index 8526cde4551f..0e90df4ca294 100644 --- a/packages/main/src/Input.js +++ b/packages/main/src/Input.js @@ -1,5 +1,6 @@ import WebComponent from "@ui5/webcomponents-base/src/sap/ui/webcomponents/base/WebComponent"; import Bootstrap from "@ui5/webcomponents-base/src/sap/ui/webcomponents/base/Bootstrap"; +import { isIE } from "@ui5/webcomponents-core/dist/sap/ui/Device"; import ValueState from "@ui5/webcomponents-base/src/sap/ui/webcomponents/base/types/ValueState"; import ShadowDOM from "@ui5/webcomponents-base/src/sap/ui/webcomponents/base/compatibility/ShadowDOM"; import { @@ -285,17 +286,14 @@ class Input extends WebComponent { // Indicates if there is selected suggestionItem. this.hasSuggestionItemSelected = false; - // Indicates if there is focused suggestionItem. - // Used to ignore the Input "focusedOut" and thus preventing firing "change" event. - this.hasSuggestionItemFocused = false; - - this.previousValue = undefined; - // Represents the value before user moves selection between the suggestion items. // Used to register and fire "input" event upon [SPACE] or [ENTER]. // Note: the property "value" is updated upon selection move and can`t be used. this.valueBeforeItemSelection = ""; + // tracks the value between focus in and focus out to detect that change event should be fired. + this.previousValue = undefined; + // Indicates, if the component is rendering for first time. this.firstRendering = true; @@ -307,11 +305,13 @@ class Input extends WebComponent { // all user interactions this.ACTION_ENTER = "enter"; - this.ACTION_FOCUSOUT = "focusOut"; this.ACTION_USER_INPUT = "input"; this._input = { onInput: this._onInput.bind(this), + change: event => { + this.fireEvent(this.EVENT_CHANGE); + }, }; this._whenShadowRootReady().then(this.attachFocusHandlers.bind(this)); @@ -327,7 +327,6 @@ class Input extends WebComponent { if (!this.firstRendering && this.Suggestions) { this.Suggestions.toggle(this.shouldOpenSuggestions()); } - this.checkFocusOut(); this.firstRendering = false; } @@ -392,13 +391,13 @@ class Input extends WebComponent { } onfocusin() { - this.previousValue = this.value; - this.hasSuggestionItemFocused = false; this._focused = true; // invalidating property + this.previousValue = this.value; } onfocusout() { this._focused = false; // invalidating property + this.previousValue = ""; } _onInput(event) { @@ -454,6 +453,7 @@ class Input extends WebComponent { this.value = itemText; this.valueBeforeItemSelection = itemText; this.fireEvent(this.EVENT_INPUT); + this.fireEvent(this.EVENT_CHANGE); } } @@ -469,35 +469,27 @@ class Input extends WebComponent { const inputValue = this.getInputValue(); const isSubmit = action === this.ACTION_ENTER; - const isFocusOut = action === this.ACTION_FOCUSOUT; const isUserInput = action === this.ACTION_USER_INPUT; this.value = inputValue; - const valueChanged = (this.previousValue !== undefined) && (this.previousValue !== this.value); - if (isUserInput) { // input this.fireEvent(this.EVENT_INPUT); return; } - if ((isSubmit || isFocusOut) && valueChanged) { // change - this.previousValue = this.value; - this.fireEvent(this.EVENT_CHANGE); - } - if (isSubmit) { // submit this.fireEvent(this.EVENT_SUBMIT); } - } - checkFocusOut() { - if (!this._focused && !this.hasSuggestionItemFocused) { - this.fireEventByAction(this.ACTION_FOCUSOUT); - this.previousValue = ""; + // In IE, pressing the ENTER does not fire change + const valueChanged = (this.previousValue !== undefined) && (this.previousValue !== this.value); + if (isIE() && isSubmit && valueChanged) { + this.fireEvent(this.EVENT_CHANGE); } } + getInputValue() { const inputDOM = this.getDomRef(); if (inputDOM) { @@ -519,9 +511,7 @@ class Input extends WebComponent { } /* Suggestions interface */ - onItemFocused() { - this.hasSuggestionItemFocused = true; - } + onItemFocused() {} onItemSelected(item, keyboardUsed) { this.selectSuggestion(item, keyboardUsed); diff --git a/packages/main/test/sap/ui/webcomponents/main/pages/DatePicker_test_page.html b/packages/main/test/sap/ui/webcomponents/main/pages/DatePicker_test_page.html index bba4e4c87b73..a678cc738a85 100644 --- a/packages/main/test/sap/ui/webcomponents/main/pages/DatePicker_test_page.html +++ b/packages/main/test/sap/ui/webcomponents/main/pages/DatePicker_test_page.html @@ -32,20 +32,22 @@ + - + + \ No newline at end of file diff --git a/packages/main/test/sap/ui/webcomponents/main/pages/Input.html b/packages/main/test/sap/ui/webcomponents/main/pages/Input.html index 0398059611c6..6f071c095fd8 100644 --- a/packages/main/test/sap/ui/webcomponents/main/pages/Input.html +++ b/packages/main/test/sap/ui/webcomponents/main/pages/Input.html @@ -37,24 +37,31 @@

Input disabled

-

Input valueState Warning

+

Input valueState Success

+ + + + +

Test 'change' event

+

'change' event result

+ -

Input valueState Error

+

Test 'input' event

+

'input' test result

+ -

Input valueState Success

- - - + -

Input

- - +

Input test change

+ +

Input test change result

+

Input readonly

@@ -139,7 +146,7 @@

Input type 'URL'

input2.addEventListener("input", function (event) { inputCounter += 1; - inputResult.value = inputCounter; + inputLiveChangeResult.value = inputCounter; }); myInput2.addEventListener("suggestionItemSelect", function (event) { @@ -147,6 +154,15 @@

Input type 'URL'

suggestionSelectedCounter += 1; inputResult.value = suggestionSelectedCounter; }); + + var inputChangeResultCounter = 0; + + inputChange.addEventListener("submit", function (event) { + inputChange.value = "Changed via API"; + }); + inputChange.addEventListener("change", function (event) { + inputChangeResult.value = ++inputChangeResultCounter; + }); \ No newline at end of file diff --git a/packages/main/test/sap/ui/webcomponents/main/qunit/Input.qunit.js b/packages/main/test/sap/ui/webcomponents/main/qunit/Input.qunit.js index 7a4333268ddf..a348fbff1881 100644 --- a/packages/main/test/sap/ui/webcomponents/main/qunit/Input.qunit.js +++ b/packages/main/test/sap/ui/webcomponents/main/qunit/Input.qunit.js @@ -269,40 +269,5 @@ TestHelper.ready(function() { fixture.innerHTML = ""; this.input = null; }); - - QUnit.test("type in input fires change onfocusout", function(assert) { - assert.expect(1); - - var input = this.getInput(); - var done = assert.async(); - - input.addEventListener("change", function(e){ - assert.ok(true, "change fired, when onfocusout"); - done(); - }); - - input.value = "abc"; - input.onfocusout(); - }); - - QUnit.test("type in input does not fire change when the initial value remains the same", function(assert) { - assert.expect(1); - - var input = this.getInput(); - var done = assert.async(); - - input.addEventListener("change", function(e){ - assert.ok(false, "change fired incorreclty, when onfocusout"); - }); - - input.value = "abc"; - input.value = ""; - input.onfocusout(); - - setTimeout(function() { - assert.ok(true, "change event not fired on onfocusout, when the value did not changed"); - done(); - }, 200) - }); }); }); \ No newline at end of file diff --git a/packages/main/test/specs/DatePicker.spec.js b/packages/main/test/specs/DatePicker.spec.js index b45cdd88108d..84341c504247 100644 --- a/packages/main/test/specs/DatePicker.spec.js +++ b/packages/main/test/specs/DatePicker.spec.js @@ -228,19 +228,44 @@ describe("Date Picker Tests", () => { datepicker.innerInput.click(); browser.keys("\b\b\b\b\b\b\b\b\b\b\b"); - datepicker.innerInput.setValue("Jan 8, 2015"); + datepicker.innerInput.keys("Jan 8, 2015"); browser.findElementDeep("#dp1 >>> ui5-input >>> input").click(); //click elsewhere to focusout assert.equal(browser.findElementDeep("#lbl").getHTML(false), "1", 'change has fired once'); datepicker.innerInput.click(); browser.keys("\b\b\b\b\b\b\b\b\b\b\b"); - datepicker.innerInput.setValue("Jan 6, 2015"); + datepicker.innerInput.keys("Jan 6, 2015"); browser.findElementDeep("#dp1 >>> ui5-input >>> input").click(); //click elsewhere to focusout assert.equal(browser.findElementDeep("#lbl").getHTML(false), "2", 'change has fired once'); }); + it("change fires every time tomorrow is typed and normalized", () => { + let tomorrowDate; + const lblChangeCounter = browser.$("#lblTomorrow"); + const lblTomorrowDate = browser.$("#lblTomorrowDate"); + + datepicker.id = "#dp13"; + + // Type tomorrow. + datepicker.innerInput.click(); + datepicker.innerInput.keys("tomorrow"); + + // Press Enter, store the date and delete it. + datepicker.innerInput.keys("Enter"); + tomorrowDate = lblTomorrowDate.getHTML(false); + browser.keys("\b\b\b\b\b\b\b\b\b\b\b\b\b"); + + // Type tomorrow and press Enter for the second time. + datepicker.innerInput.keys("tomorrow"); + datepicker.innerInput.keys("Enter"); + + // Two change events should be fired and the date should twice normalized + assert.equal(lblChangeCounter.getHTML(false), "2", 'change event is being fired twice'); + assert.equal(lblTomorrowDate.getHTML(false), tomorrowDate, 'tomorrow is normalazid to date twice as well'); + }); + it("today value is normalized and correctly rounded to 00:00:00", () => { datepicker.id = "#dp9"; diff --git a/packages/main/test/specs/Input.spec.js b/packages/main/test/specs/Input.spec.js index f3a40dc35403..6bf46f90dbf7 100644 --- a/packages/main/test/specs/Input.spec.js +++ b/packages/main/test/specs/Input.spec.js @@ -7,12 +7,18 @@ describe("Input general interaction", () => { const input1 = browser.findElementDeep("#input1 >>> input"); const inputResult = browser.findElementDeep("#inputResult >>> input"); + // Start typing. input1.click(); - input1.setValue("abc"); + input1.keys("abc"); + + // Click somewhere else to focus out - should fire change event. inputResult.click(); + // Get back and continue typing. input1.click(); - input1.setValue("def"); + input1.keys("def"); + + // Click somewhere else to force focus out - should fire change event. inputResult.click(); assert.strictEqual(inputResult.getProperty("value"), "2", "change is called twice"); @@ -20,12 +26,33 @@ describe("Input general interaction", () => { it("fires input", () => { const input2 = browser.findElementDeep("#input2 >>> input"); - const inputResult = browser.findElementDeep("#inputResult >>> input"); + const inputLiveChangeResult = browser.findElementDeep("#inputLiveChangeResult >>> input"); input2.click(); - input2.keys("abc"); + input2.setValue("abc"); + + assert.strictEqual(inputLiveChangeResult.getProperty("value"), "3", "input is fired 3 times"); + }); + + it("fires change when same value typed, but value is mutated via API in between", () => { + const inputChange = browser.findElementDeep("#inputChange >>> input"); + const inputChangeResult = browser.findElementDeep("#inputChangeResult >>> input"); - assert.strictEqual(inputResult.getProperty("value"), "3", "input is fired 3 times"); + inputChange.click(); + inputChange.keys("abc"); + + // The submit event listener mutates the value via the API + // Note: along with the sumbit event - the first change event is fired. + inputChange.keys("Enter"); + + // Type the same value once again. + inputChange.keys("abc"); + + // Clicking on another input to force focus out, + // which should trigger second change event, although same value is typed in. + inputChangeResult.click(); + + assert.strictEqual(inputChangeResult.getProperty("value"), "2", "change is called twice"); }); it("handles suggestions", () => { @@ -72,6 +99,5 @@ describe("Input general interaction", () => { assert.strictEqual(suggestionsInput.getProperty("value"), "Condensed", "First item has been selected"); assert.strictEqual(inputResult.getProperty("value"), "4", "suggestionItemSelected event called once"); - - }); + }); }); \ No newline at end of file