fix(ui5-list): correct keyboard handling (#4890)

packages/main/src/List.hbs

@@ -74,6 +74,7 @@
 		<div
 			tabindex="0"
 			role="button"
+			id="{{_id}}-growing-btn" 
 			aria-labelledby="{{_id}}-growingButton-text"
 			?active="{{_loadMoreActive}}"
 			@click="{{_onLoadMoreClick}}"

packages/main/src/List.js

@@ -5,7 +5,12 @@ import ItemNavigation from "@ui5/webcomponents-base/dist/delegate/ItemNavigation
 import { isIE } from "@ui5/webcomponents-base/dist/Device.js";
 import { renderFinished } from "@ui5/webcomponents-base/dist/Render.js";
 import { getLastTabbableElement } from "@ui5/webcomponents-base/dist/util/TabbableElements.js";
-import { isTabNext, isSpace, isEnter } from "@ui5/webcomponents-base/dist/Keys.js";
+import {
+	isTabNext,
+	isSpace,
+	isEnter,
+	isTabPrevious,
+} from "@ui5/webcomponents-base/dist/Keys.js";
 import Integer from "@ui5/webcomponents-base/dist/types/Integer.js";
 import NavigationMode from "@ui5/webcomponents-base/dist/types/NavigationMode.js";
 import { getEffectiveAriaLabelText } from "@ui5/webcomponents-base/dist/util/AriaLabelHelper.js";
@@ -759,9 +764,17 @@ class List extends UI5Element {
 		}
 
 		if (isTabNext(event)) {
-			this.setPreviouslyFocusedItem(event.target);
 			this.focusAfterElement();
 		}
+
+		if (isTabPrevious(event)) {
+			if (this.getPreviouslyFocusedItem()) {
+				this.focusPreviouslyFocusedItem();
+			} else {
+				this.focusFirstItem();
+			}
+			event.preventDefault();
+		}
 	}
 
 	_onLoadMoreKeyup(event) {
@@ -824,23 +837,34 @@ class List extends UI5Element {
 	}
 
 	_onfocusin(event) {
+		const target = this.getNormalizedTarget(event.target);
 		// If the focusin event does not origin from one of the 'triggers' - ignore it.
-		if (!this.isForwardElement(this.getNormalizedTarget(event.target))) {
+		if (!this.isForwardElement(target)) {
 			event.stopImmediatePropagation();
 			return;
 		}
 
 		// The focus arrives in the List for the first time.
 		// If there is selected item - focus it or focus the first item.
 		if (!this.getPreviouslyFocusedItem()) {
-			this.focusFirstItem();
+			if (this.growsWithButton && this.isForwardAfterElement(target)) {
+				this.focusGrowingButton();
+			} else {
+				this.focusFirstItem();
+			}
 			event.stopImmediatePropagation();
 			return;
 		}
 
 		// The focus returns to the List,
 		// focus the first selected item or the previously focused element.
 		if (!this.getForwardingFocus()) {
+			if (this.growsWithButton && this.isForwardAfterElement(target)) {
+				this.focusGrowingButton();
+				event.stopImmediatePropagation();
+				return;
+			}
+
 			this.focusPreviouslyFocusedItem();
 			event.stopImmediatePropagation();
 		}
@@ -850,13 +874,19 @@ class List extends UI5Element {
 
 	isForwardElement(node) {
 		const nodeId = node.id;
-		const afterElement = this.getAfterElement();
 		const beforeElement = this.getBeforeElement();
 
 		if (this._id === nodeId || (beforeElement && beforeElement.id === nodeId)) {
 			return true;
 		}
 
+		return this.isForwardAfterElement(node);
+	}
+
+	isForwardAfterElement(node) {
+		const nodeId = node.id;
+		const afterElement = this.getAfterElement();
+
 		return afterElement && afterElement.id === nodeId;
 	}
 
@@ -920,6 +950,9 @@ class List extends UI5Element {
 
 		if (!this.growsWithButton) {
 			this.focusAfterElement();
+		} else {
+			this.focusGrowingButton();
+			event.preventDefault();
 		}
 	}
 
@@ -933,6 +966,18 @@ class List extends UI5Element {
 		this.getAfterElement().focus();
 	}
 
+	focusGrowingButton() {
+		const growingBtn = this.getGrowingButton();
+
+		if (growingBtn) {
+			growingBtn.focus();
+		}
+	}
+
+	getGrowingButton() {
+		return this.shadowRoot.querySelector(`#${this._id}-growing-btn`);
+	}
+
 	/**
 	 * Focuses the first list item and sets its tabindex to "0" via the ItemNavigation
 	 * @protected

packages/main/src/ListItemBase.js

@@ -117,7 +117,9 @@ class ListItemBase extends UI5Element {
 		const target = event.target;
 
 		if (this.shouldForwardTabAfter(target)) {
-			this.fireEvent("_forward-after", { item: target });
+			if (!this.fireEvent("_forward-after", { item: target }, true)) {
+				event.preventDefault();
+			}
 		}
 	}
 

packages/main/test/pages/List_test_page.html

@@ -38,15 +38,6 @@
 		</ui5-list>
 	</section>
 
-	<br><br><br>
-
-	<ui5-list id="growingListButton" growing="Button" class="list_test_page4auto">
-		<ui5-li>Javascript</ui5-li>
-		<ui5-li>PHP</ui5-li>
-	</ui5-list>
-
-	<button id="nextInteractiveElement">Next Interactive element</button>
-
 	<br><br><br>
 
 	<br/>
@@ -132,7 +123,8 @@
 
 	<ui5-input id="fieldMultiSelResult"></ui5-input>
 
-	<ui5-list id="keyboardTestList">
+	<ui5-button id="beforeBtn">Before button</ui5-button>
+	<ui5-list id="keyboardTestList" growing="Button">
 		<div slot="header" class="header"><ui5-button id="headerBtn" design="Accept">Export</ui5-button></div>
 
 		<ui5-li class="firstItem">Argentina</ui5-li>
@@ -147,7 +139,7 @@
 				<ui5-radio-button text="Option C" disabled="disabled"></ui5-radio-button>
 		</ui5-li-custom>
 	</ui5-list>
-	<ui5-button id="randomBtn">Random button</ui5-button>
+	<ui5-button id="afterBtn">After button</ui5-button>
 
 	<ui5-list id="no-data-list" header-text="API: noDataText" separators="None" no-data-text="No Data Available"></ui5-list>
 

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

@@ -215,19 +215,55 @@ describe("List Tests", () => {
 		assert.strictEqual(firstItemHeight, ITEM_WITH_DESCRIPTION_AND_TITLE_HEIGHT, "The size of the item is : " + firstItemHeight);
 	});
 
+	it("keyboard handling on SHIFT + TAB", async () => {
+		const list = await browser.$("#keyboardTestList");
+		const growingBtn = await list.shadow$('[id$="growing-btn"]');
+		const headerBtn = await browser.$("#headerBtn");
+		const firstItem = await browser.$("ui5-li.firstItem");
+		const afterBtn = await browser.$("#afterBtn");
+		const beforeBtn = await browser.$("#beforeBtn");
+
+		await afterBtn.click();
+		assert.ok(await afterBtn.isFocused(), "after btn is focused");
+
+		// act: Shift + Tab from element outside of the list -> tab should go to growing button if exist
+		await afterBtn.keys(["Shift", "Tab"]);
+		assert.ok(await growingBtn.isFocusedDeep(), "growing buton is focused");
+
+		// act: Shift + Tab from growing button -> should focus previously focused item or first item
+		await growingBtn.keys(["Shift", "Tab"]);
+		assert.ok(await firstItem.isFocused(), "first item is focused");
+
+		// act: Shift + Tab from first item -> focus should go to the header content (if there is tabbable element)
+		await firstItem.keys(["Shift", "Tab"]);
+		assert.ok(await headerBtn.isFocused(), "header button item is focused");
+
+		// act: Shift + Tab from the growing button - the focus should leave the ui5-list
+		// and before button should be focused
+		await headerBtn.keys(["Shift", "Tab"]);
+		assert.ok(await beforeBtn.isFocused(), "first item is focused");
+	});
+
 	it("keyboard handling on TAB", async () => {
+		const list = await browser.$("#keyboardTestList");
+		const growingBtn = await list.shadow$('[id$="growing-btn"]');
 		const headerBtn = await browser.$("#headerBtn");
 		const firstItem = await browser.$("ui5-li.firstItem");
+		const afterBtn = await browser.$("#afterBtn");
+		const beforeBtn = await browser.$("#beforeBtn");
 		const item = await browser.$("ui5-li-custom.item");
 		const itemBtn = await browser.$("ui5-button.itemBtn");
 		const itemLink = await browser.$("ui5-link.itemLink");
 		const itemRadioBtn = await browser.$("ui5-radio-button.itemRadio");
-		const randomBtn = await browser.$("#randomBtn");
 
-		await headerBtn.click();
-		assert.ok(await headerBtn.isFocused(), "header btn is focused");
+		await beforeBtn.click();
+		assert.ok(await beforeBtn.isFocused(), "before button is focused");
+
+		// act: Tab from element outside of the list -> focus should go to the header content (if there is tabbable element)
+		await beforeBtn.keys("Tab");
+		assert.ok(await headerBtn.isFocused(), "header button is focused");
 
-		// act: TAB from headerButton -> the focus should go to the 1st selected item
+		// act: TAB from headerButton -> the focus should go to the 1st item
 		await headerBtn.keys("Tab");
 		assert.ok(await firstItem.isFocused(), "first item is focused");
 
@@ -247,10 +283,13 @@ describe("List Tests", () => {
 		await itemLink.keys("Tab");
 		assert.ok(await itemRadioBtn.isFocused(), "the last tabbable element (radio) is focused");
 
-		// act: TAB from the "Option B" radio button - the focus should leave  the ui5-list
-		// and Random button should be focused
-		await itemLink.keys("Tab");
-		assert.ok(await randomBtn.isFocused(), "element outside of the list is focused");
+		await firstItem.keys("Tab");
+		assert.ok(await growingBtn.isFocusedDeep(), "growing buton is focused");
+
+		// act: TAB from the growing button - the focus should leave the ui5-list
+		// and after button should be focused
+		await growingBtn.keys("Tab");
+		assert.ok(await afterBtn.isFocused(), "element outside of the list is focused");
 	});
 
 	it("does not focus next / prev item when right / left arrow is pressed", async () => {
@@ -422,16 +461,6 @@ describe("List Tests", () => {
 		assert.ok(await item3.getProperty("focused"), "disabled item is skipped");
 	});
 
-	it('should focus next interactive element if TAB is pressed when focus is on "More" growing button', async () => {
-		const growingListButton = await browser.$('#growingListButton').shadow$("div[growing-button-inner]");
-		const nextInteractiveElement = await browser.$('#nextInteractiveElement');
-
-		await growingListButton.click() // focus growing button
-		await growingListButton.keys("Tab") // focus next list
-
-		assert.ok(await nextInteractiveElement.isFocused(), "Focus is moved to next interactive element.");
-	});
-
 	it('should include selected state text', async () => {
 		const item = await browser.$("#justList #justList-country");
 		const notSelectedItem = await browser.$("#listSelectedItem #not-selected-country");