From 92748d6218d842c8ec0e496bac6392704c952e9f Mon Sep 17 00:00:00 2001 From: Teodor Taushanov Date: Wed, 16 Jul 2025 13:16:38 +0300 Subject: [PATCH 1/7] chore: improve test page --- packages/base/src/features/patchPopup.ts | 3 +-- .../main/test/pages/DialogAndOpenUI5Dialog.html | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/base/src/features/patchPopup.ts b/packages/base/src/features/patchPopup.ts index 616365a705e0..d98176ca9c75 100644 --- a/packages/base/src/features/patchPopup.ts +++ b/packages/base/src/features/patchPopup.ts @@ -70,9 +70,8 @@ const patchClosed = (Popup: OpenUI5Popup) => { const patchFocusEvent = (Popup: OpenUI5Popup) => { const origFocusEvent = Popup.prototype.onFocusEvent; Popup.prototype.onFocusEvent = function onFocusEvent(e: FocusEvent) { - const isTypeFocus = e.type === "focus" || e.type === "activate"; const target = e.target as HTMLElement; - if (!isTypeFocus || !target.closest("[ui5-popover],[ui5-responsive-popover],[ui5-dialog]")) { + if (!target.closest("[ui5-popover],[ui5-responsive-popover],[ui5-dialog]")) { origFocusEvent.call(this, e); } }; diff --git a/packages/main/test/pages/DialogAndOpenUI5Dialog.html b/packages/main/test/pages/DialogAndOpenUI5Dialog.html index bbde81aec47d..eb2d50f11290 100644 --- a/packages/main/test/pages/DialogAndOpenUI5Dialog.html +++ b/packages/main/test/pages/DialogAndOpenUI5Dialog.html @@ -52,6 +52,19 @@ document.getElementById("dialog1").open = true; }); + sap.ui.require(["sap/m/Select", "sap/ui/core/Item"], (Select, Item) => { + new Select({ + items: [ + new Item({ text: "Item 1" }), + new Item({ text: "Item 2" }), + new Item({ text: "Item 3" }) + ], + change: function (oEvent) { + console.error("Selected item:", oEvent.getParameter("selectedItem").getText()); + } + }).placeAt("dilog1content"); + }); + document.getElementById("dialogButton").addEventListener("click", function () { sap.ui.require(["sap/m/Button", "sap/m/Dialog"], (Button, Dialog) => { new Dialog({ @@ -81,6 +94,7 @@ Open WebC Dialog +
Open UI5 dialog From de0ccd3161d6b1ede53f64194620abef045a53dd Mon Sep 17 00:00:00 2001 From: Teodor Taushanov Date: Thu, 17 Jul 2025 15:45:11 +0300 Subject: [PATCH 2/7] fix(OpenUI5Support): fix sap.m.Select inside a ui5-dialog --- packages/base/src/features/OpenUI5Support.ts | 14 +++++++++- packages/base/src/features/patchPopup.ts | 26 ++++++++++++++++--- packages/main/src/Popup.ts | 9 +++++++ .../src/popup-utils/OpenedPopupsRegistry.ts | 15 +++++++++++ 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/packages/base/src/features/OpenUI5Support.ts b/packages/base/src/features/OpenUI5Support.ts index 37970bf052a8..efff4f7faea0 100644 --- a/packages/base/src/features/OpenUI5Support.ts +++ b/packages/base/src/features/OpenUI5Support.ts @@ -1,6 +1,6 @@ import patchPatcher from "./patchPatcher.js"; import type { OpenUI5Patcher } from "./patchPatcher.js"; -import patchPopup from "./patchPopup.js"; +import { patchPopup, addOpenedPopup, removeOpenedPopup, getTopMostPopup } from "./patchPopup.js"; import type { OpenUI5Popup } from "./patchPopup.js"; import { registerFeature } from "../FeaturesRegistry.js"; import { setTheme } from "../config/Theme.js"; @@ -223,6 +223,18 @@ class OpenUI5Support { return !!link.href.match(/\/css(-|_)variables\.css/); } + + static addOpenedPopup(popup: object) { + addOpenedPopup(popup); + } + + static removeOpenedPopup(popup: object) { + removeOpenedPopup(popup); + } + + static getTopMostPopup() { + return getTopMostPopup(); + } } registerFeature("OpenUI5Support", OpenUI5Support); diff --git a/packages/base/src/features/patchPopup.ts b/packages/base/src/features/patchPopup.ts index d98176ca9c75..5599a9846f2a 100644 --- a/packages/base/src/features/patchPopup.ts +++ b/packages/base/src/features/patchPopup.ts @@ -1,4 +1,6 @@ // OpenUI5's Control.js subset +import getSharedResource from "../getSharedResource.js"; + type Control = { getDomRef: () => HTMLElement | null, } @@ -14,15 +16,34 @@ type OpenUI5Popup = { } }; +const OpenedPopupsRegistry = getSharedResource<{ openedRegistry: Array }>("OpenedAnyPopupsRegistry", { openedRegistry: [] }); + +const addOpenedPopup = (popup: object) => { + OpenedPopupsRegistry.openedRegistry.push(popup); +}; + +const removeOpenedPopup = (popup: object) => { + const index = OpenedPopupsRegistry.openedRegistry.indexOf(popup); + if (index > -1) { + OpenedPopupsRegistry.openedRegistry.splice(index, 1); + } +}; + +const getTopMostPopup = () => { + return OpenedPopupsRegistry.openedRegistry[OpenedPopupsRegistry.openedRegistry.length - 1]; +}; + const openNativePopover = (domRef: HTMLElement) => { domRef.setAttribute("popover", "manual"); domRef.showPopover(); + addOpenedPopup(domRef); }; const closeNativePopover = (domRef: HTMLElement) => { if (domRef.hasAttribute("popover")) { domRef.hidePopover(); domRef.removeAttribute("popover"); + removeOpenedPopup(domRef); } }; @@ -70,8 +91,7 @@ const patchClosed = (Popup: OpenUI5Popup) => { const patchFocusEvent = (Popup: OpenUI5Popup) => { const origFocusEvent = Popup.prototype.onFocusEvent; Popup.prototype.onFocusEvent = function onFocusEvent(e: FocusEvent) { - const target = e.target as HTMLElement; - if (!target.closest("[ui5-popover],[ui5-responsive-popover],[ui5-dialog]")) { + if ((this.getContent() as any).getDomRef?.() === getTopMostPopup()) { origFocusEvent.call(this, e); } }; @@ -90,5 +110,5 @@ const patchPopup = (Popup: OpenUI5Popup) => { patchFocusEvent(Popup);// Popup.prototype.onFocusEvent }; -export default patchPopup; +export { patchPopup, addOpenedPopup, removeOpenedPopup, getTopMostPopup }; export type { OpenUI5Popup }; diff --git a/packages/main/src/Popup.ts b/packages/main/src/Popup.ts index 9cfe7d9890bf..9ef06eb678c4 100644 --- a/packages/main/src/Popup.ts +++ b/packages/main/src/Popup.ts @@ -29,6 +29,8 @@ import toLowercaseEnumValue from "@ui5/webcomponents-base/dist/util/toLowercaseE import PopupTemplate from "./PopupTemplate.js"; import PopupAccessibleRole from "./types/PopupAccessibleRole.js"; import { addOpenedPopup, removeOpenedPopup } from "./popup-utils/OpenedPopupsRegistry.js"; +import { getFeature } from "@ui5/webcomponents-base/dist/FeaturesRegistry.js"; +import type OpenUI5Support from "@ui5/webcomponents-base/dist/features/OpenUI5Support.js"; // Styles import popupStlyes from "./generated/themes/Popup.css.js"; @@ -548,6 +550,13 @@ abstract class Popup extends UI5Element { return; } + if (escPressed) { + const openUI5Support = getFeature("OpenUI5Support"); + if (openUI5Support && openUI5Support.getTopMostPopup() !== this) { + return; + } + } + const prevented = !this.fireDecoratorEvent("before-close", { escPressed }); if (prevented) { this.open = true; diff --git a/packages/main/src/popup-utils/OpenedPopupsRegistry.ts b/packages/main/src/popup-utils/OpenedPopupsRegistry.ts index 505f1d54a1c6..27078e91480a 100644 --- a/packages/main/src/popup-utils/OpenedPopupsRegistry.ts +++ b/packages/main/src/popup-utils/OpenedPopupsRegistry.ts @@ -1,5 +1,7 @@ import getSharedResource from "@ui5/webcomponents-base/dist/getSharedResource.js"; import { isEscape } from "@ui5/webcomponents-base/dist/Keys.js"; +import { getFeature } from "@ui5/webcomponents-base/dist/FeaturesRegistry.js"; +import type OpenUI5Support from "@ui5/webcomponents-base/dist/features/OpenUI5Support.js"; import type Popup from "../Popup.js"; type RegisteredPopup = { @@ -8,6 +10,15 @@ type RegisteredPopup = { } const OpenedPopupsRegistry = getSharedResource<{ openedRegistry: Array }>("OpenedPopupsRegistry", { openedRegistry: [] }); +const openUI5Support = getFeature("OpenUI5Support"); + +function addAnyPopup(popup: object) { + openUI5Support?.addOpenedPopup(popup); +} + +function removeAnyPopup(popup: object) { + openUI5Support?.removeOpenedPopup(popup); +} const addOpenedPopup = (instance: Popup, parentPopovers: Array = []) => { if (!OpenedPopupsRegistry.openedRegistry.some(popup => popup.instance === instance)) { @@ -15,6 +26,8 @@ const addOpenedPopup = (instance: Popup, parentPopovers: Array = []) => { instance, parentPopovers, }); + + addAnyPopup(instance); } _updateTopModalPopup(); @@ -29,6 +42,8 @@ const removeOpenedPopup = (instance: Popup) => { return el.instance !== instance; }); + removeAnyPopup(instance); + _updateTopModalPopup(); if (!OpenedPopupsRegistry.openedRegistry.length) { From 6d1d541b498f21b385569bde52667dea06a6bba7 Mon Sep 17 00:00:00 2001 From: Teodor Taushanov Date: Thu, 17 Jul 2025 16:28:15 +0300 Subject: [PATCH 3/7] fix: fix lint errors --- packages/base/src/features/OpenUI5Support.ts | 7 ++++++- packages/base/src/features/patchPopup.ts | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/base/src/features/OpenUI5Support.ts b/packages/base/src/features/OpenUI5Support.ts index efff4f7faea0..9af13d3073dc 100644 --- a/packages/base/src/features/OpenUI5Support.ts +++ b/packages/base/src/features/OpenUI5Support.ts @@ -1,6 +1,11 @@ import patchPatcher from "./patchPatcher.js"; import type { OpenUI5Patcher } from "./patchPatcher.js"; -import { patchPopup, addOpenedPopup, removeOpenedPopup, getTopMostPopup } from "./patchPopup.js"; +import { + patchPopup, + addOpenedPopup, + removeOpenedPopup, + getTopMostPopup, +} from "./patchPopup.js"; import type { OpenUI5Popup } from "./patchPopup.js"; import { registerFeature } from "../FeaturesRegistry.js"; import { setTheme } from "../config/Theme.js"; diff --git a/packages/base/src/features/patchPopup.ts b/packages/base/src/features/patchPopup.ts index 5599a9846f2a..38bf6849df80 100644 --- a/packages/base/src/features/patchPopup.ts +++ b/packages/base/src/features/patchPopup.ts @@ -110,5 +110,11 @@ const patchPopup = (Popup: OpenUI5Popup) => { patchFocusEvent(Popup);// Popup.prototype.onFocusEvent }; -export { patchPopup, addOpenedPopup, removeOpenedPopup, getTopMostPopup }; +export { + patchPopup, + addOpenedPopup, + removeOpenedPopup, + getTopMostPopup, +}; + export type { OpenUI5Popup }; From d5ff5f76ea798f2edcef2d98003cdc1c9afd3a61 Mon Sep 17 00:00:00 2001 From: Teodor Taushanov Date: Fri, 18 Jul 2025 10:23:07 +0300 Subject: [PATCH 4/7] chore: simplifying code --- packages/base/src/features/patchPopup.ts | 8 +++++--- packages/main/src/Popup.ts | 9 --------- packages/main/src/popup-utils/OpenedPopupsRegistry.ts | 8 +++++++- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/base/src/features/patchPopup.ts b/packages/base/src/features/patchPopup.ts index 38bf6849df80..e50bebc529d7 100644 --- a/packages/base/src/features/patchPopup.ts +++ b/packages/base/src/features/patchPopup.ts @@ -36,14 +36,12 @@ const getTopMostPopup = () => { const openNativePopover = (domRef: HTMLElement) => { domRef.setAttribute("popover", "manual"); domRef.showPopover(); - addOpenedPopup(domRef); }; const closeNativePopover = (domRef: HTMLElement) => { if (domRef.hasAttribute("popover")) { domRef.hidePopover(); domRef.removeAttribute("popover"); - removeOpenedPopup(domRef); } }; @@ -73,6 +71,8 @@ const patchOpen = (Popup: OpenUI5Popup) => { } } } + + addOpenedPopup(this); }; }; @@ -85,13 +85,15 @@ const patchClosed = (Popup: OpenUI5Popup) => { if (domRef) { closeNativePopover(domRef); // unset the popover attribute and close the native popover, but only if still in DOM } + + removeOpenedPopup(this); }; }; const patchFocusEvent = (Popup: OpenUI5Popup) => { const origFocusEvent = Popup.prototype.onFocusEvent; Popup.prototype.onFocusEvent = function onFocusEvent(e: FocusEvent) { - if ((this.getContent() as any).getDomRef?.() === getTopMostPopup()) { + if (this === getTopMostPopup()) { origFocusEvent.call(this, e); } }; diff --git a/packages/main/src/Popup.ts b/packages/main/src/Popup.ts index 9ef06eb678c4..9cfe7d9890bf 100644 --- a/packages/main/src/Popup.ts +++ b/packages/main/src/Popup.ts @@ -29,8 +29,6 @@ import toLowercaseEnumValue from "@ui5/webcomponents-base/dist/util/toLowercaseE import PopupTemplate from "./PopupTemplate.js"; import PopupAccessibleRole from "./types/PopupAccessibleRole.js"; import { addOpenedPopup, removeOpenedPopup } from "./popup-utils/OpenedPopupsRegistry.js"; -import { getFeature } from "@ui5/webcomponents-base/dist/FeaturesRegistry.js"; -import type OpenUI5Support from "@ui5/webcomponents-base/dist/features/OpenUI5Support.js"; // Styles import popupStlyes from "./generated/themes/Popup.css.js"; @@ -550,13 +548,6 @@ abstract class Popup extends UI5Element { return; } - if (escPressed) { - const openUI5Support = getFeature("OpenUI5Support"); - if (openUI5Support && openUI5Support.getTopMostPopup() !== this) { - return; - } - } - const prevented = !this.fireDecoratorEvent("before-close", { escPressed }); if (prevented) { this.open = true; diff --git a/packages/main/src/popup-utils/OpenedPopupsRegistry.ts b/packages/main/src/popup-utils/OpenedPopupsRegistry.ts index 27078e91480a..9ec3bcccd61a 100644 --- a/packages/main/src/popup-utils/OpenedPopupsRegistry.ts +++ b/packages/main/src/popup-utils/OpenedPopupsRegistry.ts @@ -61,8 +61,14 @@ const _keydownListener = (event: KeyboardEvent) => { } if (isEscape(event)) { + const topPopup = OpenedPopupsRegistry.openedRegistry[OpenedPopupsRegistry.openedRegistry.length - 1].instance; + + if (openUI5Support && topPopup !== openUI5Support.getTopMostPopup()) { + return; + } + event.stopPropagation(); - OpenedPopupsRegistry.openedRegistry[OpenedPopupsRegistry.openedRegistry.length - 1].instance.closePopup(true); + topPopup.closePopup(true); } }; From 99c64cb2f8fe70bbff1278702ac8b424bd821770 Mon Sep 17 00:00:00 2001 From: Teodor Taushanov Date: Fri, 18 Jul 2025 11:02:14 +0300 Subject: [PATCH 5/7] chore: rename methods --- packages/base/src/features/patchPopup.ts | 11 ++++++----- packages/main/src/popup-utils/OpenedPopupsRegistry.ts | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/base/src/features/patchPopup.ts b/packages/base/src/features/patchPopup.ts index e50bebc529d7..748c37eea43b 100644 --- a/packages/base/src/features/patchPopup.ts +++ b/packages/base/src/features/patchPopup.ts @@ -16,21 +16,22 @@ type OpenUI5Popup = { } }; -const OpenedPopupsRegistry = getSharedResource<{ openedRegistry: Array }>("OpenedAnyPopupsRegistry", { openedRegistry: [] }); +// contains all OpenUI5 and Web Component popups that are currently opened +const AllOpenedPopupsRegistry = getSharedResource<{ openedRegistry: Array }>("AllOpenedPopupsRegistry", { openedRegistry: [] }); const addOpenedPopup = (popup: object) => { - OpenedPopupsRegistry.openedRegistry.push(popup); + AllOpenedPopupsRegistry.openedRegistry.push(popup); }; const removeOpenedPopup = (popup: object) => { - const index = OpenedPopupsRegistry.openedRegistry.indexOf(popup); + const index = AllOpenedPopupsRegistry.openedRegistry.indexOf(popup); if (index > -1) { - OpenedPopupsRegistry.openedRegistry.splice(index, 1); + AllOpenedPopupsRegistry.openedRegistry.splice(index, 1); } }; const getTopMostPopup = () => { - return OpenedPopupsRegistry.openedRegistry[OpenedPopupsRegistry.openedRegistry.length - 1]; + return AllOpenedPopupsRegistry.openedRegistry[AllOpenedPopupsRegistry.openedRegistry.length - 1]; }; const openNativePopover = (domRef: HTMLElement) => { diff --git a/packages/main/src/popup-utils/OpenedPopupsRegistry.ts b/packages/main/src/popup-utils/OpenedPopupsRegistry.ts index 9ec3bcccd61a..315b0b7a8756 100644 --- a/packages/main/src/popup-utils/OpenedPopupsRegistry.ts +++ b/packages/main/src/popup-utils/OpenedPopupsRegistry.ts @@ -12,11 +12,11 @@ type RegisteredPopup = { const OpenedPopupsRegistry = getSharedResource<{ openedRegistry: Array }>("OpenedPopupsRegistry", { openedRegistry: [] }); const openUI5Support = getFeature("OpenUI5Support"); -function addAnyPopup(popup: object) { +function registerPopupWithOpenUI5Support(popup: object) { openUI5Support?.addOpenedPopup(popup); } -function removeAnyPopup(popup: object) { +function unregisterPopupWithOpenUI5Support(popup: object) { openUI5Support?.removeOpenedPopup(popup); } @@ -27,7 +27,7 @@ const addOpenedPopup = (instance: Popup, parentPopovers: Array = []) => { parentPopovers, }); - addAnyPopup(instance); + registerPopupWithOpenUI5Support(instance); } _updateTopModalPopup(); @@ -42,7 +42,7 @@ const removeOpenedPopup = (instance: Popup) => { return el.instance !== instance; }); - removeAnyPopup(instance); + unregisterPopupWithOpenUI5Support(instance); _updateTopModalPopup(); From d24b45c3b64f0c6892f8554e1f1c73a3865fb85d Mon Sep 17 00:00:00 2001 From: Teodor Taushanov Date: Fri, 18 Jul 2025 11:57:03 +0300 Subject: [PATCH 6/7] chore: add comments --- packages/base/src/features/patchPopup.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/base/src/features/patchPopup.ts b/packages/base/src/features/patchPopup.ts index 748c37eea43b..62795ccb7937 100644 --- a/packages/base/src/features/patchPopup.ts +++ b/packages/base/src/features/patchPopup.ts @@ -94,6 +94,8 @@ const patchClosed = (Popup: OpenUI5Popup) => { const patchFocusEvent = (Popup: OpenUI5Popup) => { const origFocusEvent = Popup.prototype.onFocusEvent; Popup.prototype.onFocusEvent = function onFocusEvent(e: FocusEvent) { + // If the popup is the topmost one, we call the original focus event handler from the OpenUI5 Popup, + // otherwise the focus event is handled by the Web Component Popup. if (this === getTopMostPopup()) { origFocusEvent.call(this, e); } From a4801ba7ded62c6aaf416461f13e66f76406ee13 Mon Sep 17 00:00:00 2001 From: Teodor Taushanov Date: Wed, 23 Jul 2025 13:28:48 +0300 Subject: [PATCH 7/7] chore: rename topMost to topmost --- packages/base/src/features/OpenUI5Support.ts | 6 +++--- packages/base/src/features/patchPopup.ts | 6 +++--- packages/main/src/popup-utils/OpenedPopupsRegistry.ts | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/base/src/features/OpenUI5Support.ts b/packages/base/src/features/OpenUI5Support.ts index 9af13d3073dc..de05716fc8ee 100644 --- a/packages/base/src/features/OpenUI5Support.ts +++ b/packages/base/src/features/OpenUI5Support.ts @@ -4,7 +4,7 @@ import { patchPopup, addOpenedPopup, removeOpenedPopup, - getTopMostPopup, + getTopmostPopup, } from "./patchPopup.js"; import type { OpenUI5Popup } from "./patchPopup.js"; import { registerFeature } from "../FeaturesRegistry.js"; @@ -237,8 +237,8 @@ class OpenUI5Support { removeOpenedPopup(popup); } - static getTopMostPopup() { - return getTopMostPopup(); + static getTopmostPopup() { + return getTopmostPopup(); } } diff --git a/packages/base/src/features/patchPopup.ts b/packages/base/src/features/patchPopup.ts index 62795ccb7937..f724bfe9ecf5 100644 --- a/packages/base/src/features/patchPopup.ts +++ b/packages/base/src/features/patchPopup.ts @@ -30,7 +30,7 @@ const removeOpenedPopup = (popup: object) => { } }; -const getTopMostPopup = () => { +const getTopmostPopup = () => { return AllOpenedPopupsRegistry.openedRegistry[AllOpenedPopupsRegistry.openedRegistry.length - 1]; }; @@ -96,7 +96,7 @@ const patchFocusEvent = (Popup: OpenUI5Popup) => { Popup.prototype.onFocusEvent = function onFocusEvent(e: FocusEvent) { // If the popup is the topmost one, we call the original focus event handler from the OpenUI5 Popup, // otherwise the focus event is handled by the Web Component Popup. - if (this === getTopMostPopup()) { + if (this === getTopmostPopup()) { origFocusEvent.call(this, e); } }; @@ -119,7 +119,7 @@ export { patchPopup, addOpenedPopup, removeOpenedPopup, - getTopMostPopup, + getTopmostPopup, }; export type { OpenUI5Popup }; diff --git a/packages/main/src/popup-utils/OpenedPopupsRegistry.ts b/packages/main/src/popup-utils/OpenedPopupsRegistry.ts index 315b0b7a8756..3b3898a68079 100644 --- a/packages/main/src/popup-utils/OpenedPopupsRegistry.ts +++ b/packages/main/src/popup-utils/OpenedPopupsRegistry.ts @@ -61,14 +61,14 @@ const _keydownListener = (event: KeyboardEvent) => { } if (isEscape(event)) { - const topPopup = OpenedPopupsRegistry.openedRegistry[OpenedPopupsRegistry.openedRegistry.length - 1].instance; + const topmostPopup = OpenedPopupsRegistry.openedRegistry[OpenedPopupsRegistry.openedRegistry.length - 1].instance; - if (openUI5Support && topPopup !== openUI5Support.getTopMostPopup()) { + if (openUI5Support && topmostPopup !== openUI5Support.getTopmostPopup()) { return; } event.stopPropagation(); - topPopup.closePopup(true); + topmostPopup.closePopup(true); } };