Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/base/src/features/OpenUI5Support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
removeOpenedPopup,
getTopmostPopup,
} from "./patchPopup.js";
import type { OpenUI5Popup } from "./patchPopup.js";
import type { OpenUI5Popup, PopupInfo } from "./patchPopup.js";
import { registerFeature } from "../FeaturesRegistry.js";
import { setTheme } from "../config/Theme.js";
import type { CLDRData } from "../asset-registries/LocaleData.js";
Expand Down Expand Up @@ -230,8 +230,8 @@ class OpenUI5Support {
return !!link.href.match(/\/css(-|_)variables\.css/) || !!link.href.match(/\/library\.css/);
}

static addOpenedPopup(popup: object) {
addOpenedPopup(popup);
static addOpenedPopup(popupInfo: PopupInfo) {
addOpenedPopup(popupInfo);
}

static removeOpenedPopup(popup: object) {
Expand Down
48 changes: 38 additions & 10 deletions packages/base/src/features/patchPopup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,49 @@ type OpenUI5Popup = {
}
};

type PopupInfo = {
type: "OpenUI5" | "WebComponent";
instance: object;
};

// contains all OpenUI5 and Web Component popups that are currently opened
const AllOpenedPopupsRegistry = getSharedResource<{ openedRegistry: Array<object> }>("AllOpenedPopupsRegistry", { openedRegistry: [] });
const AllOpenedPopupsRegistry = getSharedResource<{ openedRegistry: Array<PopupInfo> }>("AllOpenedPopupsRegistry", { openedRegistry: [] });

const addOpenedPopup = (popup: object) => {
AllOpenedPopupsRegistry.openedRegistry.push(popup);
const addOpenedPopup = (popupInfo: PopupInfo) => {
AllOpenedPopupsRegistry.openedRegistry.push(popupInfo);
};

const removeOpenedPopup = (popup: object) => {
const index = AllOpenedPopupsRegistry.openedRegistry.indexOf(popup);
const index = AllOpenedPopupsRegistry.openedRegistry.findIndex(el => el.instance === popup);
if (index > -1) {
AllOpenedPopupsRegistry.openedRegistry.splice(index, 1);
}
};

const getTopmostPopup = () => {
return AllOpenedPopupsRegistry.openedRegistry[AllOpenedPopupsRegistry.openedRegistry.length - 1];
return AllOpenedPopupsRegistry.openedRegistry[AllOpenedPopupsRegistry.openedRegistry.length - 1].instance;
};

/**
* Original OpenUI5 popup focus event is triggered only
* if there are no Web Component popups opened on top of it.
*
* @param {object} popup - The popup instance to check.
* @returns {boolean} True if the focus event should be triggered, false otherwise.
*/
const shouldCallOpenUI5FocusEvent = (popup: object) => {
for (let i = AllOpenedPopupsRegistry.openedRegistry.length - 1; i >= 0; i--) {
const popupInfo = AllOpenedPopupsRegistry.openedRegistry[i];
if (popupInfo.type !== "OpenUI5") {
return false;
}

if (popupInfo.instance === popup) {
break;
}
}

return true;
};

const openNativePopover = (domRef: HTMLElement) => {
Expand Down Expand Up @@ -73,7 +100,10 @@ const patchOpen = (Popup: OpenUI5Popup) => {
}
}

addOpenedPopup(this);
addOpenedPopup({
type: "OpenUI5",
instance: this,
});
};
};

Expand All @@ -94,9 +124,7 @@ 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()) {
if (shouldCallOpenUI5FocusEvent(this)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an alternative is to patch only the logic for topmost popup - see 54781ca and https://git.wdf.sap.corp/c/openui5/+/6794573
unfortunately most issues still remain, but seems better patching for long term solution as it directly fixes the initial problem

origFocusEvent.call(this, e);
}
};
Expand All @@ -122,4 +150,4 @@ export {
getTopmostPopup,
};

export type { OpenUI5Popup };
export type { OpenUI5Popup, PopupInfo };
10 changes: 7 additions & 3 deletions packages/main/src/popup-utils/OpenedPopupsRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ 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";
import type { PopupInfo } from "@ui5/webcomponents-base/dist/features/patchPopup.js";

type RegisteredPopup = {
instance: Popup;
Expand All @@ -12,8 +13,8 @@ type RegisteredPopup = {
const OpenedPopupsRegistry = getSharedResource<{ openedRegistry: Array<RegisteredPopup> }>("OpenedPopupsRegistry", { openedRegistry: [] });
const openUI5Support = getFeature<typeof OpenUI5Support>("OpenUI5Support");

function registerPopupWithOpenUI5Support(popup: object) {
openUI5Support?.addOpenedPopup(popup);
function registerPopupWithOpenUI5Support(popupInfo: PopupInfo) {
openUI5Support?.addOpenedPopup(popupInfo);
}

function unregisterPopupWithOpenUI5Support(popup: object) {
Expand All @@ -27,7 +28,10 @@ const addOpenedPopup = (instance: Popup, parentPopovers: Array<Popup> = []) => {
parentPopovers,
});

registerPopupWithOpenUI5Support(instance);
registerPopupWithOpenUI5Support({
type: "WebComponent",
instance,
});
}

_updateTopModalPopup();
Expand Down
73 changes: 50 additions & 23 deletions packages/main/test/pages/DialogAndOpenUI5Dialog.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
data-sap-ui-compatVersion="edge"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need more samples

  • openui5 dialog with several ui5-webcomponents inputs like select, combobox and input
  • webc dialog with several openui5 inputs
  • openui5 popover with webc inputs
  • webc popover with openui5 inputs

<script>
function onOpenUI5Init() {
sap.ui.require(["sap/m/Button", "sap/m/Dialog"], (Button, Dialog) => {
sap.ui.require(["sap/m/Button", "sap/m/Dialog", "sap/m/Input"], (Button, Dialog, Input) => {
new Button("openUI5Button", {
text: "Open OpenUI5 Dialog",
press: function () {
new Dialog({
title: "OpenUI5 Dialog",
content: [
new Input(),
new Button({
text: "Focus stop"
}),
Expand All @@ -47,12 +48,41 @@
});
}

function openUI5Dialog() {
sap.ui.require(["sap/m/Button", "sap/m/Dialog"], (Button, Dialog) => {
new Dialog({
title: "OpenUI5 Dialog",
content: [
new Button({
text: "Focus stop"
}),
new Button("openUI5DialogButton", {
text: "Open WebC Dialog",
press: function () {
document.getElementById("newDialog1").open = true;
}
})
],
afterClose: function () {
this.destroy();
}
}).open();
});
}

function init() {
document.getElementById("myButton").addEventListener("click", function() {
document.getElementById("dialog1").open = true;
});

sap.ui.require(["sap/m/Select", "sap/ui/core/Item"], (Select, Item) => {
sap.ui.require(["sap/m/Select",
"sap/m/Button",
"sap/ui/core/Item",
"sap/ui/core/ShortcutHintsMixin"],
(Select,
Button,
Item,
ShortcutHintsMixin) => {
new Select({
items: [
new Item({ text: "Item 1" }),
Expand All @@ -62,29 +92,26 @@
change: function (oEvent) {
console.error("Selected item:", oEvent.getParameter("selectedItem").getText());
}
}).placeAt("dilog1content");
}).placeAt("dialog1content");

const button = new Button({
text: "OpenUI5 with Shortcut (Ctrl+S)",
press: function () {
openUI5Dialog();
}
}).placeAt("dialog1content");


ShortcutHintsMixin.addConfig(button, {
event: "press",
position: "0 0",
addAccessibilityLabel: true,
message: "Save"
}, button);
});

document.getElementById("dialogButton").addEventListener("click", function () {
sap.ui.require(["sap/m/Button", "sap/m/Dialog"], (Button, Dialog) => {
new Dialog({
title: "OpenUI5 Dialog",
content: [
new Button({
text: "Focus stop"
}),
new Button("openUI5DialogButton", {
text: "Open WebC Dialog",
press: function () {
document.getElementById("newDialog1").open = true;
}
})
],
afterClose: function () {
this.destroy();
}
}).open();
});
openUI5Dialog();
});
}
</script>
Expand All @@ -94,7 +121,7 @@
<ui5-button id="myButton">Open WebC Dialog</ui5-button>
</div>
<ui5-dialog id="dialog1" header-text="This is an WebC Dialog 1">
<div id="dilog1content"></div>
<div id="dialog1content"></div>
<ui5-button id="dialogButton">Open UI5 dialog</ui5-button>
</ui5-dialog>
<ui5-dialog id="newDialog1" header-text="This is an WebC Dialog 2">
Expand Down
Loading