fix(ui5-notification-list): fix keyboard issues (#9040)

Fix trigger actions and close buttons with "space". Add left and right arrows navigation.
SAP · May 28, 2024 · f1c0635 · f1c0635
1 parent 92fe765
commit f1c0635
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 16 deletions.
diff --git a/packages/fiori/src/NotificationList.ts b/packages/fiori/src/NotificationList.ts
@@ -1,6 +1,7 @@
 import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
 import { getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js";
 import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js";
+import NavigationMode from "@ui5/webcomponents-base/dist/types/NavigationMode.js";
 import ListItemBase from "@ui5/webcomponents/dist/ListItemBase.js";
 import List from "@ui5/webcomponents/dist/List.js";
 import NotificationListGroupItem from "./NotificationListGroupItem.js";
@@ -24,9 +25,12 @@ import {
 @customElement("ui5-notification-list")
 
 class NotificationList extends List {
+	navigationMode = NavigationMode.Auto;
+
 	constructor() {
 		super();
 		this.accessibleName = NotificationList.i18nFioriBundle.getText(NOTIFICATION_LIST_ACCESSIBLE_NAME);
+		this._itemNavigation._navigationMode = NavigationMode.Auto;
 	}
 
 	static i18nFioriBundle: I18nBundle;

diff --git a/packages/fiori/src/NotificationListGroupItem.ts b/packages/fiori/src/NotificationListGroupItem.ts
@@ -189,12 +189,12 @@ class NotificationListGroupItem extends NotificationListItemBase {
 	}
 
 	async _onkeydown(e: KeyboardEvent) {
-		await super._onkeydown(e);
-
 		if (!this.focused) {
 			return;
 		}
 
+		await super._onkeydown(e);
+
 		const space = isSpace(e);
 		const plus = isPlus(e);
 		const minus = isMinus(e);
@@ -209,13 +209,15 @@ class NotificationListGroupItem extends NotificationListItemBase {
 			// expand
 			if (this.collapsed) {
 				this.toggleCollapsed();
+				e.stopImmediatePropagation();
 			}
 		}
 
 		if (minus || left) {
 			// collapse
 			if (!this.collapsed) {
 				this.toggleCollapsed();
+				e.stopImmediatePropagation();
 			}
 		}
 	}

diff --git a/packages/fiori/src/NotificationListItem.ts b/packages/fiori/src/NotificationListItem.ts
@@ -1,5 +1,5 @@
 import {
-	isSpace, isEnter, isDelete, isF10Shift, isEnterShift, isUp, isDown,
+	isSpace, isDelete, isF10Shift, isEnterShift, isUp, isDown,
 } from "@ui5/webcomponents-base/dist/Keys.js";
 import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
 import property from "@ui5/webcomponents-base/dist/decorators/property.js";
@@ -484,10 +484,6 @@ class NotificationListItem extends NotificationListItemBase {
 	async _onkeydown(e: KeyboardEvent) {
 		await super._onkeydown(e);
 
-		if (isEnter(e)) {
-			this.fireItemPress(e);
-		}
-
 		if (isF10Shift(e)) {
 			e.preventDefault();
 		}
@@ -496,6 +492,11 @@ class NotificationListItem extends NotificationListItemBase {
 	}
 
 	focusSameItemOnNextRow(e: KeyboardEvent) {
+		const target = e.target as HTMLElement;
+		if (!target || target.hasAttribute("ui5-menu-item")) {
+			return;
+		}
+
 		if (this.focused || (!isUp(e) && !isDown(e))) {
 			return;
 		}
@@ -515,11 +516,6 @@ class NotificationListItem extends NotificationListItemBase {
 			return;
 		}
 
-		const target = e.target as HTMLElement;
-		if (!target) {
-			return;
-		}
-
 		const sameItemOnNextRow = nextItem.getHeaderDomRef()!.querySelector(`.${target.className}`) as HTMLElement;
 		if (sameItemOnNextRow && sameItemOnNextRow.offsetParent) {
 			sameItemOnNextRow.focus();

diff --git a/packages/fiori/src/NotificationListItemBase.ts b/packages/fiori/src/NotificationListItemBase.ts
@@ -2,9 +2,12 @@ import { isSpace, isF2 } from "@ui5/webcomponents-base/dist/Keys.js";
 import property from "@ui5/webcomponents-base/dist/decorators/property.js";
 import { getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js";
 import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js";
+import { getTabbableElements } from "@ui5/webcomponents-base/dist/util/TabbableElements.js";
+import getActiveElement from "@ui5/webcomponents-base/dist/util/getActiveElement.js";
 import ListItemBase from "@ui5/webcomponents/dist/ListItemBase.js";
 import Integer from "@ui5/webcomponents-base/dist/types/Integer.js";
 import { getFirstFocusableElement } from "@ui5/webcomponents-base/dist/util/FocusableElements.js";
+import { getEventMark } from "@ui5/webcomponents-base/dist/MarkedEvents.js";
 
 /**
  * @class
@@ -64,8 +67,9 @@ class NotificationListItemBase extends ListItemBase {
 	async _onkeydown(e: KeyboardEvent) {
 		super._onkeydown(e);
 
-		if (isSpace(e)) {
+		if (isSpace(e) && getEventMark(e) !== "button") {
 			e.preventDefault();
+			return;
 		}
 
 		if (isF2(e)) {
@@ -83,6 +87,12 @@ class NotificationListItemBase extends ListItemBase {
 		return this.getFocusDomRef();
 	}
 
+	shouldForwardTabAfter() {
+		const aContent = getTabbableElements(this.getHeaderDomRef()!);
+
+		return aContent.length === 0 || (aContent[aContent.length - 1] === getActiveElement());
+	}
+
 	static async onDefine() {
 		NotificationListItemBase.i18nFioriBundle = await getI18nBundle("@ui5/webcomponents-fiori");
 	}

diff --git a/packages/fiori/test/pages/NotificationList_test_page.html b/packages/fiori/test/pages/NotificationList_test_page.html
@@ -170,7 +170,9 @@
 			</ui5-li-notification-group>
 		</ui5-notification-list>
 	</div>
-
+	<br >
+	<br >
+	<ui5-button>Some button</ui5-button>
 	<script>
 		notificationList.addEventListener("ui5-itemClick", function(event) {
 			clickInput.value = event.detail.item.titleText;

diff --git a/packages/fiori/test/specs/NotificationList.spec.js b/packages/fiori/test/specs/NotificationList.spec.js
@@ -31,10 +31,17 @@ describe("Notification List Item Tests", () => {
 
 		assert.strictEqual((await thirdGroupList.getCSSProperty("display")).value, 'none', "The group is collapsed (items are not visible).");
 		// act
-		await thirdGroupRoot.click();
+		await thirdGroup.click(); // expand
+
+		assert.strictEqual((await thirdGroupList.getCSSProperty("display")).value, 'block', "The group is expanded (items are visible).");
+		// act
+		await thirdGroup.click(); // collapse
+
+		assert.strictEqual((await thirdGroupList.getCSSProperty("display")).value, 'none', "The group is collapsed (items are not visible).");
+		// act
 		await thirdGroupRoot.keys("ArrowRight");
 
-		assert.strictEqual((await thirdGroupList.getCSSProperty("display")).value, 'block', "The group is collapsed (items are not visible).");
+		assert.strictEqual((await thirdGroupList.getCSSProperty("display")).value, 'block', "The group is expanded (items are visible).");
 		// act
 		await thirdGroupRoot.keys("ArrowLeft");
 
@@ -351,6 +358,29 @@ describe("Keyboard navigation", () => {
 
 		await browser.keys("ArrowUp");
 		assert.ok(await browser.$("#nlgi1").isFocused(), "First group is focused.");
+
+		await browser.keys("ArrowRight");
+		assert.ok(await browser.$("#nli1").isFocused(), "First item is focused.");
+
+		await browser.keys("ArrowRight");
+		assert.ok(await browser.$("#nli2").isFocused(), "Second item is focused.");
+
+		await browser.keys("ArrowRight");
+		assert.ok(await browser.$("#nlgi2").isFocused(), "Second group  is focused.");
+
+		await browser.keys("ArrowLeft");
+		assert.ok(await browser.$("#nlgi2").hasAttribute("collapsed"), "Second group is collapsed.");
+		assert.ok(await browser.$("#nlgi2").isFocused(), "Second group is focused.");
+
+		await browser.keys("ArrowLeft");
+		assert.ok(await browser.$("#nli2").isFocused(), "Second item is focused.");
+
+		await browser.keys("ArrowRight");
+		assert.ok(await browser.$("#nlgi2").isFocused(), "Second group is focused.");
+
+		await browser.keys("ArrowRight");
+		assert.notOk(await browser.$("#nlgi2").hasAttribute("collapsed"), "Second group is expanded.");
+		assert.ok(await browser.$("#nlgi2").isFocused(), "Second group is focused.");
 	});
 
 	it("Tab and F2 navigation", async () => {

diff --git a/packages/main/src/ListItemBase.ts b/packages/main/src/ListItemBase.ts
@@ -123,6 +123,10 @@ class ListItemBase extends UI5Element implements ITabbable {
 			return this._handleTabPrevious(e);
 		}
 
+		if (getEventMark(e) === "button") {
+			return;
+		}
+
 		if (isSpace(e)) {
 			e.preventDefault();
 		}