fix(ui5-dialog): Dynamically opened dialogs no longer flicker when opened

Author: dimovpetar

packages/main/cypress/specs/Dialog.cy.tsx

@@ -357,8 +357,6 @@ describe("Events", () => {
 
 describe("Dialog general interaction", () => {
 	it("tests dialog toggling", () => {
-
-
 		cy.mount(
 			<>
 				<Dialog id="dialog" accessibleName="Resizable" stretch>
@@ -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<Dialog>("#dynamic-dialog").ui5DialogOpened();
+
+		// Assert the captured style values
+		cy.get<Array<{ top: number; left: number }>>("@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<Dialog>("#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(
 			<>
 				<Dialog id="resizable-dialog" resizable>
@@ -1583,4 +1646,4 @@ describe("Dialog initially open", () => {
 		// Assert dialog matches :popover-open selector
 		cy.get("#dialogOpen").should("match", ":popover-open");
 	});
-});
+});

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() {

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() {

packages/main/test/pages/Dialog.html

@@ -24,6 +24,7 @@
 </head>
 
 <body>
+	<ui5-button id="btnUnwanted">Open dialog with unwanted animation</ui5-button>
 	<div class="wrapper">
 		<div>
 			<ui5-segmented-button id="theme-switch">
@@ -871,6 +872,16 @@
 	</ui5-dialog>
 
 	<script>
+		document.getElementById("btnUnwanted").addEventListener("click", function () {
+			const dialog = document.createElement("ui5-dialog");
+			const child1 = `<div data-v-f5de9ea6="" data-v-d7dd9f52="" class="header" slot="header" design="Header"><h3 data-v-f5de9ea6="" automation-id="contactEmailFormModal-header">Contact form</h3><udex-control-button data-v-f5de9ea6="" automation-id="contactEmailFormModal-headerCloseButton" slot="endContent" icon-only="" has-icon="" accessible-name="close button"><template shadowrootmode="open" shadowrootdelegatesfocus=""></template></udex-control-button></div>`
+			const child2 = `<div data-v-d7dd9f52="" class="modal-content" automation-id="contactEmailForm-modalContent"><div data-v-d7dd9f52="" class="modal-form-header" automation-id="contactEmailForm-modalFormHeader"><div data-v-9c54a22e="" data-v-d7dd9f52="" class="header-container" automation-id="contactEmailForm-contactEmailHeader"><div data-v-9c54a22e="" class="relative block h-full max-w-[130px]" automation-id="partnerProfilePage-partnerLogo-root"><div automation-id="partnerProfilePage-partnerLogo" class="border-grey-2 block h-[62px] w-[62px] rounded-sm border bg-white flex justify-center items-center !w-[60px] !h-[60px] !border-none"><img class="h-full w-full object-contain" src="/sap/details/api//media/storage/0000225382/1740396106410-0000225382.png" automation-id="partnerProfilePage-partnerLogo-img"></div></div><div data-v-9c54a22e="" class="HeadingMediumM-size"><div data-v-9c54a22e="">Contact SEIDOR</div><div data-v-9c54a22e=""></div></div></div></div><form data-v-a57c5c36="" data-v-d7dd9f52="" automation-id="contactEmailForm-contactEmailFormContent"><div data-v-a57c5c36="" class="form-field-wrapper mobile-only"><div data-v-a57c5c36="" class="newsletter-option"><p data-v-a57c5c36="" class="newsletter-label">Stay informed about our updates</p><udex-checkbox data-v-a57c5c36="" id="newsletter" tabindex="-1" automation-id="newsletter-option"><template shadowrootmode="open"></template></udex-checkbox><small data-v-a57c5c36="" class="newsletter-info">You can unsubscribe at any time.</small></div></div><div data-v-a57c5c36="" class="system-field alternate-view"><udex-text-field data-v-a57c5c36="" id="referral_code" name="referral_code" placeholder="Enter your referral code" tabindex="-1" automation-id="referral-code-input"><template shadowrootmode="open"></template></udex-text-field><small data-v-a57c5c36="" class="field-hint">Enter code above</small></div><udex-message-strip data-v-a57c5c36="" status-type="Icon" class="mb-8"><template shadowrootmode="open"></template><div data-v-a57c5c36="" class="flex items-center justify-between gap-4"><span data-v-a57c5c36="" class="body-s-size">We suggest to read some tips before selecting a Partner</span><udex-button data-v-a57c5c36="" class="flex shrink-0 justify-center" id="custom-usage-patterns-forms-learn-more" accessible-name="Read Tips before Selecting a Partner" udex-button=""><template shadowrootmode="open" shadowrootdelegatesfocus=""></template>Read Tips</udex-button></div></udex-message-strip><div data-v-a57c5c36="" class="HeadingMediumXXXS-size mb-4" automation-id="contactEmailFormContent-personalDetailsTitle">Personal Details</div><div data-v-a57c5c36="" class="flex flex-col gap-1.5 max-[850px]:gap-2" automation-id="contactEmailFormContent-personalDetails"><div data-v-a57c5c36="" class="flex justify-between gap-6 max-[850px]:flex-col max-[850px]:gap-2" automation-id="contactEmailFormContent-personalDetailsNames"><udex-text-field data-v-a57c5c36="" value-state="Standard" aria-describedby="firstNameError" supporting-text="" automation-id="contactEmailFormContent-firstName"><template shadowrootmode="open"></template></udex-text-field><udex-text-field data-v-a57c5c36="" value-state="Standard" aria-describedby="lastNameError" supporting-text="" automation-id="contactEmailFormContent-lastName"><template shadowrootmode="open"></template></udex-text-field></div><div data-v-a57c5c36="" class="flex justify-between gap-6 max-[850px]:flex-col max-[850px]:gap-2" automation-id="contactEmailFormContent-personalDetailsContacts"><udex-text-field data-v-a57c5c36="" value-state="Standard" aria-describedby="emailError" supporting-text="" automation-id="contactEmailFormContent-email"><template shadowrootmode="open"></template></udex-text-field><udex-phone-number-field data-v-a57c5c36="" id="udex-phone-number-field" value-state="Standard" automation-id="contactEmailFormContent-phoneNumber"><template shadowrootmode="open"></template></udex-phone-number-field></div></div><div data-v-a57c5c36="" class="HeadingMediumXXXS-size mb-4" automation-id="contactEmailFormContent-companyDetailsTitle">Company Details</div><div data-v-a57c5c36="" class="mb-1 flex flex-col gap-1.5" automation-id="contactEmailFormContent-companyDetails"><div data-v-a57c5c36="" class="flex gap-6 max-[850px]:flex-col max-[850px]:gap-2" automation-id="contactEmailFormContent-companyDetailsInputs"><udex-text-field data-v-a57c5c36="" value-state="Standard" aria-describedby="companyError" supporting-text="" automation-id="contactEmailFormContent-companyName"><template shadowrootmode="open"></template></udex-text-field><udex-country-selector data-v-a57c5c36="" id="country-selector" value-state="Standard" width="380px" search-placeholder="Search Countries and Regions" supporting-text="" automation-id="contactEmailFormContent-countrySelector" value=""><template shadowrootmode="open"></template></udex-country-selector></div><div data-v-a57c5c36="" class="flex gap-6 max-[850px]:flex-col max-[850px]:gap-2" automation-id="contactEmailFormContent-companyDetailsInputs"><udex-select-box data-v-a57c5c36="" value-state="Standard" supporting-text="" value=""><template shadowrootmode="open"></template><udex-select-box-item data-v-a57c5c36=""></udex-select-box-item><udex-select-box-item data-v-a57c5c36=""></udex-select-box-item><udex-select-box-item data-v-a57c5c36=""></udex-select-box-item><udex-select-box-item data-v-a57c5c36=""></udex-select-box-item><udex-select-box-item data-v-a57c5c36=""></udex-select-box-item><udex-select-box-item data-v-a57c5c36=""></udex-select-box-item><udex-select-box-item data-v-a57c5c36=""></udex-select-box-item><udex-select-box-item data-v-a57c5c36=""></udex-select-box-item><udex-select-box-item data-v-a57c5c36=""></udex-select-box-item></udex-select-box></div></div><udex-textarea data-v-a57c5c36="" id="textarea" value-state="Standard" supporting-text="" automation-id="contactEmailFormContent-comments"><template shadowrootmode="open"></template></udex-textarea><div data-v-1f58e182="" data-v-a57c5c36="" class="agreement body-s-size" automation-id="contactEmailFormContent-agreement"><div data-v-1f58e182="" class="checkbox-agreement-wrapper" automation-id="contactEmailForm-checkboxWrapper"><udex-checkbox data-v-1f58e182="" value-state="Standard" automation-id="contactEmailForm-checkbox"><template shadowrootmode="open"></template></udex-checkbox><span data-v-1f58e182="" automation-id="contactEmailForm-agreementText"> I agree that SAP will share this data with    SEIDOR    so that they can contact me regarding my request.  * <udex-link data-v-1f58e182="" automation-id="contactEmailForm-privacyLink"><template shadowrootmode="open"></template>View Privacy Statement.</udex-link></span></div><!----></div><div data-v-a57c5c36="" class="mt-6" automation-id="contactEmailFormContent-recaptcha"></div><!----><udex-button data-v-a57c5c36="" class="mt-6" id="custom-usage-patterns-forms-submit" accessible-name="Contact Sales Submit" udex-button="" automation-id="contactEmailFormContent-submitButton"><template shadowrootmode="open" shadowrootdelegatesfocus=""></template>Send</udex-button></form></div>`;
+
+			dialog.innerHTML = child1 + child2;
+			document.body.appendChild(dialog);
+			dialog.open = true;
+		});
+
 		cbCompact.addEventListener("ui5-change", function () {
 			document.body.classList.toggle("ui5-content-density-compact", cbCompact.checked);
 		});