From f8c8e4c3aaee9da3c3e13082b05072306c7bf692 Mon Sep 17 00:00:00 2001 From: Iliana Bobeva Date: Tue, 6 Jul 2021 16:25:25 +0300 Subject: [PATCH 1/6] Select typing focus fix --- packages/main/src/Select.js | 58 ++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/main/src/Select.js b/packages/main/src/Select.js index b9bdbbdb6855..e9b46e4fbd03 100644 --- a/packages/main/src/Select.js +++ b/packages/main/src/Select.js @@ -316,6 +316,8 @@ class Select extends UI5Element { this._selectedIndexBeforeOpen = -1; this._escapePressed = false; this._lastSelectedOption = null; + this._sTypedChars = ""; + this._iTypingTimeoutID = -1; this.i18nBundle = getI18nBundle("@ui5/webcomponents"); } @@ -458,11 +460,65 @@ class Select extends UI5Element { this._handleEndKey(event); } else if (isEnter(event)) { this._handleSelectionChange(); - } else { + } else if (isDown(event) || isUp(event)) { this._handleArrowNavigation(event); + } else if (event.keyCode >= 65 && event.keyCode <= 90) { + this._handleKeyboardNavigation(event); } } + _handleKeyboardNavigation(event) { + //debugger; + + // note: jQuery oEvent.which normalizes oEvent.keyCode and oEvent.charCode + const sTypedCharacter = event.key; + let sText; + console.log(sTypedCharacter); + this._sTypedChars += sTypedCharacter; + + // We check if we have more than one characters and they are all duplicate, we set the + // text to be the last input character (sTypedCharacter). If not, we set the text to be + // the whole input string. + + sText = (/^(.)\1+$/i).test(this._sTypedChars) ? sTypedCharacter : this._sTypedChars; + + clearTimeout(this._iTypingTimeoutID); + + this._iTypingTimeoutID = setTimeout(function() { + this._sTypedChars = ""; + this._iTypingTimeoutID = -1; + this._selectTypedItem(sText); + }.bind(this), 100); + } + + _selectTypedItem(sText) { + const currentIndex = this._selectedIndex; + let oItemToSelect =this._searchNextItemByText(sText); + + if (oItemToSelect) { + const nextIndex = this._getSelectedItemIndex(oItemToSelect); + + this._changeSelectedItem(this._selectedIndex, nextIndex); + + if (currentIndex !== this._selectedIndex) { + // Announce new item even if picker is opened. + // The aria-activedescendents attribute can't be used, + // because listitem elements are in different shadow dom + this.itemSelectionAnnounce(); + } + } + } + + _searchNextItemByText(sText) { + let aOrderedOptions = this.options.slice(0); + let aOptionsAfterSelected = aOrderedOptions.splice(this._selectedIndex + 1, aOrderedOptions.length - this._selectedIndex); + let aOptionsBeforeSelected = aOrderedOptions.splice(0, aOrderedOptions.length - 1); + + aOrderedOptions = aOptionsAfterSelected.concat(aOptionsBeforeSelected); + + return aOrderedOptions.find(o => o.textContent.toLowerCase().startsWith(sText)); + } + _handleHomeKey(event) { event.preventDefault(); this._changeSelectedItem(this._selectedIndex, 0); From b5339ec604f2e1c6c6e73dc0b17563309e1ebe46 Mon Sep 17 00:00:00 2001 From: Iliana Bobeva Date: Mon, 26 Jul 2021 10:39:49 +0300 Subject: [PATCH 2/6] Add units tests --- packages/main/src/Select.js | 21 +++++++----------- packages/main/test/specs/Select.spec.js | 29 +++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/packages/main/src/Select.js b/packages/main/src/Select.js index e9b46e4fbd03..55158567ed90 100644 --- a/packages/main/src/Select.js +++ b/packages/main/src/Select.js @@ -468,12 +468,9 @@ class Select extends UI5Element { } _handleKeyboardNavigation(event) { - //debugger; - - // note: jQuery oEvent.which normalizes oEvent.keyCode and oEvent.charCode const sTypedCharacter = event.key; let sText; - console.log(sTypedCharacter); + this._sTypedChars += sTypedCharacter; // We check if we have more than one characters and they are all duplicate, we set the @@ -484,16 +481,17 @@ class Select extends UI5Element { clearTimeout(this._iTypingTimeoutID); - this._iTypingTimeoutID = setTimeout(function() { + this._iTypingTimeoutID = setTimeout(async () => { this._sTypedChars = ""; this._iTypingTimeoutID = -1; - this._selectTypedItem(sText); - }.bind(this), 100); + }, 1000); + + this._selectTypedItem(sText); } _selectTypedItem(sText) { const currentIndex = this._selectedIndex; - let oItemToSelect =this._searchNextItemByText(sText); + let oItemToSelect = this._searchNextItemByText(sText); if (oItemToSelect) { const nextIndex = this._getSelectedItemIndex(oItemToSelect); @@ -501,9 +499,6 @@ class Select extends UI5Element { this._changeSelectedItem(this._selectedIndex, nextIndex); if (currentIndex !== this._selectedIndex) { - // Announce new item even if picker is opened. - // The aria-activedescendents attribute can't be used, - // because listitem elements are in different shadow dom this.itemSelectionAnnounce(); } } @@ -511,8 +506,8 @@ class Select extends UI5Element { _searchNextItemByText(sText) { let aOrderedOptions = this.options.slice(0); - let aOptionsAfterSelected = aOrderedOptions.splice(this._selectedIndex + 1, aOrderedOptions.length - this._selectedIndex); - let aOptionsBeforeSelected = aOrderedOptions.splice(0, aOrderedOptions.length - 1); + const aOptionsAfterSelected = aOrderedOptions.splice(this._selectedIndex + 1, aOrderedOptions.length - this._selectedIndex); + const aOptionsBeforeSelected = aOrderedOptions.splice(0, aOrderedOptions.length - 1); aOrderedOptions = aOptionsAfterSelected.concat(aOptionsBeforeSelected); diff --git a/packages/main/test/specs/Select.spec.js b/packages/main/test/specs/Select.spec.js index e437bb305881..c591c3e9f6ca 100644 --- a/packages/main/test/specs/Select.spec.js +++ b/packages/main/test/specs/Select.spec.js @@ -111,13 +111,13 @@ describe("Select general interaction", () => { select.keys("ArrowDown"); assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT2), "Arrow Down should change selected item"); assert.strictEqual(selectionText.getHTML(false), EXPECTED_SELECTION_TEXT2, "Selection announcement text should be equalt to the current selected item's text"); - + // change previewed item with picker opened select.click(); select.keys("ArrowUp"); assert.strictEqual(selectionText.getHTML(false), EXPECTED_SELECTION_TEXT1, "Selection announcement text should be equalt to the current selected item's text"); select.keys("Escape"); - + // change selection with picker opened select.click(); select.keys("ArrowUp"); @@ -205,6 +205,31 @@ describe("Select general interaction", () => { select.keys("Escape"); }); + it("changes selection with typing single letter", () => { + const select = browser.$("#keyboardHandling"); + const EXPECTED_SELECTION_TEXT = "Banana"; + + select.click(); // Open select + select.keys("b"); + + const selectText = select.shadow$(".ui5-select-label-root"); + + assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Typing letter should change selection"); + }); + + it("changes selection with typing more letters", () => { + const select = browser.$("#mySelect3"); + const EXPECTED_SELECTION_TEXT = "Brazil"; + + select.click(); // Open select + select.keys("b"); + select.keys("r"); + + const selectText = select.shadow$(".ui5-select-label-root"); + + assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Typing text should change selection"); + }); + it("opens upon space", () => { browser.url(`http://localhost:${PORT}/test-resources/pages/Select.html`); From 2ec3c8affb10f7adb3085279b25f5b0f3783e2a9 Mon Sep 17 00:00:00 2001 From: Iliana Bobeva Date: Mon, 26 Jul 2021 10:59:13 +0300 Subject: [PATCH 3/6] Fix if-else section --- packages/main/src/Select.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/main/src/Select.js b/packages/main/src/Select.js index 55158567ed90..b6759b581596 100644 --- a/packages/main/src/Select.js +++ b/packages/main/src/Select.js @@ -460,10 +460,10 @@ class Select extends UI5Element { this._handleEndKey(event); } else if (isEnter(event)) { this._handleSelectionChange(); - } else if (isDown(event) || isUp(event)) { - this._handleArrowNavigation(event); } else if (event.keyCode >= 65 && event.keyCode <= 90) { this._handleKeyboardNavigation(event); + } else { + this._handleArrowNavigation(event); } } From cbcc3046290fb67f2c88593a6ae6d4e36cd0b658 Mon Sep 17 00:00:00 2001 From: Iliana Bobeva Date: Mon, 26 Jul 2021 14:11:52 +0300 Subject: [PATCH 4/6] Fix eslint errors --- packages/main/src/Select.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/main/src/Select.js b/packages/main/src/Select.js index b6759b581596..6c6f1c1f080c 100644 --- a/packages/main/src/Select.js +++ b/packages/main/src/Select.js @@ -469,7 +469,6 @@ class Select extends UI5Element { _handleKeyboardNavigation(event) { const sTypedCharacter = event.key; - let sText; this._sTypedChars += sTypedCharacter; @@ -477,7 +476,7 @@ class Select extends UI5Element { // text to be the last input character (sTypedCharacter). If not, we set the text to be // the whole input string. - sText = (/^(.)\1+$/i).test(this._sTypedChars) ? sTypedCharacter : this._sTypedChars; + const sText = (/^(.)\1+$/i).test(this._sTypedChars) ? sTypedCharacter : this._sTypedChars; clearTimeout(this._iTypingTimeoutID); @@ -491,17 +490,17 @@ class Select extends UI5Element { _selectTypedItem(sText) { const currentIndex = this._selectedIndex; - let oItemToSelect = this._searchNextItemByText(sText); + const oItemToSelect = this._searchNextItemByText(sText); - if (oItemToSelect) { - const nextIndex = this._getSelectedItemIndex(oItemToSelect); + if (oItemToSelect) { + const nextIndex = this._getSelectedItemIndex(oItemToSelect); - this._changeSelectedItem(this._selectedIndex, nextIndex); + this._changeSelectedItem(this._selectedIndex, nextIndex); - if (currentIndex !== this._selectedIndex) { - this.itemSelectionAnnounce(); - } + if (currentIndex !== this._selectedIndex) { + this.itemSelectionAnnounce(); } + } } _searchNextItemByText(sText) { From fc6f345f5e3a60f54895b6cc6399bd6c3f95c849 Mon Sep 17 00:00:00 2001 From: Iliana Bobeva Date: Tue, 27 Jul 2021 13:17:29 +0300 Subject: [PATCH 5/6] Fix typing with CAPSLOCK on --- packages/main/src/Select.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/src/Select.js b/packages/main/src/Select.js index 6c6f1c1f080c..37b6c1673169 100644 --- a/packages/main/src/Select.js +++ b/packages/main/src/Select.js @@ -468,7 +468,7 @@ class Select extends UI5Element { } _handleKeyboardNavigation(event) { - const sTypedCharacter = event.key; + const sTypedCharacter = event.key.toLowerCase(); this._sTypedChars += sTypedCharacter; From e813c880d090b0115c3b28e1d282be7275f95396 Mon Sep 17 00:00:00 2001 From: Iliana Bobeva Date: Wed, 28 Jul 2021 17:20:58 +0300 Subject: [PATCH 6/6] Enable typing of numbers and characters, fix comments --- packages/main/src/Select.js | 80 +++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/packages/main/src/Select.js b/packages/main/src/Select.js index 37b6c1673169..57258a9aaf78 100644 --- a/packages/main/src/Select.js +++ b/packages/main/src/Select.js @@ -316,8 +316,8 @@ class Select extends UI5Element { this._selectedIndexBeforeOpen = -1; this._escapePressed = false; this._lastSelectedOption = null; - this._sTypedChars = ""; - this._iTypingTimeoutID = -1; + this._typedChars = ""; + this._typingTimeoutID = -1; this.i18nBundle = getI18nBundle("@ui5/webcomponents"); } @@ -460,40 +460,45 @@ class Select extends UI5Element { this._handleEndKey(event); } else if (isEnter(event)) { this._handleSelectionChange(); - } else if (event.keyCode >= 65 && event.keyCode <= 90) { - this._handleKeyboardNavigation(event); - } else { + } else if (isUp(event) || isDown(event)) { this._handleArrowNavigation(event); + } else { + this._handleKeyboardNavigation(event); } } _handleKeyboardNavigation(event) { - const sTypedCharacter = event.key.toLowerCase(); + // Waiting for the actual symbol to trigger the keydown event + if (event.shiftKey && event.key === "Shift") { + return; + } + + const typedCharacter = event.key.toLowerCase(); - this._sTypedChars += sTypedCharacter; + this._typedChars += typedCharacter; // We check if we have more than one characters and they are all duplicate, we set the - // text to be the last input character (sTypedCharacter). If not, we set the text to be + // text to be the last input character (typedCharacter). If not, we set the text to be // the whole input string. - const sText = (/^(.)\1+$/i).test(this._sTypedChars) ? sTypedCharacter : this._sTypedChars; + const text = (/^(.)\1+$/i).test(this._typedChars) ? typedCharacter : this._typedChars; - clearTimeout(this._iTypingTimeoutID); + clearTimeout(this._typingTimeoutID); - this._iTypingTimeoutID = setTimeout(async () => { - this._sTypedChars = ""; - this._iTypingTimeoutID = -1; + this._typingTimeoutID = setTimeout(() => { + this._typedChars = ""; + this._typingTimeoutID = -1; }, 1000); - this._selectTypedItem(sText); + this._selectTypedItem(text); } - _selectTypedItem(sText) { + _selectTypedItem(text) { const currentIndex = this._selectedIndex; - const oItemToSelect = this._searchNextItemByText(sText); + const itemToSelect = this._searchNextItemByText(text); - if (oItemToSelect) { - const nextIndex = this._getSelectedItemIndex(oItemToSelect); + if (itemToSelect) { + const nextIndex = this._getSelectedItemIndex(itemToSelect); this._changeSelectedItem(this._selectedIndex, nextIndex); @@ -503,14 +508,14 @@ class Select extends UI5Element { } } - _searchNextItemByText(sText) { - let aOrderedOptions = this.options.slice(0); - const aOptionsAfterSelected = aOrderedOptions.splice(this._selectedIndex + 1, aOrderedOptions.length - this._selectedIndex); - const aOptionsBeforeSelected = aOrderedOptions.splice(0, aOrderedOptions.length - 1); + _searchNextItemByText(text) { + let orderedOptions = this.options.slice(0); + const optionsAfterSelected = orderedOptions.splice(this._selectedIndex + 1, orderedOptions.length - this._selectedIndex); + const optionsBeforeSelected = orderedOptions.splice(0, orderedOptions.length - 1); - aOrderedOptions = aOptionsAfterSelected.concat(aOptionsBeforeSelected); + orderedOptions = optionsAfterSelected.concat(optionsBeforeSelected); - return aOrderedOptions.find(o => o.textContent.toLowerCase().startsWith(sText)); + return orderedOptions.find(option => option.textContent.toLowerCase().startsWith(text)); } _handleHomeKey(event) { @@ -580,24 +585,21 @@ class Select extends UI5Element { let nextIndex = -1; const currentIndex = this._selectedIndex; const isDownKey = isDown(event); - const isUpKey = isUp(event); - if (isDownKey || isUpKey) { - event.preventDefault(); - if (isDownKey) { - nextIndex = this._getNextOptionIndex(); - } else { - nextIndex = this._getPreviousOptionIndex(); - } + event.preventDefault(); + if (isDownKey) { + nextIndex = this._getNextOptionIndex(); + } else { + nextIndex = this._getPreviousOptionIndex(); + } - this._changeSelectedItem(this._selectedIndex, nextIndex); + this._changeSelectedItem(this._selectedIndex, nextIndex); - if (currentIndex !== this._selectedIndex) { - // Announce new item even if picker is opened. - // The aria-activedescendents attribute can't be used, - // because listitem elements are in different shadow dom - this.itemSelectionAnnounce(); - } + if (currentIndex !== this._selectedIndex) { + // Announce new item even if picker is opened. + // The aria-activedescendents attribute can't be used, + // because listitem elements are in different shadow dom + this.itemSelectionAnnounce(); } }