From bf8caaf2eb71156e6bbda3c1492022519fe25bf7 Mon Sep 17 00:00:00 2001
From: Petar Dimov <32839090+dimovpetar@users.noreply.github.com>
Date: Thu, 7 Jan 2021 18:35:23 +0200
Subject: [PATCH] fix(ui5-popup): correct focus when there is no focusable
 content (#2583)

---
 packages/main/src/Dialog.js               | 11 ----------
 packages/main/src/Popover.js              | 11 ----------
 packages/main/src/Popup.hbs               |  2 ++
 packages/main/src/Popup.js                | 26 ++++++++++++++++++++---
 packages/main/src/themes/PopupsCommon.css |  1 +
 packages/main/test/pages/Popover.html     | 16 ++++++++++++++
 packages/main/test/specs/Popover.spec.js  | 19 +++++++++++++++++
 7 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/packages/main/src/Dialog.js b/packages/main/src/Dialog.js
index 4f7b17cd874c..101e0c760f20 100644
--- a/packages/main/src/Dialog.js
+++ b/packages/main/src/Dialog.js
@@ -203,17 +203,6 @@ class Dialog extends Popup {
 		return "flex";
 	}
 
-	get classes() {
-		return {
-			root: {
-				"ui5-popup-root": true,
-			},
-			content: {
-				"ui5-popup-content": true,
-			},
-		};
-	}
-
 	show() {
 		super.show();
 		this._center();
diff --git a/packages/main/src/Popover.js b/packages/main/src/Popover.js
index 12ec2757acf8..be710d65b72e 100644
--- a/packages/main/src/Popover.js
+++ b/packages/main/src/Popover.js
@@ -655,17 +655,6 @@ class Popover extends Popup {
 		};
 	}
 
-	get classes() {
-		return {
-			root: {
-				"ui5-popup-root": true,
-			},
-			content: {
-				"ui5-popup-content": true,
-			},
-		};
-	}
-
 	/**
 	 * Hook for descendants to hide header.
 	 */
diff --git a/packages/main/src/Popup.hbs b/packages/main/src/Popup.hbs
index 7d7f1f49d88d..a03f3a45848f 100644
--- a/packages/main/src/Popup.hbs
+++ b/packages/main/src/Popup.hbs
@@ -6,6 +6,8 @@
 	aria-label="{{_ariaLabel}}"
 	aria-labelledby="{{_ariaLabelledBy}}"
 	dir="{{dir}}"
+	tabindex="-1"
+	@keydown={{_onkeydown}}
 >
 
 	<span class="first-fe" data-ui5-focus-trap tabindex="0" @focusin={{forwardToLast}}></span>
diff --git a/packages/main/src/Popup.js b/packages/main/src/Popup.js
index 5931c30119df..1cb5258c2871 100644
--- a/packages/main/src/Popup.js
+++ b/packages/main/src/Popup.js
@@ -3,6 +3,7 @@ import { getRTL } from "@ui5/webcomponents-base/dist/config/RTL.js";
 import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
 import { getFirstFocusableElement, getLastFocusableElement } from "@ui5/webcomponents-base/dist/util/FocusableElements.js";
 import createStyleInHead from "@ui5/webcomponents-base/dist/util/createStyleInHead.js";
+import { isTabPrevious } from "@ui5/webcomponents-base/dist/Keys.js";
 import PopupTemplate from "./generated/templates/PopupTemplate.lit.js";
 import PopupBlockLayer from "./generated/templates/PopupBlockLayerTemplate.lit.js";
 import { getNextZIndex, getFocusedElement, isFocusedElementWithinNode } from "./popup-utils/PopupUtils.js";
@@ -242,6 +243,12 @@ class Popup extends UI5Element {
 		});
 	}
 
+	_onkeydown(e) {
+		if (e.target === this._root && isTabPrevious(e)) {
+			e.preventDefault();
+		}
+	}
+
 	/**
 	 * Focus trapping
 	 * @private
@@ -251,6 +258,8 @@ class Popup extends UI5Element {
 
 		if (firstFocusable) {
 			firstFocusable.focus();
+		} else {
+			this._root.focus();
 		}
 	}
 
@@ -263,6 +272,8 @@ class Popup extends UI5Element {
 
 		if (lastFocusable) {
 			lastFocusable.focus();
+		} else {
+			this._root.focus();
 		}
 	}
 
@@ -284,7 +295,8 @@ class Popup extends UI5Element {
 
 		const element = this.getRootNode().getElementById(this.initialFocus)
 			|| document.getElementById(this.initialFocus)
-			|| await getFirstFocusableElement(this);
+			|| await getFirstFocusableElement(this)
+			|| this._root; // in case of no focusable content focus the root
 
 		if (element) {
 			element.focus();
@@ -468,6 +480,10 @@ class Popup extends UI5Element {
 		return this.ariaLabel || undefined;
 	}
 
+	get _root() {
+		return this.shadowRoot.querySelector(".ui5-popup-root");
+	}
+
 	get dir() {
 		return getRTL() ? "rtl" : "ltr";
 	}
@@ -484,8 +500,12 @@ class Popup extends UI5Element {
 
 	get classes() {
 		return {
-			root: {},
-			content: {},
+			root: {
+				"ui5-popup-root": true,
+			},
+			content: {
+				"ui5-popup-content": true,
+			},
 		};
 	}
 }
diff --git a/packages/main/src/themes/PopupsCommon.css b/packages/main/src/themes/PopupsCommon.css
index 091cbb06a80d..33a5ee9af43b 100644
--- a/packages/main/src/themes/PopupsCommon.css
+++ b/packages/main/src/themes/PopupsCommon.css
@@ -20,6 +20,7 @@
     overflow: hidden;
     max-height: 94vh;
     max-width: 90vw;
+    outline: none;
 }
 
 @media screen and (-ms-high-contrast: active) {
diff --git a/packages/main/test/pages/Popover.html b/packages/main/test/pages/Popover.html
index a54fb1b8b138..6df3f5c34abb 100644
--- a/packages/main/test/pages/Popover.html
+++ b/packages/main/test/pages/Popover.html
@@ -371,6 +371,12 @@
 	<br>
 	<br>
 
+	<ui5-button id="firstBtn">First button</ui5-button>
+	<ui5-button id="secondBtn">Second button</ui5-button>
+	<ui5-popover id="popNoFocusableContent" placement-type="Bottom">Sample text for the ui5-popover</ui5-popover>
+
+	<br>
+
 	<script>
 		btn.addEventListener("click", function (event) {
 			pop.openBy(btn);
@@ -451,6 +457,16 @@
 		btnOpenXRight.addEventListener("click", function (event) {
 			popXRight.openBy(targetOpener2);
 		});
+
+		firstBtn.addEventListener("click", (event) => {
+			popNoFocusableContent.innerHTML = "First button is clicked";
+			popNoFocusableContent.openBy(event.target);
+		});
+
+		secondBtn.addEventListener("click", (event) => {
+			popNoFocusableContent.innerHTML = "Second button is clicked";
+			popNoFocusableContent.openBy(event.target);
+		});
 	</script>
 </body>
 
diff --git a/packages/main/test/specs/Popover.spec.js b/packages/main/test/specs/Popover.spec.js
index ce762f5cebb6..1a90dbb1d623 100644
--- a/packages/main/test/specs/Popover.spec.js
+++ b/packages/main/test/specs/Popover.spec.js
@@ -231,6 +231,25 @@ describe("Popover general interaction", () => {
 
 		assert.ok(ff.getProperty("focused"), "The first focusable element is focused.");
 	});
+
+	it("tests focus when there is no focusable content", () => {
+		browser.url("http://localhost:8080/test-resources/pages/Popover.html");
+
+		const firstBtn = $("#firstBtn");
+		const popoverId = "popNoFocusableContent";
+
+		firstBtn.click();
+
+		let activeElementId = $(browser.getActiveElement()).getAttribute("id");
+
+		assert.strictEqual(activeElementId, popoverId, "Popover is focused");
+
+		browser.keys(["Shift", "Tab"]);
+
+		activeElementId = $(browser.getActiveElement()).getAttribute("id");
+
+		assert.ok(activeElementId, popoverId, "Popover remains focused");
+	});
 });
 
 describe("Acc", () => {