From 04b44b42e0e752efdc6e29f293bdc34a7267bee3 Mon Sep 17 00:00:00 2001
From: ilhan orhan <ilhan.myumyun@sap.com>
Date: Mon, 18 May 2020 10:08:47 +0300
Subject: [PATCH] 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.
---
 packages/fiori/src/ShellBar.js                |  8 ++++---
 .../test/pages/NotificationListGroupItem.html |  1 +
 .../test/pages/NotificationListItem.html      |  1 +
 packages/fiori/test/pages/ShellBar.html       | 22 ++++++++++++++-----
 .../NotificationListGroupItem.sample.html     |  1 +
 .../samples/NotificationListItem.sample.html  |  1 +
 .../test/samples/ProductSwitch.sample.html    |  1 +
 .../fiori/test/samples/ShellBar.sample.html   |  2 +-
 packages/fiori/test/specs/ShellBar.spec.js    | 15 +++++++------
 9 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/packages/fiori/src/ShellBar.js b/packages/fiori/src/ShellBar.js
index 2aaed191b657..23ea692c5857 100644
--- 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
index 430d6a7d3b9f..36d7945ca626 100644
--- 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
index 0604b9fb210a..2cb1f7eb20c7 100644
--- 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
index 3753cc4d1eab..800c3c04538e 100644
--- 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,16 +233,18 @@ <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) {
@@ -250,7 +252,7 @@ <h3>ShellBar in Compact</h3>
 	});
 
 	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
index 4903d1e76e7e..a6b351841eed 100644
--- 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
index fef18c7d9215..97135b8a3a52 100644
--- 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
index 188efceb2492..cfa0b8724e3f 100644
--- 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
index 1250d0729845..3a171946e962 100644
--- 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
index c38649a14189..9969e9afdee7 100644
--- 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,7 +241,7 @@ 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");
 
@@ -249,7 +249,7 @@ describe("Component Behavior", () => {
 				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,9 +342,10 @@ 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");
 
@@ -352,7 +353,7 @@ describe("Component Behavior", () => {
 				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");