fix(ui5-popover): allow setting opener to an abstract element DOM r…

…eference (#10419)

When Popover's opener is set as a DOM reference and if happens to be a UI5 Element, we need to return its DOM (focus) reference. As of now we just return the element itself. While this works for most of the cases, it does not work for abstract elements as the popover won't be able to calculate its position due to not having a physically DOM element as opener.

The case when the opener is set as string, ID, pointing to an element has been already handled. For this case we already call getFocusDomRef. And, this change addresses the other case - setting the opener as DOM reference.

packages/main/cypress/specs/Popover.cy.ts

@@ -0,0 +1,62 @@
+import { html } from "lit";
+import "../../src/Button.js";
+import "../../src/Toolbar.js";
+import "../../src/ToolbarButton.js";
+import "../../src/Popover.js";
+
+describe("Popover opener", () => {
+	it("tests 'opener' set as string of abstract element's ID ", () => {
+		cy.mount(html`
+			<ui5-toolbar id="tb">
+				<ui5-toolbar-button text="Add"></ui5-toolbar-button>
+				<ui5-toolbar-button text="Delete" id="clearCounter"></ui5-toolbar-button>
+				<ui5-toolbar-button id="btnOpenMenu" text="Open Menu" prevent-overflow-closing></ui5-toolbar-button>
+			</ui5-toolbar>
+			<ui5-popover id="popup" opener="btnOpenMenu">
+				<ui5-button id="btnClosePopover">Close</ui5-button>
+			</ui5-popover>
+		`);
+
+		// act
+		cy.get("#popup").invoke("prop", "open", "true");
+
+		// assert - The button inside the popover is accessible => popover is opened properly.
+		cy.get("#btnClosePopover").then($btnClosePopover => {
+			$btnClosePopover.get(0).addEventListener("click", () => {
+				cy.get("#popup").invoke("prop", "open", false);
+			});
+		});
+		cy.get("#btnClosePopover").realClick();
+	});
+
+	it("tests 'opener' set as DOM ref of abstract element's DOM reference", () => {
+		cy.mount(html`
+			<ui5-toolbar id="tb">
+				<ui5-toolbar-button text="Add"></ui5-toolbar-button>
+				<ui5-toolbar-button text="Delete"></ui5-toolbar-button>
+				<ui5-toolbar-button id="btnOpenPopover" text="Open Menu" prevent-overflow-closing></ui5-toolbar-button>
+			</ui5-toolbar>
+			<ui5-popover id="popup">
+				<ui5-button id="btnClosePopover">Close</ui5-button>
+			</ui5-popover>
+		`);
+
+		cy.get("#btnOpenPopover").then($toolbarBtn => {
+			cy.wrap($toolbarBtn.get(0)).as("toolbarBtn");
+		});
+
+		// act
+		cy.get("@toolbarBtn").then($toolbarBtn => {
+			cy.get("#popup").invoke("prop", "opener", $toolbarBtn.get(0));
+		});
+		cy.get("#popup").invoke("prop", "open", true);
+
+		// assert - The button inside the popover is accessible => popover is opened properly.
+		cy.get("#btnClosePopover").then($btnClosePopover => {
+			$btnClosePopover.get(0).addEventListener("click", () => {
+				cy.get("#popup").invoke("prop", "open", false);
+			});
+		});
+		cy.get("#btnClosePopover").realClick();
+	});
+});

packages/main/cypress/specs/Toolbar.cy.ts

@@ -46,27 +46,6 @@ describe("Toolbar general interaction", () => {
 			});
 	});
 
-	it("should be able to use toolbar button as popover opener", () => {
-		cy.mount(html`
-			<ui5-toolbar id="tb">
-				<ui5-toolbar-button icon="add" text="Left 1 (long)" id="clickCounter"></ui5-toolbar-button>
-				<ui5-toolbar-button icon="decline" text="Left 2" id="clearCounter"></ui5-toolbar-button>
-				<ui5-toolbar-button icon="employee" text="Left 3"></ui5-toolbar-button>
-				<ui5-toolbar-separator></ui5-toolbar-separator>
-				<ui5-toolbar-button icon="decline" text="Left 4"></ui5-toolbar-button>
-				<ui5-toolbar-button id="btnOpenMenu" text="Open Menu" prevent-overflow-closing></ui5-toolbar-button>
-			</ui5-toolbar>
-			<ui5-popover id="popup" opener="btnOpenMenu">
-				Hello World
-			</ui5-popover>
-		`);
-
-		cy.get("#popup").invoke("prop", "open", "true");
-
-		cy.get("#popup")
-			.should("be.visible");
-	});
-
 	it("shouldn't have toolbar button as popover opener when there is spacer before last toolbar item", () => {
 		cy.mount(html`
 			<ui5-toolbar id="otb_spacer">

packages/main/src/Popover.ts

@@ -295,10 +295,14 @@ class Popover extends Popup {
 	}
 
 	getOpenerHTMLElement(opener: HTMLElement | string | undefined): HTMLElement | null | undefined {
-		if (opener === undefined || opener instanceof HTMLElement) {
+		if (opener === undefined) {
 			return opener;
 		}
 
+		if (opener instanceof HTMLElement) {
+			return this._isUI5Element(opener) ? opener.getFocusDomRef() : opener;
+		}
+
 		let rootNode = this.getRootNode();
 
 		if (rootNode === this) {

packages/main/test/pages/MultiComboBox.html

@@ -233,6 +233,7 @@
 			<ui5-mcb-item text="est"></ui5-mcb-item>
 		</ui5-multi-combobox>
 
+		<ui5-button id="btnAfter">Press</ui5-button>
 		<br>
 
 		<div class="demo-section">

packages/main/test/pages/Toolbar.html

@@ -34,6 +34,7 @@
 			<ui5-toolbar-separator></ui5-toolbar-separator>
 			<ui5-toolbar-button icon="decline" text="Left 4"></ui5-toolbar-button>
 			<ui5-toolbar-button id="btnOpenMenu" text="Open Menu" prevent-overflow-closing></ui5-toolbar-button>
+			<ui5-toolbar-button id="btnOpenMenu2" text="Open Menu 2" prevent-overflow-closing></ui5-toolbar-button>
 		</ui5-toolbar>
 		<ui5-menu header-text="Basic Menu with Items" id="menu" opener="btnOpenMenu">
 			<ui5-menu-item text="New File" icon="add-document"></ui5-menu-item>
@@ -45,7 +46,14 @@
 			<ui5-menu-item text="Preferences" icon="action-settings"></ui5-menu-item>
 			<ui5-menu-item text="Exit" icon="journey-arrive"></ui5-menu-item>
 		</ui5-menu>
-
+		<ui5-menu header-text="Basic Menu 2 with Items" id="menu2">
+			<ui5-menu-item text="New File" icon="add-document"></ui5-menu-item>
+			<ui5-menu-item text="New Window" icon="add-folder" disabled></ui5-menu-item>
+			<ui5-menu-item text="Open.." icon="open-folder"></ui5-menu-item>
+			<ui5-menu-item text="Open Folder" icon="open-folder"></ui5-menu-item>
+			<ui5-menu-item text="Save" icon="save"></ui5-menu-item>
+		</ui5-menu>
+
 		<ui5-input id="input" value="0"></ui5-input>
 
 		<br /><br />
@@ -374,11 +382,20 @@
 		})
 
 		const btnOpenMenu = document.getElementById("btnOpenMenu");
+		const btnOpenMenu2 = document.getElementById("btnOpenMenu2");
 		const menu = document.getElementById("menu");
+		const menu2 = document.getElementById("menu2");
 
-		btnOpenMenu.addEventListener("ui5-click", function (event) {
+		btnOpenMenu.addEventListener("ui5-click", function(event) {
+			// opener is set as string (the ID of the ui5-toolbar-button#btnOpenMenu)
 			menu.open = !menu.open;
 		});
+
+		btnOpenMenu2.addEventListener("ui5-click", function(event) {
+			// opener is set as property (the DOM reference of the ui5-toolbar-button#btnOpenMenu2)
+			menu2.open = !menu2.open;
+			menu2.opener = btnOpenMenu2;
+		});
 	</script>
 </body>
 

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

@@ -120,8 +120,8 @@ describe("MultiComboBox general interaction", () => {
 			const mcb = await browser.$("#mcb-select-all-vs");
 			const arrow = await mcb.shadow$(".inputIcon");
 			const tokenizer = await mcb.shadow$("ui5-tokenizer");
-			const body = await browser.$(".multicombobox1auto");
 			const nMoreText = await tokenizer.shadow$(".ui5-tokenizer-more-text");
+			const btnAfter = await browser.$("#btnAfter");
 			const firstItemCheckbox = await mcb.$("ui5-mcb-item").shadow$("ui5-checkbox");
 
 			await arrow.click();
@@ -130,15 +130,17 @@ describe("MultiComboBox general interaction", () => {
 
 			// select all items
 			await browser.keys("Space");
-
-			assert.ok(await tokenizer.getProperty("expanded"), "The tokenizer is expanded");
-
-			await body.click();
+			assert.strictEqual(await tokenizer.getProperty("expanded"), true, "The tokenizer is expanded - all items are selected");
+
+			await btnAfter.click();
+			assert.strictEqual(await tokenizer.getProperty("expanded"), false, "The tokenizer is collapsed - pressing outside");
+
 			await nMoreText.click();
-			await firstItemCheckbox.click();
-			await body.click();
+			assert.strictEqual(await tokenizer.getProperty("expanded"), true, "The tokenizer is expanded - n-more clicked");
 
-			assert.notOk(await tokenizer.getProperty("expanded"), "The tokenizer is collapsed");
+			await firstItemCheckbox.click();
+			await btnAfter.click();
+			assert.strictEqual(await tokenizer.getProperty("expanded"), false, "The tokenizer is collapsed - pressing outside");
 		});
 	});