fix(ui5-shellbar): fix open popup on notificationClick (#1632)

The built-in notification icon can overflow and if you try to open another popover by clicking on the corresponding list item within the overflow, it does not open as the provided targetRef is not correct. Now it provides the correct targetRef, considering whether the notification icon is in or out of the overflow. In addition, all samples and test pages that opens popover from the notification icon use event.preventDefault() to prevent the overflow popover from closing.
SAP · May 18, 2020 · 04b44b4 · 04b44b4
1 parent d9a5e59
commit 04b44b4
Show file tree

Hide file tree

Showing 9 changed files with 36 additions and 16 deletions.
diff --git a/packages/fiori/src/ShellBar.js b/packages/fiori/src/ShellBar.js
@@ -694,9 +694,11 @@ class ShellBar extends UI5Element {
 	}
 
 	_handleNotificationsPress(event) {
-		this.fireEvent("notificationsClick", {
-			targetRef: this.shadowRoot.querySelector(".ui5-shellbar-bell-button"),
-		});
+		const notificationIconRef = this.shadowRoot.querySelector(".ui5-shellbar-bell-button");
+
+		this._defaultItemPressPrevented = !this.fireEvent("notificationsClick", {
+			targetRef: notificationIconRef.classList.contains("ui5-shellbar-hidden-button") ? event.target : notificationIconRef,
+		}, true);
 	}
 
 	_handleProfilePress(event) {

diff --git a/packages/fiori/test/pages/NotificationListGroupItem.html b/packages/fiori/test/pages/NotificationListGroupItem.html
@@ -314,6 +314,7 @@ <h3>Events on ui5-list level</h3>
 		});
 
 		shellbar.addEventListener("ui5-notificationsClick", function(event) {
+			event.preventDefault();
 			notificationsPopover.openBy(event.detail.targetRef);
 		});
 	</script>

diff --git a/packages/fiori/test/pages/NotificationListItem.html b/packages/fiori/test/pages/NotificationListItem.html
@@ -282,6 +282,7 @@ <h3>Events on ui5-list level</h3>
 		});
 
 		shellbar.addEventListener("ui5-notificationsClick", function(event) {
+			event.preventDefault();
 			notificationsPopover.openBy(event.detail.targetRef);
 		});
 	</script>

diff --git a/packages/fiori/test/pages/ShellBar.html b/packages/fiori/test/pages/ShellBar.html
@@ -134,7 +134,7 @@
 <section class="ui5-content-density-compact">
 	<h3>ShellBar in Compact</h3>
 	<div>
-		<ui5-shellbar primary-title="Product Title" show-notifications>
+		<ui5-shellbar id="shelbarCompact" primary-title="Product Title" show-notifications show-product-switch>
 			<ui5-button icon="nav-back" slot="startButton" id="start-button"></ui5-button>
 			<ui5-shellbar-item icon="disconnected" text="Disconnect"></ui5-shellbar-item>
 			<ui5-shellbar-item icon="incoming-call" text="Incoming Calls"></ui5-shellbar-item>
@@ -233,24 +233,26 @@ <h3>ShellBar in Compact</h3>
 	});
 
 	shellbar.addEventListener("ui5-notificationsClick", function(event) {
-		window["press-input"].value = "Notifications"
+		window["press-input"].value = "Notifications";
+		event.preventDefault();
+		popover.openBy(event.detail.targetRef);
 	});
 
 	shellbar.addEventListener("ui5-productSwitchClick", function(event) {
 		event.preventDefault();
-		window["press-input"].value = "Product Switch"
+		window["press-input"].value = "Product Switch";
 	});
 
 	shellbar.addEventListener("ui5-logoClick", function(event) {
-		window["press-input"].value = "Logo"
+		window["press-input"].value = "Logo";
 	});
 
 	shellbarWithLogoClick.addEventListener("ui5-logoClick", function(event) {
 		window["press-input2"].value = shellbarWithLogoClick.primaryTitle.replace(/\s/g, "");
 	});
 
 	shellbar.addEventListener("ui5-coPilotClick", function(event) {
-		window["press-input"].value = "CoPilot"
+		window["press-input"].value = "CoPilot";
 	});
 
 	shellbar.addEventListener("ui5-menuItemClick", function(event) {
@@ -259,6 +261,16 @@ <h3>ShellBar in Compact</h3>
 		window["press-data"].value = item.getAttribute("data-key");
 	});
 
+	shelbarCompact.addEventListener("ui5-notificationsClick", function(event) {
+		event.preventDefault();
+		popover.openBy(event.detail.targetRef);
+	});
+
+	shelbarCompact.addEventListener("ui5-productSwitchClick", function(event) {
+		event.preventDefault();
+		popover.openBy(event.detail.targetRef);
+	});
+
 	menuItemsSB.addEventListener("ui5-menuItemClick", function(event) {
 		var item = event.detail.item;
 		window["result-value"].value = item.textContent;

diff --git a/packages/fiori/test/samples/NotificationListGroupItem.sample.html b/packages/fiori/test/samples/NotificationListGroupItem.sample.html
@@ -300,6 +300,7 @@ <h3>NotificationListGroupItem In ShellBar</h3>
 
 		<script>
 			shellbar.addEventListener("ui5-notificationsClick", function(event) {
+				event.preventDefault();
 				notificationsPopover.openBy(event.detail.targetRef);
 			});
 		</script>

diff --git a/packages/fiori/test/samples/NotificationListItem.sample.html b/packages/fiori/test/samples/NotificationListItem.sample.html
@@ -220,6 +220,7 @@ <h3>NotificationListItem In ShellBar</h3>
 
 		<script>
 			shellbar.addEventListener("ui5-notificationsClick", function(event) {
+				event.preventDefault();
 				notificationsPopover.openBy(event.detail.targetRef);
 			});
 		</script>

diff --git a/packages/fiori/test/samples/ProductSwitch.sample.html b/packages/fiori/test/samples/ProductSwitch.sample.html
@@ -67,6 +67,7 @@ <h3>ProductSwitch within ShellBar</h3>
 				if (popover.opened) {
 					popover.close();
 				} else {
+					event.preventDefault();
 					popover.openBy(event.detail.targetRef);
 				}
 			});

diff --git a/packages/fiori/test/samples/ShellBar.sample.html b/packages/fiori/test/samples/ShellBar.sample.html
@@ -9,7 +9,7 @@ <h2 class="control-header">ShellBar</h2>
 
 <div class="control-tag">&lt;ui5-shellbar&gt;</div>
 
-	<!-- ShellBar -->
+<!-- ShellBar -->
 <section>
 	<h3>ShellBar</h3>
 	<div class="snippet">

diff --git a/packages/fiori/test/specs/ShellBar.spec.js b/packages/fiori/test/specs/ShellBar.spec.js
@@ -231,7 +231,7 @@ describe("Component Behavior", () => {
 				assert.ok(menuPopover.isDisplayedInViewport(), "Menu should be shown");
 			});
 
-			it("tests notificationsPress event", () => {
+			it("tests notificationsClick event", () => {
 				const notificationsIcon = browser.$("#shellbar").shadow$(".ui5-shellbar-bell-button");
 				const input = browser.$("#press-input");
 
@@ -241,15 +241,15 @@ describe("Component Behavior", () => {
 				assert.strictEqual(input.getValue(), "Notifications", "Input value is set by click event of Notifications icon");
 			});
 
-			it("tests profilePress event", () => {
+			it("tests profileClick event", () => {
 				const profileIcon = browser.$("#shellbar").shadow$("[profile-btn]");
 				const input = browser.$("#press-input");
 
 				profileIcon.click();
 				assert.strictEqual(input.getValue(), "Profile", "Input value is set by click event of Profile");
 			});
 
-			it("tests productSwitchPress event", () => {
+			it("tests productSwitchClick event", () => {
 				const productSwitchIcon = browser.$("#shellbar").shadow$(".ui5-shellbar-button-product-switch");
 				const input = browser.$("#press-input");
 
@@ -331,7 +331,7 @@ describe("Component Behavior", () => {
 				assert.ok(menuPopover.isDisplayedInViewport(), "Menu should be shown");
 			});
 
-			it("tests notificationsPress event", () => {
+			it("tests notificationsClick event", () => {
 				const overflowButton = browser.$("#shellbar").shadow$(".ui5-shellbar-overflow-button");
 				const staticAreaItemClassName = browser.getStaticAreaItemClassName("#shellbar")
 				const overflowPopover = browser.$(`.${staticAreaItemClassName}`).shadow$(".ui5-shellbar-overflow-popover");
@@ -342,17 +342,18 @@ describe("Component Behavior", () => {
 				notificationListItem.click();
 
 				assert.strictEqual(input.getValue(), "Notifications", "Input value is set by click event of Notifications icon");
+				assert.ok(overflowPopover.isDisplayed(), "overflow popover should not be closed");
 			});
 
-			it("tests profilePress event", () => {
+			it("tests profileClick event", () => {
 				const profileIcon = browser.$("#shellbar").shadow$("[profile-btn]");
 				const input = browser.$("#press-input");
 
 				profileIcon.click();
 				assert.strictEqual(input.getValue(), "Profile", "Input value is set by click event of Profile");
 			});
 
-			it("tests productSwitchPress event", () => {
+			it("tests productSwitchClick event", () => {
 				const overflowButton = browser.$("#shellbar").shadow$(".ui5-shellbar-overflow-button");
 				const staticAreaItemClassName = browser.getStaticAreaItemClassName("#shellbar")
 				const overflowPopover = browser.$(`.${staticAreaItemClassName}`).shadow$(".ui5-shellbar-overflow-popover");
@@ -365,7 +366,7 @@ describe("Component Behavior", () => {
 				assert.strictEqual(input.getValue(), "Product Switch", "Input value is set by click event of Product Switch icon");
 			});
 
-			it("tests preventDefalt of productSwitchClick event", () => {
+			it("tests preventDefault of productSwitchClick event", () => {
 				const overflowButton = browser.$("#shellbar").shadow$(".ui5-shellbar-overflow-button");
 				const staticAreaItemClassName = browser.getStaticAreaItemClassName("#shellbar")
 				const overflowPopover = browser.$(`.${staticAreaItemClassName}`).shadow$(".ui5-shellbar-overflow-popover");