Skip to content

Commit

Permalink
refactor(ui5-popover): Keep popup open if focus is inside (#1937)
Browse files Browse the repository at this point in the history
Another PR to address the "QuickCardView" topic. The change keeps the Popover open, without an opener, if the focus is inside that popover.

add prevent-focus-restore protected property in addition to the preventFocusRestore param of Popup.prototype.close method, because the quickCardView might close due to user interaction - click, TAB, ESC.
remove the recently added _closeWithOpener param, as now the popover without an opener would remain open if the focus is inside it and will be closed otherwise.
QuickCardView Interaction (test it on http://localhost:8080/test-resources/pages/Input_quickview.html)

Once opened, both Suggestions Popover and QuickCardView will close if:
neither the SearchField nor the QuickCardView has the focus, because the user clicks somewhere outside or
the user uses the TAB | SHIFT + TAB and the focus is somewhere else

Once opened, the QuickCardView remains open and Suggestions Popover closes if:
the focus moves to the QuickCardView, because the Popover.prototype.applyFocus method is called,
the user clicks inside the QuickCardView or the user uses the TAB | SHIFT + TAB key and the focus is goes inside the QuickCardView

Closes: #1768
  • Loading branch information
ilhan007 authored Jul 17, 2020
1 parent 0934f0c commit 45cdcc2
Show file tree
Hide file tree
Showing 11 changed files with 137 additions and 54 deletions.
12 changes: 9 additions & 3 deletions packages/main/src/Input.js
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ class Input extends UI5Element {
}

if (this.popover) {
this.popover.close(false, false, true);
this.popover.close();
}

this.previousValue = "";
Expand Down Expand Up @@ -898,13 +898,19 @@ class Input extends UI5Element {
onItemMouseOver(event) {
const item = event.target;
const suggestion = this.getSuggestionByListItem(item);
suggestion && suggestion.fireEvent("mouseover", { targetRef: item });
suggestion && suggestion.fireEvent("mouseover", {
item: suggestion,
targetRef: item,
});
}

onItemMouseOut(event) {
const item = event.target;
const suggestion = this.getSuggestionByListItem(item);
suggestion && suggestion.fireEvent("mouseout", { targetRef: item });
suggestion && suggestion.fireEvent("mouseout", {
item: suggestion,
targetRef: item,
});
}

onItemSelected(item, keyboardUsed) {
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/InputPopover.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
<ui5-popover
skip-registry-update
_disable-initial-focus
prevent-focus-restore
no-padding
no-arrow
class="ui5-valuestatemessage-popover"
Expand Down
15 changes: 5 additions & 10 deletions packages/main/src/Popover.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,16 +253,14 @@ class Popover extends Popup {
* Opens the popover.
* @param {HTMLElement} opener the element that the popover is opened by
* @param {boolean} preventInitialFocus prevents applying the focus inside the popover
* @param {boolean} closeWithOpener defines if the popover would closes when its opener is no longer visible (true by default)
* @public
*/
openBy(opener, preventInitialFocus = false, closeWithOpener = true) {
openBy(opener, preventInitialFocus = false) {
if (!opener || this.opened) {
return;
}

this._opener = opener;
this._closeWithOpener = closeWithOpener;

super.open(preventInitialFocus);
}
Expand Down Expand Up @@ -321,8 +319,9 @@ class Popover extends Popup {
const popoverSize = this.popoverSize;
const openerRect = this._opener.getBoundingClientRect();

if (!this._closeWithOpener && this.shouldCloseDueToNoOpener(openerRect)) {
// use the old placement when the opener is gone
if (this.shouldCloseDueToNoOpener(openerRect) && this.isFocusWithin()) {
// reuse the old placement as the opener is not available,
// but keep the popover open as the focus is within
placement = this._oldPlacement;
} else {
placement = this.calcPlacement(openerRect, popoverSize);
Expand Down Expand Up @@ -407,11 +406,7 @@ class Popover extends Popup {

const placementType = this.getActualPlacementType(targetRect, popoverSize);

if (!this._closeWithOpener) {
this._preventRepositionAndClose = this.shouldCloseDueToNoOpener(targetRect) || this.shouldCloseDueToOverflow(placementType, targetRect);
} else {
this._preventRepositionAndClose = false;
}
this._preventRepositionAndClose = this.shouldCloseDueToNoOpener(targetRect) || this.shouldCloseDueToOverflow(placementType, targetRect);

const isVertical = placementType === PopoverPlacementType.Top
|| placementType === PopoverPlacementType.Bottom;
Expand Down
22 changes: 20 additions & 2 deletions packages/main/src/Popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { getFirstFocusableElement, getLastFocusableElement } from "@ui5/webcompo
import createStyleInHead from "@ui5/webcomponents-base/dist/util/createStyleInHead.js";
import PopupTemplate from "./generated/templates/PopupTemplate.lit.js";
import PopupBlockLayer from "./generated/templates/PopupBlockLayerTemplate.lit.js";
import { getNextZIndex, getFocusedElement } from "./popup-utils/PopupUtils.js";
import { getNextZIndex, getFocusedElement, isFocusedElementWithinNode } from "./popup-utils/PopupUtils.js";
import { addOpenedPopup, removeOpenedPopup } from "./popup-utils/OpenedPopupsRegistry.js";

// Styles
Expand Down Expand Up @@ -40,9 +40,23 @@ const metadata = {
type: String,
},

/**
* Defines if the focus should be returned to the previously focused element,
* when the popup closes.
* @type {boolean}
* @defaultvalue false
* @public
* @since 1.0.0-rc.8
*/
preventFocusRestore: {
type: Boolean,
},

/**
* Indicates if the elements is open
* @private
* @type {boolean}
* @defaultvalue false
*/
opened: {
type: Boolean,
Expand Down Expand Up @@ -273,6 +287,10 @@ class Popup extends UI5Element {
return this.opened;
}

isFocusWithin() {
return isFocusedElementWithinNode(this.shadowRoot.querySelector(".ui5-popup-root"));
}

/**
* Shows the block layer (for modal popups only) and sets the correct z-index for the purpose of popup stacking
* @param {boolean} preventInitialFocus prevents applying the focus inside the popup
Expand Down Expand Up @@ -333,7 +351,7 @@ class Popup extends UI5Element {
this._removeOpenedPopup();
}

if (!preventFocusRestore) {
if (!this.preventFocusRestore && !preventFocusRestore) {
this.resetFocus();
}

Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/TextArea.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ class TextArea extends UI5Element {

async closePopover() {
this.popover = await this._getPopover();
this.popover && this.popover.close(false, false, true);
this.popover && this.popover.close();
}

async _getPopover() {
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/TextAreaPopover.hbs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{{#if displayValueStateMessagePopover}}
<ui5-popover
skip-registry-update
prevent-focus-restore
no-padding
no-arrow
_disable-initial-focus
Expand Down
4 changes: 4 additions & 0 deletions packages/main/src/popup-utils/OpenedPopupsRegistry.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ const getOpenedPopups = () => {
};

const _keydownListener = event => {
if (!openedRegistry.length) {
return;
}

if (isEscape(event)) {
openedRegistry.pop().instance.close(true);
}
Expand Down
29 changes: 29 additions & 0 deletions packages/main/src/popup-utils/PopupUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,34 @@ const getFocusedElement = () => {
return (element && typeof element.focus === "function") ? element : null;
};

const isFocusedElementWithinNode = node => {
const fe = getFocusedElement();

if (fe) {
return isNodeContainedWithin(node, fe);
}

return false;
};

const isNodeContainedWithin = (parent, child) => {
let currentNode = parent;

if (currentNode.shadowRoot) {
currentNode = Array.from(currentNode.shadowRoot.children).find(n => n.localName !== "style");
}

if (currentNode === child) {
return true;
}

const childNodes = currentNode.localName === "slot" ? currentNode.assignedNodes() : currentNode.children;

if (childNodes) {
return Array.from(childNodes).some(n => isNodeContainedWithin(n, child));
}
};

const isPointInRect = (x, y, rect) => {
return x >= rect.left && x <= rect.right
&& y >= rect.top && y <= rect.bottom;
Expand Down Expand Up @@ -52,4 +80,5 @@ export {
isClickInRect,
getClosedPopupParent,
getNextZIndex,
isFocusedElementWithinNode,
};
24 changes: 8 additions & 16 deletions packages/main/test/pages/Input.html
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ <h3>Test ariaLabel and ariaLabelledBy</h3>
inputPreview.addEventListener("ui5-suggestion-item-preview", function (event) {
var item = event.detail.targetRef;
quickViewCard.close();
quickViewCard.openBy(item, true /* preventInitialFocus */, false /* closeWithOpener */);
quickViewCard.openBy(item, true /* preventInitialFocus */);

// log info
inputItemPreviewRes.value = item.textContent;
Expand All @@ -449,36 +449,28 @@ <h3>Test ariaLabel and ariaLabelledBy</h3>
el.addEventListener("mouseover", function (event) {
const targetRef = event.detail.targetRef;
quickViewCard.close();
quickViewCard.openBy(targetRef, true /* preventInitialFocus */, false /* closeWithOpener */);
quickViewCard.openBy(targetRef, true /* preventInitialFocus */);

// log info
mouseoverResult.value = targetRef.textContent;
console.log("mouseover");
});

el.addEventListener("mouseout", function (event) {
// if (!focusQuickView) {
// quickViewCard.close(false, false, true);
// }

// focusQuickView = false;

// // log info
// mouseoutResult.value = event.detail.targetRef.textContent;
// console.log("mouseout");
});
});

inputPreview.addEventListener("ui5-suggestion-scroll", function (event) {
console.log("scroll", { scrolltop: event.detail.scrollTop });
});

inputPreview.addEventListener("focusin", function (event) {
console.log("focusin");
});
quickViewCard.addEventListener("ui5-before-close", async event => {
const esc = event.detail.escPressed;

inputPreview.addEventListener("focusout", function (event) {
console.log("focusout");
if (esc) {
await RenderScheduler.whenFinished();
inputPreview.focus();
}
});

scrollInput.addEventListener("ui5-suggestion-scroll", function (event) {
Expand Down
62 changes: 41 additions & 21 deletions packages/main/test/pages/Input_quickview.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,23 @@ <h1> Quick View sample</h1>
<li>navigate via the arrows to see quick view</li>
<li>press [ctrl + shift + 1] to enter the quick view</li>
</ul>
<ui5-input id="inputPreview" show-suggestions style="width: 200px; margin-top: 50px">

<div style="display: flex; align-items: center">
<ui5-label>focusable element: </ui5-label><ui5-button>before</ui5-button>
</div>
<br>
<br>

<ui5-input id="inputPreview" show-suggestions style="width: 200px;">
<ui5-suggestion-item class="suggestionItem" text="Laptop Lenovo"></ui5-suggestion-item>
<ui5-suggestion-item class="suggestionItem" text="HP Monitor 24"></ui5-suggestion-item>
<ui5-suggestion-item class="suggestionItem" text="IPhone 6s"></ui5-suggestion-item>
<ui5-suggestion-item class="suggestionItem" text="Dell"></ui5-suggestion-item>
<ui5-suggestion-item class="suggestionItem" text="IPad Air"></ui5-suggestion-item>
</ui5-input>

<ui5-popover id="quickViewCard" no-arrow placement-type="Right" height="500px">
<ui5-popover id="quickViewCard" no-arrow placement-type="Right" height="500px" prevent-focus-restore>
<button>hello</button>
<ui5-input id="searchInput" style="width: 300px">
<ui5-icon id="searchIcon" slot="icon" name="search"></ui5-icon>
</ui5-input>
Expand All @@ -42,23 +50,43 @@ <h1> Quick View sample</h1>
</ui5-list>
</ui5-popover>

<br>
<br>
<div style="display: flex; align-items: center">
<ui5-label>focusable element: </ui5-label><ui5-button>after</ui5-button>
</div>
<script>
var focusQuickView = false;

/*
* Open quick view on suggestion-item-preview
* Open quickviewCard on suggestion-item-preview
*/
inputPreview.addEventListener("suggestion-item-preview", function (event) {
const targetRef = event.detail.targetRef;

quickViewCard.close();
quickViewCard.openBy(targetRef, true /* preventInitialFocus */, false /* closeWithOpener */);
quickViewCard.openBy(targetRef, true /* preventInitialFocus */);
});

/*
* Toggle quickviewCard on mouseover/mouseout
*/
[].slice.call(document.querySelectorAll(".suggestionItem")).forEach(function(el) {
el.addEventListener("mouseover", function (event) {
const targetRef = event.detail.targetRef;

quickViewCard.close();
quickViewCard.openBy(targetRef, true /* preventInitialFocus */);
});

el.addEventListener("mouseout", function (event) {
});
});

/*
* Focus quick view on [ctrl + shift + 1]
* Focus quickviewCard on [ctrl + shift + 1]
*/
inputPreview.addEventListener("keyup", async function (event) {
inputPreview.addEventListener("keyup", async event => {
const combination = event.key === "1" && event.ctrlKey && event.shiftKey;

if (combination) {
Expand All @@ -69,23 +97,15 @@ <h1> Quick View sample</h1>
});

/*
* Toggle quick view on mouseover/mouseout
* Restore the focus to the input on ESC
*/
[].slice.call(document.querySelectorAll(".suggestionItem")).forEach(function(el) {
el.addEventListener("mouseover", function (event) {
const targetRef = event.detail.targetRef;

quickViewCard.close();
quickViewCard.openBy(targetRef, true /* preventInitialFocus */, false /* closeWithOpener */);
});

el.addEventListener("mouseout", function (event) {
// if (!focusQuickView) {
// quickViewCard.close(false, false, true);
// }
quickViewCard.addEventListener("before-close", async event => {
const esc = event.detail.escPressed;

// focusQuickView = false;
});
if (esc) {
await RenderScheduler.whenFinished();
inputPreview.focus();
}
});
</script>
</body>
Expand Down
19 changes: 18 additions & 1 deletion packages/main/test/specs/Input.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,28 @@ describe("Input general interaction", () => {
it("fires suggestion-item-preview", () => {
const inputItemPreview = $("#inputPreview").shadow$("input");
const inputItemPreviewRes = $("#inputItemPreviewRes");
const EXPECTED_PREVIEW_ITEM_TEXT = "Laptop Lenovo";

// act
inputItemPreview.click();
inputItemPreview.keys("ArrowDown");

// assert
const staticAreaItemClassName = browser.getStaticAreaItemClassName("#inputPreview");
const inputPopover = browser.$(`.${staticAreaItemClassName}`).shadow$("ui5-responsive-popover");
const helpPopover = browser.$("#quickViewCard");

assert.strictEqual(inputItemPreviewRes.getValue(), EXPECTED_PREVIEW_ITEM_TEXT, "First item has been previewed");
assert.ok(helpPopover.isDisplayedInViewport(), "The help popover is open.");
assert.ok(inputPopover.isDisplayedInViewport(), "The input popover is open.");

assert.strictEqual(inputItemPreviewRes.getValue(), "Laptop Lenovo", "First item has been previewed");
// act
const inputInHelpPopover = browser.$("#searchInput").shadow$("input");
inputInHelpPopover.click();

// assert
assert.notOk(inputPopover.isDisplayedInViewport(), "The inpuit popover is closed as it lost the focus.");
assert.ok(helpPopover.isDisplayedInViewport(), "The help popover remains open as the focus is within.");
});

it("fires suggestion-scroll event", () => {
Expand Down

0 comments on commit 45cdcc2

Please sign in to comment.