fix(ui5-popover): fix closing order of popovers (#1676)

Author: fifoosid

packages/main/src/Popover.js

@@ -278,7 +278,7 @@ class Popover extends Popup {
 	 * Closes the popover.
 	 * @public
 	 */
-	close(escPressed = false, preventRegitryUpdate = false, preventFocusRestore = false) {
+	close(escPressed = false, preventRegistryUpdate = false, preventFocusRestore = false) {
 		if (!this.opened) {
 			return;
 		}
@@ -296,7 +296,7 @@ class Popover extends Popup {
 
 		this.opened = false;
 
-		if (!preventRegitryUpdate) {
+		if (!preventRegistryUpdate) {
 			removeOpenedPopover(this);
 		}
 

packages/main/src/popup-utils/OpenedPopupsRegistry.js

@@ -1,36 +1,37 @@
 import { isEscape } from "@ui5/webcomponents-base/dist/Keys.js";
 
-let registry = [];
-
-const addOpenedPopup = instance => {
-	if (!registry.includes(instance)) {
-		registry.push(instance);
+let openedRegistry = [];
+
+const addOpenedPopup = (instance, parentPopovers = []) => {
+	if (!openedRegistry.includes(instance)) {
+		openedRegistry.push({
+			instance,
+			parentPopovers,
+		});
 	}
 
-	if (registry.length === 1) {
+	if (openedRegistry.length === 1) {
 		attachGlobalListener();
 	}
 };
 
 const removeOpenedPopup = instance => {
-	registry = registry.filter(el => {
-		return el !== instance;
+	openedRegistry = openedRegistry.filter(el => {
+		return el !== instance.instance;
 	});
 
-	if (!registry.length) {
+	if (!openedRegistry.length) {
 		detachGlobalListener();
 	}
 };
 
 const getOpenedPopups = () => {
-	return [...registry];
+	return [...openedRegistry];
 };
 
 const _keydownListener = event => {
 	if (isEscape(event)) {
-		const topPopup = registry[registry.length - 1];
-
-		topPopup && topPopup.close(true);
+		openedRegistry.pop().instance.close(true);
 	}
 };
 

packages/main/src/popup-utils/PopoverRegistry.js

@@ -1,13 +1,14 @@
 import { isClickInRect } from "./PopupUtils.js";
 import { getOpenedPopups, addOpenedPopup, removeOpenedPopup } from "./OpenedPopupsRegistry.js";
 
+
 let updateInterval = null;
 const intervalTimeout = 300;
 const openedRegistry = [];
 
 const repositionPopovers = event => {
 	openedRegistry.forEach(popover => {
-		popover.reposition();
+		popover.instance.reposition();
 	});
 };
 
@@ -39,15 +40,15 @@ const detachGlobalClickHandler = () => {
 
 const clickHandler = event => {
 	const openedPopups = getOpenedPopups();
-	const isTopPopupPopover = openedPopups[openedPopups.length - 1].openBy;
+	const isTopPopupPopover = openedPopups[openedPopups.length - 1].instance.openBy;
 
 	if (openedPopups.length === 0 || !isTopPopupPopover) {
 		return;
 	}
 
 	// loop all open popovers
 	for (let i = (openedPopups.length - 1); i !== -1; i--) {
-		const popup = openedPopups[i];
+		const popup = openedPopups[i].instance;
 
 		// if popup is modal, opener is clicked, popup is dialog skip closing
 		if (popup.isModal || popup.isOpenerClicked(event)) {
@@ -71,8 +72,13 @@ const detachScrollHandler = popover => {
 };
 
 const addOpenedPopover = instance => {
-	addOpenedPopup(instance);
-	openedRegistry.push(instance);
+	const parentPopovers = getParentPopoversIfNested(instance);
+
+	addOpenedPopup(instance, parentPopovers);
+	openedRegistry.push({
+		instance,
+		parentPopovers,
+	});
 
 	attachScrollHandler(instance);
 
@@ -84,18 +90,31 @@ const addOpenedPopover = instance => {
 };
 
 const removeOpenedPopover = instance => {
-	let count = 0;
-
+	const popoversToClose = [instance];
 
-	for (let i = openedRegistry.indexOf(instance); i < openedRegistry.length; i++) {
-		openedRegistry[i].close(false, true);
-		removeOpenedPopup(openedRegistry[i]);
-		detachScrollHandler(openedRegistry[i]);
-		count++;
+	for (let i = 0; i < openedRegistry.length; i++) {
+		const indexOfCurrentInstance = openedRegistry[i].parentPopovers.indexOf(instance);
+		if (openedRegistry[i].parentPopovers.length > 0 && indexOfCurrentInstance > -1) {
+			popoversToClose.push(openedRegistry[i].instance);
+		}
 	}
 
-	// remove top popovers from registry
-	Array(count).fill().forEach(() => { openedRegistry.pop(); });
+	for (let i = popoversToClose.length - 1; i >= 0; i--) {
+		for (let j = 0; j < openedRegistry.length; j++) {
+			let indexOfItemToRemove;
+			if (popoversToClose[i] === openedRegistry[j].instance) {
+				indexOfItemToRemove = j;
+			}
+
+
+			if (indexOfItemToRemove >= 0) {
+				removeOpenedPopup(openedRegistry[indexOfItemToRemove].instance);
+				detachScrollHandler(openedRegistry[indexOfItemToRemove].instance);
+				const itemToClose = openedRegistry.splice(indexOfItemToRemove, 1);
+				itemToClose[0].instance.close(false, true);
+			}
+		}
+	}
 
 	if (!openedRegistry.length) {
 		detachGlobalScrollHandler();
@@ -108,4 +127,21 @@ const getRegistry = () => {
 	return openedRegistry;
 };
 
+const getParentPopoversIfNested = instance => {
+	let currentElement = instance.parentNode;
+	const parentPopovers = [];
+
+	while (currentElement.parentNode) {
+		for (let i = 0; i < openedRegistry.length; i++) {
+			if (currentElement && currentElement === openedRegistry[i].instance) {
+				parentPopovers.push(currentElement);
+			}
+		}
+
+		currentElement = currentElement.parentNode;
+	}
+
+	return parentPopovers;
+};
+
 export { addOpenedPopover, removeOpenedPopover, getRegistry };

packages/main/test/pages/Popover.html

@@ -74,9 +74,9 @@
 		<br>
 		<br>
 
-		<ui5-button id='popbtn'>Open Popover</ui5-button>
+		<ui5-button id="popbtn">Open Popover</ui5-button>
 
-		<ui5-popover placement-type="Bottom" no-arrow id='danger'>
+		<ui5-popover placement-type="Bottom" no-arrow id="danger">
 
 
 			<ui5-list>
@@ -88,9 +88,9 @@
 			<br>
 			<br>
 
-			<ui5-button id='bigDanger'>Super Danger !</ui5-button>
+			<ui5-button id="bigDanger">Super Danger !</ui5-button>
 
-			<ui5-popover id='bigDangerPop'>
+			<ui5-popover id="bigDangerPop">
 				<ui5-list>
 					<ui5-li>Hello</ui5-li>
 					<ui5-li>World</ui5-li>
@@ -293,27 +293,27 @@
 	</ui5-popover>
 
 	<script>
-		btn.addEventListener('click', function (event) {
+		btn.addEventListener("click", function (event) {
 			pop.openBy(btn);
 		});
 
-		popbtn.addEventListener('click', function (event) {
+		popbtn.addEventListener("click", function (event) {
 			danger.openBy(popbtn);
 		});
 
-		bigDanger.addEventListener('click', function (event) {
+		bigDanger.addEventListener("click", function (event) {
 			bigDangerPop.openBy(bigDanger);
 		});
 
-		btnPopModal.addEventListener('click', function (event) {
+		btnPopModal.addEventListener("click", function (event) {
 			modalPopover.openBy(btnPopModal);
 		});
 
-		modalPopoverClose.addEventListener('click', function (event) {
+		modalPopoverClose.addEventListener("click", function (event) {
 			modalPopover.close();
 		});
 
-		btnPopFocus.addEventListener('click', function (event) {
+		btnPopFocus.addEventListener("click", function (event) {
 			popFocus.openBy(btnPopFocus);
 		});
 
@@ -329,30 +329,30 @@
 			document.getElementById("big-popover").openBy(event.target);
 		});
 
-		btnFullWidth.addEventListener('click', function (event) {
+		btnFullWidth.addEventListener("click", function (event) {
 			popFullWidth.openBy(btnFullWidth);
 		});
 
-		btnFullWidthTargetOverlap.addEventListener('click', function (event) {
+		btnFullWidthTargetOverlap.addEventListener("click", function (event) {
 			popFullWidthTargetOverlap.openBy(btnFullWidthTargetOverlap);
 		});
 
-		btnFullWidthLeft.addEventListener('click', function (event) {
+		btnFullWidthLeft.addEventListener("click", function (event) {
 			popFullWidthLeft.openBy(btnFullWidthLeft);
 		});
 
-		btnFullWidthLeftTargetOverlap.addEventListener('click', function (event) {
+		btnFullWidthLeftTargetOverlap.addEventListener("click", function (event) {
 			popFullWidthLeftTargetOverlap.openBy(btnFullWidthLeftTargetOverlap);
 		});
 
-		btnFullWidthTop.addEventListener('click', function (event) {
+		btnFullWidthTop.addEventListener("click", function (event) {
 			popFullWidthTop.openBy(btnFullWidthTop);
 		});
 
-		btnFullWidthBottom.addEventListener('click', function (event) {
+		btnFullWidthBottom.addEventListener("click", function (event) {
 			popFullWidthBottom.openBy(btnFullWidthBottom);
 		});
 	</script>
 </body>
 
-</html>
+</html>