Skip to content

Commit

Permalink
fix(ui5-popup): prevent focus on elements below block layer (#2800)
Browse files Browse the repository at this point in the history
Problem: When element inside the popup, which can't get focus is clicked, the focus goes on the body element.
Then any element below the block layer can be focused.
Solution: Don't let the focus leave the popup.

Fixes: #2626
  • Loading branch information
dimovpetar authored Feb 11, 2021
1 parent 747738e commit f2f3889
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 0 deletions.
1 change: 1 addition & 0 deletions packages/main/src/Popup.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
dir="{{dir}}"
tabindex="-1"
@keydown={{_onkeydown}}
@focusout={{_onfocusout}}
>

<span class="first-fe" data-ui5-focus-trap tabindex="0" @focusin={{forwardToLast}}></span>
Expand Down
7 changes: 7 additions & 0 deletions packages/main/src/Popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,13 @@ class Popup extends UI5Element {
}
}

_onfocusout(e) {
// relatedTarget is the element, which will get focus. If no such element exists, focus the root
if (!e.relatedTarget) {
this._root.focus();
}
}

/**
* Focus trapping
* @private
Expand Down
14 changes: 14 additions & 0 deletions packages/main/test/pages/Popover.html
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,15 @@

<br>

<ui5-button id="btnOpenPopoverWithDiv">Open Popover with div Inside</ui5-button>

<ui5-popover id="popWithDiv" header-text="Newsletter subscription" modal>
<div id="divContent">I'm a div, which can't get focus. Click me and see where focus goes.</div>
<div slot="footer" class="popover-footer">
<ui5-button design="Emphasized">Subscribe</ui5-button>
</div>
</ui5-popover>

<script>
btn.addEventListener("click", function (event) {
pop.openBy(btn);
Expand Down Expand Up @@ -488,6 +497,11 @@

popover.openBy(btnOpenDynamic);
});

btnOpenPopoverWithDiv.addEventListener("click", function (event) {
popWithDiv.openBy(event.target);
});

</script>
</body>

Expand Down
12 changes: 12 additions & 0 deletions packages/main/test/specs/Popover.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,18 @@ describe("Popover general interaction", () => {
assert.ok(activeElementId, popoverId, "Popover remains focused");
});

it("tests focus when content, which can't be focused is clicked", () => {
browser.url("http://localhost:8080/test-resources/pages/Popover.html");

$("#btnOpenPopoverWithDiv").click();
$("#divContent").click();

const popoverId = "popWithDiv";
const activeElementId = $(browser.getActiveElement()).getAttribute("id");

assert.strictEqual(activeElementId, popoverId, "Popover is focused");
});

it("tests that dynamically created popover is opened", () => {
browser.url("http://localhost:8080/test-resources/pages/Popover.html");

Expand Down

0 comments on commit f2f3889

Please sign in to comment.