From f555180a5b8959fae9dbcb7d3a5b00c00a72630d Mon Sep 17 00:00:00 2001 From: Filip Siderov Date: Mon, 15 Jun 2020 14:44:37 +0300 Subject: [PATCH] fix(ui5-select): improve keyboard handling (#1771) --- packages/main/src/Select.js | 13 ++++++- packages/main/test/pages/Select.html | 6 +++ packages/main/test/specs/Select.spec.js | 50 +++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/packages/main/src/Select.js b/packages/main/src/Select.js index 04297ea332fb..8e6bd4977086 100644 --- a/packages/main/src/Select.js +++ b/packages/main/src/Select.js @@ -7,6 +7,8 @@ import { isEnter, isEscape, isShow, + isTabNext, + isTabPrevious, } from "@ui5/webcomponents-base/dist/Keys.js"; import { getFeature } from "@ui5/webcomponents-base/dist/FeaturesRegistry.js"; import ValueState from "@ui5/webcomponents-base/dist/types/ValueState.js"; @@ -322,7 +324,14 @@ class Select extends UI5Element { } _onkeydown(event) { + const isTab = (isTabNext(event) || isTabPrevious(event)); + + if (isTab && this.responsivePopover && this.responsivePopover.opened) { + this.responsivePopover.close(); + } + if (isShow(event)) { + event.preventDefault(); this._toggleRespPopover(); } @@ -477,7 +486,9 @@ class Select extends UI5Element { } get tabIndex() { - return this.disabled ? "-1" : "0"; + return this.disabled + && this.responsivePopover // Handles focus on Tab/Shift + Tab when the popover is opened + && this.responsivePopover.opened ? "-1" : "0"; } static async onDefine() { diff --git a/packages/main/test/pages/Select.html b/packages/main/test/pages/Select.html index c615878b52bd..11918d57e8ac 100644 --- a/packages/main/test/pages/Select.html +++ b/packages/main/test/pages/Select.html @@ -66,6 +66,12 @@

Selection not cycling

Condensed + + Banana + Orange + Watermelon + +

Change event counter holder

diff --git a/packages/main/test/specs/Select.spec.js b/packages/main/test/specs/Select.spec.js index 84a2fc7b87fd..214c3144c535 100644 --- a/packages/main/test/specs/Select.spec.js +++ b/packages/main/test/specs/Select.spec.js @@ -74,6 +74,48 @@ describe("Select general interaction", () => { assert.strictEqual(inputResult.getProperty("value"), "5", "Change event should have fired twice"); }); + it("changes selection on Tab", () => { + const select = browser.$("#keyboardHandling"); + const EXPECTED_SELECTION_TEXT = "Banana"; + + select.click(); // Open select + select.click(); // Close select. Focus is on the select now + select.keys("Space"); + + select.keys("ArrowUp"); + select.keys("Tab"); + const selectText = select.shadow$("ui5-label"); + + assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Arrow Up should change selected item"); + + const focusedElementId = browser.execute(() => { + return document.activeElement.id; + }); + + assert.strictEqual(focusedElementId, browser.$("#inputResult").getAttribute("id"), "Next focusable element is focused"); + }); + + it("changes selection on Shift + Tab", () => { + const select = browser.$("#keyboardHandling"); + const EXPECTED_SELECTION_TEXT = "Orange"; + + select.click(); // Open select + select.click(); // Close select. Focus is on the select now + select.keys("Space"); + + select.keys("ArrowDown"); + browser.keys(["Shift", "Tab"]); + const selectText = select.shadow$("ui5-label"); + + assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Arrow Down should change selected item"); + + const focusedElementId = browser.execute(() => { + return document.activeElement.id; + }); + + assert.strictEqual(focusedElementId, browser.$("#mySelectEsc").getAttribute("id"), "Previous focusable element is focused"); + }); + it("tests selection does not cycle with ArrowDown", () => { const select = $("#selectionNotCycling"); const EXPECTED_SELECTION_TEXT = "Opt3"; @@ -83,7 +125,7 @@ describe("Select general interaction", () => { assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text is " + EXPECTED_SELECTION_TEXT); // The last item is already selected - pressing ArrowDown should not change the focus or the selection - select.keys("ArrowDown"); + select.keys("ArrowDown"); assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text remains " + EXPECTED_SELECTION_TEXT); // Close the select not to cover other components that tests would try to click @@ -99,7 +141,7 @@ describe("Select general interaction", () => { assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text is " + EXPECTED_SELECTION_TEXT); // The last item is already selected - pressing ArrowUp should not change the focus or the selection - select.keys("ArrowUp"); + select.keys("ArrowUp"); assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text remains " + EXPECTED_SELECTION_TEXT); // Close the select not to cover other components that tests would try to click @@ -221,7 +263,7 @@ describe("Select general interaction", () => { select.keys("Escape"); select.click(); - const staticAreaItemClassName = browser.getStaticAreaItemClassName("#mySelect"); + const staticAreaItemClassName = browser.getStaticAreaItemClassName("#mySelect"); const firstItem = browser.$(`.${staticAreaItemClassName}`).shadow$("ui5-li:first-child"); firstItem.click(); @@ -245,7 +287,7 @@ describe("Select general interaction", () => { assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) !== -1, "Select label is correct."); // verify that ESC does not interfere when the picker is closed - select.keys("Escape"); + select.keys("Escape"); select.click(); thirdItem.click();