From 245b23b5de0a900983d6dccf52da2144487e51c7 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Wed, 26 Nov 2025 16:20:28 +0200 Subject: [PATCH 01/12] fix(ui5-dialog): Dynamically opened dialogs no longer flicker when opened --- packages/main/cypress/specs/Dialog.cy.tsx | 133 ++++++++++++++++------ packages/main/src/Dialog.ts | 5 +- packages/main/src/Popup.ts | 2 +- packages/main/test/pages/Dialog.html | 11 ++ 4 files changed, 114 insertions(+), 37 deletions(-) diff --git a/packages/main/cypress/specs/Dialog.cy.tsx b/packages/main/cypress/specs/Dialog.cy.tsx index 646b58dedf45..fd404ba496d2 100644 --- a/packages/main/cypress/specs/Dialog.cy.tsx +++ b/packages/main/cypress/specs/Dialog.cy.tsx @@ -357,8 +357,6 @@ describe("Events", () => { describe("Dialog general interaction", () => { it("tests dialog toggling", () => { - - cy.mount( <> @@ -406,6 +404,62 @@ describe("Dialog general interaction", () => { .should("be.calledOnce"); }); + it("dynamic dialog initial positioning", () => { + const dialog = document.createElement("ui5-dialog") as Dialog; + dialog.setAttribute("id", "dynamic-dialog"); + + const content = document.createElement("div"); + content.style.height = "600px"; + content.style.width = "600px"; + content.textContent = "Content"; + + dialog.appendChild(content); + + // Spy on Object.assign only for this dialog's style + cy.window().then(() => { + const originalAssign = Object.assign; + const topAndLeftStyles: Array<{ top: number; left: number }> = []; + + cy.stub(Object, "assign").callsFake(function(target: any, ...sources: any[]) { + // Check if target is the dialog's style object + if (target === dialog.style) { + const styleObj = sources[0]; + if (styleObj && styleObj.top !== undefined && styleObj.left !== undefined) { + topAndLeftStyles.push({ + top: parseInt(styleObj.top), + left: parseInt(styleObj.left) + }); + } + } + return originalAssign.call(this, target, ...sources); + }).as("objectAssignStub"); + + cy.wrap(topAndLeftStyles).as("topAndLeftStyles"); + + dialog.setAttribute("open", "true"); + document.body.appendChild(dialog); + }); + + cy.get("#dynamic-dialog").ui5DialogOpened(); + + // Assert the captured style values + cy.get>("@topAndLeftStyles") + .should((topAndLeftStyles) => { + expect(topAndLeftStyles.length).to.be.greaterThan(1, "'top' and 'left' styles should have been assigned"); + + // styles from initial call of _center method + const firstStyles = topAndLeftStyles[0]; + + expect(firstStyles.top).to.not.equal(0, "top should not start from 0"); + expect(firstStyles.left).to.not.equal(0, "left should not start from 0"); + + // styles from _center method called as resize handler callback + const secondStyles = topAndLeftStyles[1]; + + expect(secondStyles.top).to.equal(firstStyles.top, "top should remain the same after resize event"); + expect(secondStyles.left).to.equal(firstStyles.left, "left should remain the same after resize event"); + }); + }); it("dialog repositions after screen resize", () => { cy.mount( @@ -516,46 +570,56 @@ describe("Dialog general interaction", () => { cy.get("#draggable-dialog").invoke("attr", "open", true); cy.get("#draggable-dialog").ui5DialogOpened(); + let topBeforeDragging: number; + let leftBeforeDragging: number; + // Capture position before dragging cy.get("#draggable-dialog") - .then(dialog => { - const topBeforeDragging = parseInt(dialog.css("top")); - const leftBeforeDragging = parseInt(dialog.css("left")); + .should(dialog => { + topBeforeDragging = parseInt(dialog.css("top")); + leftBeforeDragging = parseInt(dialog.css("left")); - // Drag dialog - cy.get("#draggable-dialog") - .find("#header-slot") - .trigger("mousedown", { which: 1 }) - .trigger("mousemove", { clientX: 150, clientY: 150 }) - .trigger("mouseup"); + expect(topBeforeDragging).to.equal(492); + expect(leftBeforeDragging).to.equal(560); + }); - // Capture position after dragging - cy.get("#draggable-dialog") - .should(dialogAfterDragging => { - const topAfterDragging = parseInt(dialogAfterDragging.css("top")); - const leftAfterDragging = parseInt(dialogAfterDragging.css("left")); + // Drag dialog + cy.get("#draggable-dialog") + .find("#header-slot") + .trigger("mousedown", { which: 1 }) + .trigger("mousemove", { clientX: 200, clientY: 150, }) + .trigger("mouseup"); - // Assert position changes - expect(topBeforeDragging).not.to.equal(topAfterDragging); - expect(leftBeforeDragging).not.to.equal(leftAfterDragging); - }); + cy.get("#draggable-dialog") + .should("have.css", "top", "141px") + .should("have.css", "left", "40px"); - // Close dialog - cy.get("#draggable-dialog").invoke("attr", "open", false); + // Capture position after dragging + cy.get("#draggable-dialog") + .should(dialogAfterDragging => { + const topAfterDragging = parseInt(dialogAfterDragging.css("top")); + const leftAfterDragging = parseInt(dialogAfterDragging.css("left")); - // Reopen dialog - cy.get("#draggable-dialog").invoke("attr", "open", true); + // Assert position changes + expect(topBeforeDragging).not.to.equal(topAfterDragging); + expect(leftBeforeDragging).not.to.equal(leftAfterDragging); + }); - // Capture position after reopening - cy.get("#draggable-dialog") - .should(dialogAfterReopening => { - const topAfterReopening = parseInt(dialogAfterReopening.css("top")); - const leftAfterReopening = parseInt(dialogAfterReopening.css("left")); + // Close dialog + cy.get("#draggable-dialog").invoke("attr", "open", false); - // Assert position resets - expect(topBeforeDragging).to.equal(topAfterReopening); - expect(leftBeforeDragging).to.equal(leftAfterReopening); - }); + // Reopen dialog + cy.get("#draggable-dialog").invoke("attr", "open", true); + + // Capture position after reopening + cy.get("#draggable-dialog") + .should(dialogAfterReopening => { + const topAfterReopening = parseInt(dialogAfterReopening.css("top")); + const leftAfterReopening = parseInt(dialogAfterReopening.css("left")); + + // Assert position resets + expect(topBeforeDragging).to.equal(topAfterReopening); + expect(leftBeforeDragging).to.equal(leftAfterReopening); }); }); @@ -631,7 +695,6 @@ describe("Dialog general interaction", () => { }); it("resizable - mouse support", () => { - cy.mount( <> @@ -1583,4 +1646,4 @@ describe("Dialog initially open", () => { // Assert dialog matches :popover-open selector cy.get("#dialogOpen").should("match", ":popover-open"); }); -}); \ No newline at end of file +}); diff --git a/packages/main/src/Dialog.ts b/packages/main/src/Dialog.ts index ecb7110ed8af..79964ff77db3 100644 --- a/packages/main/src/Dialog.ts +++ b/packages/main/src/Dialog.ts @@ -329,7 +329,10 @@ class Dialog extends Popup { _show() { super._show(); - this._center(); + requestAnimationFrame(() => { + this._updateMediaRange(); // TODO: Carefully put in Popup + this._center(); + }); } onBeforeRendering() { diff --git a/packages/main/src/Popup.ts b/packages/main/src/Popup.ts index fda670bdff31..9c77fbc1a2ca 100644 --- a/packages/main/src/Popup.ts +++ b/packages/main/src/Popup.ts @@ -604,7 +604,7 @@ abstract class Popup extends UI5Element { } /** - * Sets "block" display to the popup. The property can be overriden by derivatives of Popup. + * Sets "popover=manual" to the popup. The method can be overridden by derivatives of Popup. * @protected */ _show() { diff --git a/packages/main/test/pages/Dialog.html b/packages/main/test/pages/Dialog.html index 119577f212e3..0d41936bc8f4 100644 --- a/packages/main/test/pages/Dialog.html +++ b/packages/main/test/pages/Dialog.html @@ -24,6 +24,7 @@ + Open dialog with unwanted animation
@@ -871,6 +872,16 @@ From 3dfd19c6c10688e3f4b049754162524c30b2828c Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Tue, 16 Dec 2025 10:44:33 +0200 Subject: [PATCH 06/12] fix(popup): restore media range update onAfterRendering --- packages/main/src/Popup.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/main/src/Popup.ts b/packages/main/src/Popup.ts index 9750a611ec5d..0023b69e30f4 100644 --- a/packages/main/src/Popup.ts +++ b/packages/main/src/Popup.ts @@ -270,6 +270,8 @@ abstract class Popup extends UI5Element { } onAfterRendering() { + this._updateMediaRange(); + if (this.open) { this._registerResizeHandler(); } else { From 0cf875d8aea9068afbc2c608fd9d466b56d786b3 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Tue, 16 Dec 2025 11:40:00 +0200 Subject: [PATCH 07/12] test(multicombobox): stablize test --- packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx b/packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx index 527c05e18b2b..b49e096f7bba 100644 --- a/packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx +++ b/packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx @@ -191,6 +191,11 @@ describe("Typeahead", () => { cy.get("@input") .realClick(); + cy.get("[ui5-multi-combobox]") + .shadow() + .find("[ui5-responsive-popover]") + .ui5ResponsivePopoverOpened(); + cy.get("[ui5-multi-combobox]") .shadow() .find("[ui5-responsive-popover]") From dda76bded1d407a2a57ba78cd36bbbd751ca021e Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Tue, 16 Dec 2025 11:59:15 +0200 Subject: [PATCH 08/12] test: use ui5ResponsivePopoverOpened --- .../cypress/specs/OpenUI5andWebCPopups.cy.tsx | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx b/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx index b5e62f6e546c..02cb03557fb6 100644 --- a/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx +++ b/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx @@ -518,12 +518,11 @@ describe("ui5 and web components integration", () => { .should('be.visible') .realClick(); - cy.get("#respPopover").ui5DialogOpened(); + cy.get("#respPopover").ui5ResponsivePopoverOpened(); cy.realPress("Escape"); - cy.get("#respPopover") - .should('not.be.visible'); + cy.get("#respPopover").ui5ResponsivePopoverClosed(); cy.get("#openUI5Dialog1") .should('be.visible'); @@ -549,12 +548,12 @@ describe("ui5 and web components integration", () => { .should('be.visible') .realClick(); - cy.get("#respPopoverNoInitialFocus").ui5DialogOpened(); + cy.get("#respPopoverNoInitialFocus").ui5ResponsivePopoverOpened(); cy.realPress("Escape"); - cy.get("#respPopoverNoInitialFocus") - .should('not.be.visible'); + cy.get("#respPopoverNoInitialFocus") + .ui5ResponsivePopoverClosed(); cy.get("#openResPopoverNoInitialFocusButton") .should('be.focused'); @@ -591,8 +590,8 @@ describe("ui5 and web components integration", () => { cy.get("#webCSelect1") .shadow() - .find("[ui5-responsive-popover]") - .should('not.be.visible'); + .find("[ui5-responsive-popover]") + .ui5ResponsivePopoverClosed(); cy.get("#openUI5Dialog1") .should('be.visible'); @@ -631,8 +630,8 @@ describe("ui5 and web components integration", () => { cy.get("#webCComboBox1") .shadow() - .find("[ui5-responsive-popover]") - .should('not.be.visible'); + .find("[ui5-responsive-popover]") + .ui5ResponsivePopoverClosed(); cy.get("#openUI5Dialog1") .should('be.visible'); From d3c6669356ebb23371e7496005346006e2716ab4 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Tue, 16 Dec 2025 14:11:28 +0200 Subject: [PATCH 09/12] test(OpenUI5AndWebC): expect isOpen() of sap.m.Dialog to be true --- .../cypress/specs/OpenUI5andWebCPopups.cy.tsx | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx b/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx index 02cb03557fb6..aa4795127008 100644 --- a/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx +++ b/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx @@ -7,11 +7,20 @@ import ComboBox from "../../src/ComboBox.js"; import ComboBoxItem from "../../src/ComboBoxItem.js"; import ResponsivePopover from "../../src/ResponsivePopover.js"; +let OpenUI5Element; + function onOpenUI5InitMethod(win) { - (win as any).sap.ui.require(["sap/ui/core/HTML", "sap/m/Button", "sap/m/Dialog", "sap/m/Popover", "sap/m/Input"], async (HTML, Button, Dialog, Popover, Input) => { + (win as any).sap.ui.require([ + "sap/ui/core/Element", + "sap/ui/core/HTML", + "sap/m/Button", + "sap/m/Dialog", + "sap/m/Popover", + "sap/m/Input" + ], async (Element, HTML, Button, Dialog, Popover, Input) => { await OpenUI5Support.init(); - + OpenUI5Element = Element; new Button("openUI5Button", { text: "Open OpenUI5 Dialog", press: function () { @@ -687,7 +696,15 @@ describe("ui5 and web components integration", () => { .realClick(); cy.get("#openUI5DialogFinal") - .should('be.visible'); + .should('be.visible') + .should(($dialog) => { + expect(OpenUI5Element).to.exist; + + const dialogInstance = OpenUI5Element.getElementById($dialog.attr("id")); + + expect(dialogInstance).to.exist + expect(dialogInstance.isOpen()).to.be.true; + }); cy.get("#openUI5Dialog1") .should('not.be.visible'); From eeef28286a55559f252063d30b8b8c153e266f33 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Wed, 17 Dec 2025 10:49:42 +0200 Subject: [PATCH 10/12] refactor: move _updateMediaRange back to the dialog --- packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx | 5 +++++ packages/main/src/Dialog.ts | 5 ++++- packages/main/src/Popup.ts | 2 -- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx b/packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx index b49e096f7bba..dc5a84ee0647 100644 --- a/packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx +++ b/packages/main/cypress/specs/MultiComboBox.mobile.cy.tsx @@ -224,6 +224,11 @@ describe("Typeahead", () => { cy.get("@input") .realClick(); + cy.get("[ui5-multi-combobox]") + .shadow() + .find("[ui5-responsive-popover]") + .ui5ResponsivePopoverOpened(); + cy.get("[ui5-multi-combobox]") .shadow() .find("[ui5-responsive-popover]") diff --git a/packages/main/src/Dialog.ts b/packages/main/src/Dialog.ts index 8a9625ed077d..891c8ffa0051 100644 --- a/packages/main/src/Dialog.ts +++ b/packages/main/src/Dialog.ts @@ -329,7 +329,10 @@ class Dialog extends Popup { _show() { super._show(); - requestAnimationFrame(this._center.bind(this)); + requestAnimationFrame(() => { + this._updateMediaRange(); + this._center(); + }); } onBeforeRendering() { diff --git a/packages/main/src/Popup.ts b/packages/main/src/Popup.ts index 0023b69e30f4..9c7a3c774184 100644 --- a/packages/main/src/Popup.ts +++ b/packages/main/src/Popup.ts @@ -604,8 +604,6 @@ abstract class Popup extends UI5Element { if (this.isConnected) { this.setAttribute("popover", "manual"); this.showPopover(); - - requestAnimationFrame(this._updateMediaRange.bind(this)); } } From bf6c1ed2f7ad5ae10e16f0685b7ffa79732c0424 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Wed, 17 Dec 2025 11:09:26 +0200 Subject: [PATCH 11/12] test: stabilize OpenUI5andWebCPopups --- .../cypress/specs/OpenUI5andWebCPopups.cy.tsx | 27 ++++++++++++------- packages/main/src/Dialog.ts | 5 +--- packages/main/src/Popup.ts | 2 ++ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx b/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx index aa4795127008..1b13578f9e67 100644 --- a/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx +++ b/packages/main/cypress/specs/OpenUI5andWebCPopups.cy.tsx @@ -215,6 +215,15 @@ function openUI5Popover(win, opener) { }); } +function isOpenUI5DialogOpen($dialog) { + expect(OpenUI5Element).to.exist; + + const dialogInstance = OpenUI5Element.getElementById($dialog.attr("id")); + + expect(dialogInstance).to.exist + expect(dialogInstance.isOpen()).to.be.true; +}; + describe("ui5 and web components integration", () => { beforeEach(() => { // mount the components @@ -489,6 +498,10 @@ describe("ui5 and web components integration", () => { .find('input') .focus(); + cy.get("#openUI5Combobox1") + .find('input') + .should('be.focused'); + cy.realPress("Escape"); cy.get('#dialog1') @@ -697,14 +710,8 @@ describe("ui5 and web components integration", () => { cy.get("#openUI5DialogFinal") .should('be.visible') - .should(($dialog) => { - expect(OpenUI5Element).to.exist; - - const dialogInstance = OpenUI5Element.getElementById($dialog.attr("id")); - - expect(dialogInstance).to.exist - expect(dialogInstance.isOpen()).to.be.true; - }); + .should(isOpenUI5DialogOpen); + cy.wait(1000); cy.get("#openUI5Dialog1") .should('not.be.visible'); @@ -722,7 +729,9 @@ describe("ui5 and web components integration", () => { .should('not.exist'); cy.get("#openUI5DialogWithButtons") - .should('be.visible'); + .should('be.visible') + .should(isOpenUI5DialogOpen); + cy.wait(100); cy.realPress("Escape"); diff --git a/packages/main/src/Dialog.ts b/packages/main/src/Dialog.ts index 891c8ffa0051..8a9625ed077d 100644 --- a/packages/main/src/Dialog.ts +++ b/packages/main/src/Dialog.ts @@ -329,10 +329,7 @@ class Dialog extends Popup { _show() { super._show(); - requestAnimationFrame(() => { - this._updateMediaRange(); - this._center(); - }); + requestAnimationFrame(this._center.bind(this)); } onBeforeRendering() { diff --git a/packages/main/src/Popup.ts b/packages/main/src/Popup.ts index 9c7a3c774184..0023b69e30f4 100644 --- a/packages/main/src/Popup.ts +++ b/packages/main/src/Popup.ts @@ -604,6 +604,8 @@ abstract class Popup extends UI5Element { if (this.isConnected) { this.setAttribute("popover", "manual"); this.showPopover(); + + requestAnimationFrame(this._updateMediaRange.bind(this)); } } From ee05d5d4bfb57cf17283a8a621cef5c80a931d12 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Thu, 15 Jan 2026 15:31:05 +0200 Subject: [PATCH 12/12] fix: avoid issues with virtual keyboard on ios devices --- packages/main/src/Dialog.ts | 12 +++++++++++- packages/main/src/Popup.ts | 2 -- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/main/src/Dialog.ts b/packages/main/src/Dialog.ts index 8a9625ed077d..5bd9c6ac4d24 100644 --- a/packages/main/src/Dialog.ts +++ b/packages/main/src/Dialog.ts @@ -29,6 +29,7 @@ import DialogTemplate from "./DialogTemplate.js"; import PopupsCommonCss from "./generated/themes/PopupsCommon.css.js"; import dialogCSS from "./generated/themes/Dialog.css.js"; import PopupAccessibleRole from "./types/PopupAccessibleRole.js"; +import { isIOS } from "@ui5/webcomponents-base"; /** * Defines the step size at which this component would change by when being dragged or resized with the keyboard. @@ -329,7 +330,16 @@ class Dialog extends Popup { _show() { super._show(); - requestAnimationFrame(this._center.bind(this)); + + if (isIOS()) { + this._updateMediaRange(); + this._center(); + } else { + requestAnimationFrame(() => { + this._updateMediaRange(); + this._center(); + }); + } } onBeforeRendering() { diff --git a/packages/main/src/Popup.ts b/packages/main/src/Popup.ts index 0023b69e30f4..9c7a3c774184 100644 --- a/packages/main/src/Popup.ts +++ b/packages/main/src/Popup.ts @@ -604,8 +604,6 @@ abstract class Popup extends UI5Element { if (this.isConnected) { this.setAttribute("popover", "manual"); this.showPopover(); - - requestAnimationFrame(this._updateMediaRange.bind(this)); } }