Skip to content
Open
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
5 changes: 5 additions & 0 deletions packages/base/src/css/OpenUI5PopupStyles.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,9 @@
border: none;
overflow: visible;
margin: 0;
}

.sapUiBLy[popover] {
width: 100%;
height: 100%;
}
4 changes: 2 additions & 2 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, OpenUI5PopupBasedControl, PopupInfo } from "./patchPopup.js";
import type { OpenUI5PopupClass, OpenUI5DialogClass, 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 @@ -110,7 +110,7 @@ class OpenUI5Support {
"sap/ui/core/date/CalendarUtils",
];
}
window.sap.ui.require(deps, (Popup: OpenUI5Popup, Dialog: OpenUI5PopupBasedControl, Patcher: OpenUI5Patcher) => {
window.sap.ui.require(deps, (Popup: OpenUI5PopupClass, Dialog: OpenUI5DialogClass, Patcher: OpenUI5Patcher) => {
patchPatcher(Patcher);
patchPopup(Popup, Dialog);
resolve();
Expand Down
100 changes: 75 additions & 25 deletions packages/base/src/features/patchPopup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,33 @@ type Control = {
getDomRef: () => HTMLElement | null,
}

// The lifecycle of Popup.js is open -> _opened -> close -> _closed, we're interested in the first (open) and last (_closed)
type OpenUI5Popup = {
prototype: {
open: (...args: any[]) => void,
_closed: (...args: any[]) => void,
getOpenState: () => "CLOSED" | "CLOSING" | "OPEN" | "OPENING",
getContent: () => Control | HTMLElement | null, // this is the OpenUI5 Element/Control instance that opens the Popup (usually sap.m.Popover/sap.m.Dialog)
onFocusEvent: (...args: any[]) => void,
}
open: (...args: any[]) => void,
_closed: (...args: any[]) => void,
getOpenState: () => "CLOSED" | "CLOSING" | "OPEN" | "OPENING",
getContent: () => Control | HTMLElement | null, // this is the OpenUI5 Element/Control instance that opens the Popup (usually sap.m.Popover/sap.m.Dialog)
onFocusEvent: (...args: any[]) => void,
getModal: () => boolean
};

type OpenUI5PopupBasedControl = {
// The lifecycle of Popup.js is open -> _opened -> close -> _closed, we're interested in the first (open) and last (_closed)
Copy link
Member

Choose a reason for hiding this comment

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

this comment should be before the OpenUI5Popup type? Line 9

type OpenUI5PopupClass = {
prototype: OpenUI5Popup
};

type OpenUI5DialogClass = {
prototype: {
onsapescape: (...args: any[]) => void,
oPopup: OpenUI5Popup,
}
};

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

// contains all OpenUI5 and Web Component popups that are currently opened
Expand All @@ -38,6 +44,11 @@ const addOpenedPopup = (popupInfo: PopupInfo) => {

const removeOpenedPopup = (popup: object) => {
const index = AllOpenedPopupsRegistry.openedRegistry.findIndex(el => el.instance === popup);

if (index === AllOpenedPopupsRegistry.openedRegistry.length - 1) {
fixOpenUI5PopupBelow();
}

if (index > -1) {
AllOpenedPopupsRegistry.openedRegistry.splice(index, 1);
}
Expand Down Expand Up @@ -68,16 +79,56 @@ const hasWebComponentPopupAbove = (popup: object) => {
return false;
};

const openNativePopover = (domRef: HTMLElement) => {
const enableNativePopoverForOpenUI5 = (domRef: HTMLElement, popup: OpenUI5Popup) => {
Copy link
Member

Choose a reason for hiding this comment

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

enable and disable sounds strange. I was confused the first time I read it.
openNativePopover was better I think. Can be openNativePopoverForOpenUI5

same for disable and close, close sounds more clear

const openUI5BlockLayer = document.getElementById("sap-ui-blocklayer-popup");

if (popup.getModal() && openUI5BlockLayer) {
openUI5BlockLayer.setAttribute("popover", "manual");
openUI5BlockLayer.hidePopover();
openUI5BlockLayer.showPopover();
}

domRef.setAttribute("popover", "manual");
domRef.showPopover();
};

const closeNativePopover = (domRef: HTMLElement) => {
const disableNativePopoverForOpenUI5 = (domRef: HTMLElement) => {
if (domRef.hasAttribute("popover")) {
domRef.hidePopover();
domRef.removeAttribute("popover");
}

const lastPopup = AllOpenedPopupsRegistry.openedRegistry[AllOpenedPopupsRegistry.openedRegistry.length - 1];
Copy link
Member

Choose a reason for hiding this comment

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

if you close a popup which is different then lastPopup, you should not hide the block layer. E.g. if dialog behind the top most one is closed.
Currently this works because the fixOpenUI5PopupBelow fixes it, but otherwise it shouldn't be the case

if (lastPopup.type === "OpenUI5" && lastPopup.instance.getModal()) {
const openUI5BlockLayer = document.getElementById("sap-ui-blocklayer-popup");
if (openUI5BlockLayer && openUI5BlockLayer.hasAttribute("popover")) {
openUI5BlockLayer.hidePopover();
}
}
};

const fixOpenUI5PopupBelow = () => {
if (!isNativePopoverOpen()) {
return;
}

const prevPopup = AllOpenedPopupsRegistry.openedRegistry[AllOpenedPopupsRegistry.openedRegistry.length - 2];
if (!prevPopup || prevPopup.type !== "OpenUI5" || !prevPopup.instance.getModal()) {
Copy link
Member

Choose a reason for hiding this comment

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

you can split this on several lines or several ifs to make it more readable

return;
}

const prevPopupContent = prevPopup.instance.getContent()!;
const content = prevPopupContent instanceof HTMLElement ? prevPopupContent : prevPopupContent?.getDomRef();
Copy link
Member

Choose a reason for hiding this comment

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

now you have this 2 rows on 3 places. Make it a separate funciton. E.g. getPopupContentElement


const openUI5BlockLayer = document.getElementById("sap-ui-blocklayer-popup");

content?.hidePopover();

if (prevPopup.instance.getModal()) {
Copy link
Member

Choose a reason for hiding this comment

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

you already checked if it is modal on line 116

openUI5BlockLayer?.showPopover();
}

content?.showPopover();
};

const isNativePopoverOpen = (root: Document | ShadowRoot = document): boolean => {
Expand All @@ -91,9 +142,9 @@ const isNativePopoverOpen = (root: Document | ShadowRoot = document): boolean =>
});
};

const patchPopupBasedControl = (PopupBasedControl: OpenUI5PopupBasedControl) => {
const origOnsapescape = PopupBasedControl.prototype.onsapescape;
PopupBasedControl.prototype.onsapescape = function onsapescape(...args: any[]) {
const patchDialog = (Dialog: OpenUI5DialogClass) => {
const origOnsapescape = Dialog.prototype.onsapescape;
Dialog.prototype.onsapescape = function onsapescape(...args: any[]) {
if (hasWebComponentPopupAbove(this.oPopup)) {
return;
}
Expand All @@ -102,18 +153,17 @@ const patchPopupBasedControl = (PopupBasedControl: OpenUI5PopupBasedControl) =>
};
};

const patchOpen = (Popup: OpenUI5Popup) => {
const patchOpen = (Popup: OpenUI5PopupClass) => {
const origOpen = Popup.prototype.open;
Popup.prototype.open = function open(...args: any[]) {
origOpen.apply(this, args); // call open first to initiate opening
const topLayerAlreadyInUse = isNativePopoverOpen();
const openingInitiated = ["OPENING", "OPEN"].includes(this.getOpenState());
if (openingInitiated && topLayerAlreadyInUse) {
if (openingInitiated && isNativePopoverOpen()) {
const element = this.getContent();
if (element) {
const domRef = element instanceof HTMLElement ? element : element?.getDomRef();
if (domRef) {
openNativePopover(domRef);
enableNativePopoverForOpenUI5(domRef, this);
}
}
}
Expand All @@ -125,21 +175,21 @@ const patchOpen = (Popup: OpenUI5Popup) => {
};
};

const patchClosed = (Popup: OpenUI5Popup) => {
const patchClosed = (Popup: OpenUI5PopupClass) => {
const _origClosed = Popup.prototype._closed;
Popup.prototype._closed = function _closed(...args: any[]) {
const element = this.getContent();
const domRef = element instanceof HTMLElement ? element : element?.getDomRef();
_origClosed.apply(this, args); // only then call _close
if (domRef) {
closeNativePopover(domRef); // unset the popover attribute and close the native popover, but only if still in DOM
disableNativePopoverForOpenUI5(domRef); // unset the popover attribute and close the native popover, but only if still in DOM
Copy link
Member

Choose a reason for hiding this comment

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

disableNativePopoverForOpenUI5(domRef, this);
add "this" to the params of the function
then you can use it inside the function to check for lastPopup

}

removeOpenedPopup(this);
};
};

const patchFocusEvent = (Popup: OpenUI5Popup) => {
const patchFocusEvent = (Popup: OpenUI5PopupClass) => {
const origFocusEvent = Popup.prototype.onFocusEvent;
Popup.prototype.onFocusEvent = function onFocusEvent(...args: any[]) {
if (!hasWebComponentPopupAbove(this)) {
Expand All @@ -154,13 +204,13 @@ const createGlobalStyles = () => {
document.adoptedStyleSheets = [...document.adoptedStyleSheets, stylesheet];
};

const patchPopup = (Popup: OpenUI5Popup, Dialog: OpenUI5PopupBasedControl) => {
const patchPopup = (Popup: OpenUI5PopupClass, Dialog: OpenUI5DialogClass) => {
insertOpenUI5PopupStyles();
patchOpen(Popup); // Popup.prototype.open
patchClosed(Popup); // Popup.prototype._closed
createGlobalStyles(); // Ensures correct popover positioning by OpenUI5 (otherwise 0,0 is the center of the screen)
patchFocusEvent(Popup);// Popup.prototype.onFocusEvent
patchPopupBasedControl(Dialog); // Dialog.prototype.onsapescape
patchDialog(Dialog); // Dialog.prototype.onsapescape
};

export {
Expand All @@ -170,4 +220,4 @@ export {
getTopmostPopup,
};

export type { OpenUI5Popup, OpenUI5PopupBasedControl, PopupInfo };
export type { OpenUI5PopupClass, OpenUI5DialogClass, PopupInfo };
Loading
Loading