fix(ui5-busyindicator): focus handling improvements (#3189)

Should fix #3171 as well. This has to be tested after a release is made.

Author: dimovpetar

packages/base/hash.txt

@@ -1 +1 @@
-52riaJNf3/W5XsbYKCniEmwezbc=
+VtcT3O0SUP+WFCXL1txQutIxxPY=

packages/base/src/util/FocusableElements.js

@@ -21,6 +21,10 @@ const getLastFocusableElement = async container => {
 	return findFocusableElement(container, false);
 };
 
+const isElemFocusable = el => {
+	return el.hasAttribute("data-ui5-focus-redirect") || !isNodeHidden(el);
+};
+
 const findFocusableElement = async (container, forward) => {
 	let child;
 
@@ -48,7 +52,7 @@ const findFocusableElement = async (container, forward) => {
 			return null;
 		}
 
-		if (child.nodeType === 1 && !isNodeHidden(child) && !isFocusTrap(child)) {
+		if (child.nodeType === 1 && isElemFocusable(child) && !isFocusTrap(child)) {
 			if (isNodeClickable(child)) {
 				return (child && typeof child.focus === "function") ? child : null;
 			}

packages/main/src/BusyIndicator.hbs

@@ -23,5 +23,9 @@
 		</div>
 	{{/if}}
 
-	<slot tabindex="{{slotTabIndex}}"></slot>
+	<slot></slot>
+
+	{{#if active}}
+		<span data-ui5-focus-redirect tabindex="0" @focusin="{{_redirectFocus}}"></span>
+	{{/if}}
 </div>

packages/main/src/BusyIndicator.js

@@ -2,6 +2,7 @@ import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
 import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js";
 import { isIE } from "@ui5/webcomponents-base/dist/Device.js";
 import { fetchI18nBundle, getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js";
+import { isTabNext } from "@ui5/webcomponents-base/dist/Keys.js";
 import BusyIndicatorSize from "./types/BusyIndicatorSize.js";
 import Label from "./Label.js";
 
@@ -127,22 +128,22 @@ class BusyIndicator extends UI5Element {
 		super();
 
 		this.i18nBundle = getI18nBundle("@ui5/webcomponents");
-		this._preventHandler = this._preventEvent.bind(this);
+		this._keydownHandler = this._handleKeydown.bind(this);
+		this._preventEventHandler = this._preventEvent.bind(this);
 	}
 
 	onEnterDOM() {
-		this.addEventListener("keyup", this._preventHandler, {
+		this.addEventListener("keydown", this._keydownHandler, {
 			capture: true,
 		});
-
-		this.addEventListener("keydown", this._preventHandler, {
+		this.addEventListener("keyup", this._preventEventHandler, {
 			capture: true,
 		});
 	}
 
 	onExitDOM() {
-		this.removeEventListener("keyup", this._preventHandler, true);
-		this.removeEventListener("keydown", this._preventHandler, true);
+		this.removeEventListener("keydown", this._keydownHandler, true);
+		this.removeEventListener("keyup", this._preventEventHandler, true);
 	}
 
 	static get metadata() {
@@ -182,15 +183,38 @@ class BusyIndicator extends UI5Element {
 		};
 	}
 
-	get slotTabIndex() {
-		return this.active ? -1 : 0;
+	_handleKeydown(event) {
+		if (!this.active) {
+			return;
+		}
+
+		event.stopImmediatePropagation();
+
+		// move the focus to the last element in this DOM and let TAB continue to the next focusable element
+		if (isTabNext(event)) {
+			this.focusForward = true;
+			this.shadowRoot.querySelector("[data-ui5-focus-redirect]").focus();
+			this.focusForward = false;
+		}
 	}
 
 	_preventEvent(event) {
 		if (this.active) {
 			event.stopImmediatePropagation();
 		}
 	}
+
+	/**
+	 * Moves the focus to busy area when coming with SHIFT + TAB
+	 */
+	_redirectFocus(event) {
+		if (this.focusForward) {
+			return;
+		}
+
+		event.preventDefault();
+		this.shadowRoot.querySelector(".ui5-busyindicator-busy-area").focus();
+	}
 }
 
 BusyIndicator.define();

packages/main/test/pages/BusyIndicator.html

@@ -45,9 +45,13 @@
 	<br />
 	<br />
 
-	<ui5-busyindicator id="indicatorWithBtn" size="Medium" active>
-		<ui5-button>Hello World</ui5-button>
-	</ui5-busyindicator>
+	<div>
+		<ui5-button id="beforeIndicatorWithBtn">focus stop before</ui5-button>
+		<ui5-busyindicator id="indicatorWithBtn" size="Medium" active>
+			<ui5-button>Hello World</ui5-button>
+		</ui5-busyindicator>
+		<ui5-button id="afterIndicatorWithBtn" >focus stop after</ui5-button>
+	</div>
 
 	<br />
 	<br />
@@ -139,6 +143,34 @@
 
 	<ui5-input value="0" id="tree-input"></ui5-input>
 
+	<br>
+	<br>
+
+	<ui5-button id="open-dialog-inactive-indicator">Open Dialog with Inactive Indicator</ui5-button>
+	<ui5-dialog id="dialog-inactive-indicator">
+		<div slot="header">Dialog with focus issue</div>
+		<div>
+			<div>Buttons are wrapped in busy indicator, which is inactive.</div>
+			<ui5-busyindicator>
+				<ui5-button id="dialog-inactive-indicator-focused-button">button 1</ui5-button>
+				<ui5-button>button 2</ui5-button>
+				<ui5-button>button 3</ui5-button>
+			</ui5-busyindicator>
+		</div>
+	</ui5-dialog>
+
+	<ui5-button id="open-dialog-active-indicator">Open Dialog with Active Indicator</ui5-button>
+	<ui5-dialog id="dialog-active-indicator">
+		<div slot="header">Dialog with focus issue</div>
+		<div>
+			<div>Buttons are wrapped in busy indicator, which is active.</div>
+			<ui5-busyindicator active>
+				<ui5-button>button 1</ui5-button>
+				<ui5-button>button 2</ui5-button>
+				<ui5-button>button 3</ui5-button>
+			</ui5-busyindicator>
+		</div>
+	</ui5-dialog>
 	<script>
 		var busyIndocator = document.getElementById("busy-container");
 		var list = document.getElementById("fetch-list");
@@ -189,6 +221,14 @@
 				input.value = ++inputValue;
 			}
 		});
+
+		document.getElementById("open-dialog-inactive-indicator").addEventListener("click", function () {
+			document.getElementById("dialog-inactive-indicator").open();
+		});
+
+		document.getElementById("open-dialog-active-indicator").addEventListener("click", function () {
+			document.getElementById("dialog-active-indicator").open();
+		});
 	</script>
 </body>
 

packages/main/test/specs/BusyIndicator.spec.js

@@ -43,10 +43,32 @@ describe("BusyIndicator general interaction", () => {
 		assert.strictEqual(innerFocusElement.getAttribute("aria-valuetext"), "Busy", "Correct 'aria-valuetext' is set");
 	});
 
-	it("tests content is not reachable with keyboard when active", () => {
+	it("tests content is not reachable with keyboard when active in both directions", () => {
+		const beforeBusyIndicator = browser.$("#beforeIndicatorWithBtn");
 		const busyIndicator = browser.$("#indicatorWithBtn");
-		const defaultSLot = busyIndicator.shadow$("slot");
+		const afterBusyIndicator = browser.$("#afterIndicatorWithBtn");
 
-		assert.strictEqual(defaultSLot.getAttribute("tabindex"), "-1", "Slot is not reachable via keyboard");
+		beforeBusyIndicator.click();
+		browser.keys("Tab");
+		assert.strictEqual($(browser.getActiveElement()).getAttribute("id"), busyIndicator.getAttribute("id"), "Correct element is focused with TAB");
+
+		browser.keys("Tab");
+		assert.strictEqual($(browser.getActiveElement()).getAttribute("id"), afterBusyIndicator.getAttribute("id"), "Correct element is focused with TAB");
+
+		browser.keys(["Shift", "Tab"]);
+		assert.strictEqual($(browser.getActiveElement()).getAttribute("id"), busyIndicator.getAttribute("id"), "Correct element is focused with SHIFT + TAB");
+
+		browser.keys(["Shift", "Tab"]);
+		assert.strictEqual($(browser.getActiveElement()).getAttribute("id"), beforeBusyIndicator.getAttribute("id"), "Correct element is focused with SHIFT + TAB");
+	});
+
+	it("test inactive indicator in dialog - correct element from default slot is focused", () => {
+		browser.$("#open-dialog-inactive-indicator").click();
+
+		assert.strictEqual(
+			$(browser.getActiveElement()).getAttribute("id"),
+			browser.$("#dialog-inactive-indicator-focused-button").getAttribute("id"),
+			"Correct element from default slot is focused"
+		);
 	});
 });