From fcdb89cc5e301217617e3c1e70c72cd820671fe4 Mon Sep 17 00:00:00 2001
From: Alexandar Mitsev <alexandar.mitsev@sap.com>
Date: Mon, 7 Dec 2020 18:30:16 +0200
Subject: [PATCH] fix(ui5-dialog): apply initial focus after rendering

Now the initial focus is applied also in cases when the dialog
is opened before it is fully rendered.

Fixes: #2537
---
 packages/base/src/util/FocusableElements.js |  5 +++
 packages/main/src/Popup.js                  | 38 +++++++++++++++------
 packages/main/test/pages/Dialog.html        | 16 +++++++++
 3 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/packages/base/src/util/FocusableElements.js b/packages/base/src/util/FocusableElements.js
index c335a31553f4..6df5a1d7d046 100644
--- a/packages/base/src/util/FocusableElements.js
+++ b/packages/base/src/util/FocusableElements.js
@@ -38,6 +38,11 @@ const findFocusableElement = (container, forward) => {
 	while (child) {
 		const originalChild = child;
 
+		// @todo discuss
+		if (child.isUI5Element) {
+			return child;
+		}
+
 		child = child.isUI5Element ? child.getFocusDomRef() : child;
 		if (!child) {
 			return null;
diff --git a/packages/main/src/Popup.js b/packages/main/src/Popup.js
index 93020fd75081..bb309b522f0f 100644
--- a/packages/main/src/Popup.js
+++ b/packages/main/src/Popup.js
@@ -77,7 +77,7 @@ const metadata = {
 		},
 
 		/**
-		 * @private
+		 * @protected
 		 */
 		// _disableInitialFocus: {
 		// 	type: Boolean,
@@ -91,6 +91,14 @@ const metadata = {
 			type: Boolean,
 		},
 
+		/**
+		 * True if the initial focus is already internally applied.
+		 * @private
+		 */
+		_focusAppliedAfterOpen: {
+			type: Boolean,
+		},
+
 		_blockLayerHidden: {
 			type: Boolean,
 		},
@@ -297,6 +305,23 @@ class Popup extends UI5Element {
 		}
 	}
 
+	/**
+	 * @private
+	 */
+	_applyFocusAfterOpen() {
+		const shouldApply = !(this._focusAppliedAfterOpen || this._disableInitialFocus /* || this._preventInitialFocus */);
+
+		if (this.isOpen() && shouldApply) {
+
+			// @todo discuss
+			// setTimeout(() => {
+				this.applyInitialFocus();
+			// }, 0);
+
+			this._focusAppliedAfterOpen = true;
+		}
+	}
+
 	/**
 	 * Override this method to provide custom logic for the popup's open/closed state. Maps to the "opened" property by default.
 	 * @public
@@ -342,14 +367,7 @@ class Popup extends UI5Element {
 	}
 
 	onAfterRendering() {
-		if (this.opened &&
-			// !this._disableInitialFocus &&
-			// !this._preventInitialFocus &&
-			!this._initialFocusApplied) {
-
-			this.applyInitialFocus();
-			this._initialFocusApplied = true;
-		}
+		this._applyFocusAfterOpen();
 	}
 
 	/**
@@ -390,7 +408,7 @@ class Popup extends UI5Element {
 			this.resetFocus();
 		}
 
-		this._initialFocusApplied = false;
+		this._focusAppliedAfterOpen = false;
 
 		this.fireEvent("after-close", {}, false, false);
 	}
diff --git a/packages/main/test/pages/Dialog.html b/packages/main/test/pages/Dialog.html
index 32f1c118cca5..6011b8a6fcfc 100644
--- a/packages/main/test/pages/Dialog.html
+++ b/packages/main/test/pages/Dialog.html
@@ -50,6 +50,9 @@
 	<br>
 	<br>
 	<ui5-button id="resizable-open">Open resizable dialog</ui5-button>
+	<br>
+	<br>
+	<ui5-button id="dynamic-open">Open dialog which is created dynamically</ui5-button>
 
 	<ui5-block-layer></ui5-block-layer>
 
@@ -330,6 +333,19 @@
 		window["draggable-close"].addEventListener("click", function () { window["draggable-dialog"].close(); });
 		window["resizable-open"].addEventListener("click", function () { window["resizable-dialog"].open(); });
 		window["resizable-close"].addEventListener("click", function () { window["resizable-dialog"].close(); });
+
+
+		window["dynamic-open"].addEventListener("click", function () {
+			let dialog = document.createElement("ui5-dialog"),
+				button = document.createElement("ui5-button");
+
+			button.appendChild(document.createTextNode("Ok"));
+			dialog.appendChild(button);
+
+			document.body.appendChild(dialog);
+
+			dialog.open();
+		});
 	</script>
 </body>