Skip to content

Commit

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

Fixes: SAP#2537
  • Loading branch information
alexandar-mitsev committed Dec 8, 2020
1 parent 7ef7d80 commit fcdb89c
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 10 deletions.
5 changes: 5 additions & 0 deletions packages/base/src/util/FocusableElements.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
38 changes: 28 additions & 10 deletions packages/main/src/Popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const metadata = {
},

/**
* @private
* @protected
*/
// _disableInitialFocus: {
// type: Boolean,
Expand All @@ -91,6 +91,14 @@ const metadata = {
type: Boolean,
},

/**
* True if the initial focus is already internally applied.
* @private
*/
_focusAppliedAfterOpen: {
type: Boolean,
},

_blockLayerHidden: {
type: Boolean,
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -390,7 +408,7 @@ class Popup extends UI5Element {
this.resetFocus();
}

this._initialFocusApplied = false;
this._focusAppliedAfterOpen = false;

this.fireEvent("after-close", {}, false, false);
}
Expand Down
16 changes: 16 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,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>

Expand Down

0 comments on commit fcdb89c

Please sign in to comment.