fix(ui5-select): improve keyboard handling (#1771)

Author: fifoosid

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() {

packages/main/test/pages/Select.html

@@ -66,6 +66,12 @@ <h2>Selection not cycling</h2>
 		<ui5-option>Condensed</ui5-option>
 	</ui5-select>
 
+	<ui5-select id="keyboardHandling">
+		<ui5-option>Banana</ui5-option>
+		<ui5-option selected>Orange</ui5-option>
+		<ui5-option>Watermelon</ui5-option>
+	</ui5-select>
+
 	<h2> Change event counter holder</h2>
 	<ui5-input id="inputResult"></ui5-input>
 

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();