fix(ui5-popover): render block layers in the correct order #12659
Conversation
OpenUI5 dialogs' and the WebC dialogs' block layers are rendered in order Fixes: 12444
|
🚀 Deployed on https://pr-12659--ui5-webcomponents-preview.netlify.app |
Fixes failing test
Stoev
left a comment
Stoev
left a comment
There was a problem hiding this comment.
OpenUI5 dialogs' and the Web Components dialogs' block layers are rendered and displayed in the order of opened dialogs.
Updates samples and tests
alexandar-mitsev
left a comment
alexandar-mitsev
left a comment
There was a problem hiding this comment.
- when you start from "Open WebC Dialog" and reach to the "Web Component Dialog 2". Press on "Open UI5 Popover" - the focus moves to the "Web Component Dialog 1". See attached image
| }; | ||
|
|
||
| const openNativePopover = (domRef: HTMLElement) => { | ||
| const enableNativePopoverForOpenUI5 = (domRef: HTMLElement, popup: OpenUI5Popup) => { |
There was a problem hiding this comment.
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 prevPopupContent = prevPopup.instance.getContent()!; | ||
| const content = prevPopupContent instanceof HTMLElement ? prevPopupContent : prevPopupContent?.getDomRef(); |
There was a problem hiding this comment.
now you have this 2 rows on 3 places. Make it a separate funciton. E.g. getPopupContentElement
| }; | ||
|
|
||
| type OpenUI5PopupBasedControl = { | ||
| // The lifecycle of Popup.js is open -> _opened -> close -> _closed, we're interested in the first (open) and last (_closed) |
There was a problem hiding this comment.
this comment should be before the OpenUI5Popup type? Line 9
| document.getElementById("dialogButton").addEventListener("click", function () { | ||
| openUI5Dialog(win); | ||
| }); | ||
|
|
There was a problem hiding this comment.
- add a sample with an OpenUI5 popover which is modal and opened from a dialog
| domRef.removeAttribute("popover"); | ||
| } | ||
|
|
||
| const lastPopup = AllOpenedPopupsRegistry.openedRegistry[AllOpenedPopupsRegistry.openedRegistry.length - 1]; |
There was a problem hiding this comment.
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
|
|
||
| content?.hidePopover(); | ||
|
|
||
| if (prevPopup.instance.getModal()) { |
There was a problem hiding this comment.
you already checked if it is modal on line 116
| } | ||
|
|
||
| const prevPopup = AllOpenedPopupsRegistry.openedRegistry[AllOpenedPopupsRegistry.openedRegistry.length - 2]; | ||
| if (!prevPopup || prevPopup.type !== "OpenUI5" || !prevPopup.instance.getModal()) { |
There was a problem hiding this comment.
you can split this on several lines or several ifs to make it more readable
| _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 |
There was a problem hiding this comment.
disableNativePopoverForOpenUI5(domRef, this);
add "this" to the params of the function
then you can use it inside the function to check for lastPopup
OpenUI5 dialogs' and the Web Components dialogs' block layers are rendered and displayed in the order of opened dialogs.
Fixes: #12444