From cce46b8f4ff4c125129f2178999f9a4f2bca8583 Mon Sep 17 00:00:00 2001 From: Alexandar Mitsev Date: Mon, 14 Dec 2020 14:29:41 +0200 Subject: [PATCH] fix(ui5-dialog): apply initial focus after rendering (#2551) Now the initial focus is applied also in cases when the dialog is open before it is fully rendered. Fixes: #2537 --- packages/base/src/UI5Element.js | 10 ++++++++++ packages/base/src/util/FocusableElements.js | 17 ++++++++++++----- packages/main/src/Popup.js | 18 ++++++++++-------- packages/main/test/pages/Dialog.html | 21 +++++++++++++++++++++ packages/main/test/specs/Dialog.spec.js | 14 ++++++++++++++ 5 files changed, 67 insertions(+), 13 deletions(-) diff --git a/packages/base/src/UI5Element.js b/packages/base/src/UI5Element.js index ef51a6cd1f21..9fb75c58e4e1 100644 --- a/packages/base/src/UI5Element.js +++ b/packages/base/src/UI5Element.js @@ -638,6 +638,16 @@ class UI5Element extends HTMLElement { } } + /** + * Waits for dom ref and then returns the DOM Element marked with "data-sap-focus-ref" inside the template. + * This is the element that will receive the focus by default. + * @public + */ + async getFocusDomRefAsync() { + await this._waitForDomRef(); + return this.getFocusDomRef(); + } + /** * Use this method in order to get a reference to an element in the shadow root of the web component or the static area item of the component * @public diff --git a/packages/base/src/util/FocusableElements.js b/packages/base/src/util/FocusableElements.js index c335a31553f4..d07c35a26eec 100644 --- a/packages/base/src/util/FocusableElements.js +++ b/packages/base/src/util/FocusableElements.js @@ -5,7 +5,7 @@ const isFocusTrap = el => { return el.hasAttribute("data-ui5-focus-trap"); }; -const getFirstFocusableElement = container => { +const getFirstFocusableElement = async container => { if (!container || isNodeHidden(container)) { return null; } @@ -13,7 +13,7 @@ const getFirstFocusableElement = container => { return findFocusableElement(container, true); }; -const getLastFocusableElement = container => { +const getLastFocusableElement = async container => { if (!container || isNodeHidden(container)) { return null; } @@ -21,7 +21,7 @@ const getLastFocusableElement = container => { return findFocusableElement(container, false); }; -const findFocusableElement = (container, forward) => { +const findFocusableElement = async (container, forward) => { let child; if (container.shadowRoot) { @@ -35,10 +35,15 @@ const findFocusableElement = (container, forward) => { let focusableDescendant; + /* eslint-disable no-await-in-loop */ + while (child) { const originalChild = child; - child = child.isUI5Element ? child.getFocusDomRef() : child; + if (child.isUI5Element) { + child = await child.getFocusDomRefAsync(); + } + if (!child) { return null; } @@ -48,7 +53,7 @@ const findFocusableElement = (container, forward) => { return (child && typeof child.focus === "function") ? child : null; } - focusableDescendant = findFocusableElement(child, forward); + focusableDescendant = await findFocusableElement(child, forward); if (focusableDescendant) { return (focusableDescendant && typeof focusableDescendant.focus === "function") ? focusableDescendant : null; } @@ -57,6 +62,8 @@ const findFocusableElement = (container, forward) => { child = forward ? originalChild.nextSibling : originalChild.previousSibling; } + /* eslint-enable no-await-in-loop */ + return null; }; diff --git a/packages/main/src/Popup.js b/packages/main/src/Popup.js index 7a6052312953..5931c30119df 100644 --- a/packages/main/src/Popup.js +++ b/packages/main/src/Popup.js @@ -246,8 +246,8 @@ class Popup extends UI5Element { * Focus trapping * @private */ - forwardToFirst() { - const firstFocusable = getFirstFocusableElement(this); + async forwardToFirst() { + const firstFocusable = await getFirstFocusableElement(this); if (firstFocusable) { firstFocusable.focus(); @@ -258,8 +258,8 @@ class Popup extends UI5Element { * Focus trapping * @private */ - forwardToLast() { - const lastFocusable = getLastFocusableElement(this); + async forwardToLast() { + const lastFocusable = await getLastFocusableElement(this); if (lastFocusable) { lastFocusable.focus(); @@ -270,8 +270,8 @@ class Popup extends UI5Element { * Use this method to focus the element denoted by "initialFocus", if provided, or the first focusable element otherwise. * @protected */ - applyInitialFocus() { - this.applyFocus(); + async applyInitialFocus() { + await this.applyFocus(); } /** @@ -279,10 +279,12 @@ class Popup extends UI5Element { * or the first focusable element otherwise. * @public */ - applyFocus() { + async applyFocus() { + await this._waitForDomRef(); + const element = this.getRootNode().getElementById(this.initialFocus) || document.getElementById(this.initialFocus) - || getFirstFocusableElement(this); + || await getFirstFocusableElement(this); if (element) { element.focus(); diff --git a/packages/main/test/pages/Dialog.html b/packages/main/test/pages/Dialog.html index 32f1c118cca5..c2cb24bec0f8 100644 --- a/packages/main/test/pages/Dialog.html +++ b/packages/main/test/pages/Dialog.html @@ -50,6 +50,9 @@

Open resizable dialog +
+
+ Open dialog which is created dynamically @@ -330,6 +333,24 @@ 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 () { + var dialog = document.createElement("ui5-dialog"), + button = document.createElement("ui5-button"); + + button.setAttribute("id", "dynamic-dialog-close-button"); + button.appendChild(document.createTextNode("Close")); + button.addEventListener("click", function () { + dialog.close(); + }); + + dialog.setAttribute("id", "dynamic-dialog"); + dialog.appendChild(button); + + document.body.appendChild(dialog); + + dialog.open(); + }); diff --git a/packages/main/test/specs/Dialog.spec.js b/packages/main/test/specs/Dialog.spec.js index 7806d2706bd0..6f6c15b35405 100644 --- a/packages/main/test/specs/Dialog.spec.js +++ b/packages/main/test/specs/Dialog.spec.js @@ -113,6 +113,20 @@ describe("Dialog general interaction", () => { closeResizableDialogButton.click(); }); + + it("initial focus after dynamic dialog creation", () => { + const openDynamicDialog = browser.$("#dynamic-open"); + openDynamicDialog.click(); + + const closeButton = browser.$("#dynamic-dialog-close-button"); + + browser.pause(500); + + const activeElement = $(browser.getActiveElement()); + assert.strictEqual(activeElement.getProperty("id"), closeButton.getProperty("id"), "the active element is the close button"); + + closeButton.click(); + }); });