Skip to content

Commit

Permalink
fix(ui5-dialog): apply initial focus after rendering (SAP#2551)
Browse files Browse the repository at this point in the history
Now the initial focus is applied also in cases when the dialog is open before it is fully rendered.

Fixes: SAP#2537
  • Loading branch information
alexandar-mitsev authored and niyap committed Dec 17, 2020
1 parent 2471de3 commit cce46b8
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 13 deletions.
10 changes: 10 additions & 0 deletions packages/base/src/UI5Element.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 12 additions & 5 deletions packages/base/src/util/FocusableElements.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,23 @@ const isFocusTrap = el => {
return el.hasAttribute("data-ui5-focus-trap");
};

const getFirstFocusableElement = container => {
const getFirstFocusableElement = async container => {
if (!container || isNodeHidden(container)) {
return null;
}

return findFocusableElement(container, true);
};

const getLastFocusableElement = container => {
const getLastFocusableElement = async container => {
if (!container || isNodeHidden(container)) {
return null;
}

return findFocusableElement(container, false);
};

const findFocusableElement = (container, forward) => {
const findFocusableElement = async (container, forward) => {
let child;

if (container.shadowRoot) {
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -57,6 +62,8 @@ const findFocusableElement = (container, forward) => {
child = forward ? originalChild.nextSibling : originalChild.previousSibling;
}

/* eslint-enable no-await-in-loop */

return null;
};

Expand Down
18 changes: 10 additions & 8 deletions packages/main/src/Popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -270,19 +270,21 @@ 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();
}

/**
* Focuses the element denoted by <code>initialFocus</code>, if provided,
* 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();
Expand Down
21 changes: 21 additions & 0 deletions packages/main/test/pages/Dialog.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -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();
});
</script>
</body>

Expand Down
14 changes: 14 additions & 0 deletions packages/main/test/specs/Dialog.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});


Expand Down

0 comments on commit cce46b8

Please sign in to comment.