fix: fix Item Navigation cycling (#985)

ilhan007 · web-flow · commit 3d46e2d9d321 · 2019-11-27T09:43:54.000+02:00
**Background**
By default the focus (managed by ItemNavigation) does not cycle trough the items, which turned out that has not been working.
**Root Cause**
We check for the `cycle` config when border cases are reached (first or last item), but the only thing we do is to fire a private event ("_borderReach"), but updating the index, so it actually makes a whole loop (cycle).
**Fix**
In these border cases we just update the internal index to `0` (when "top" border is reached) and `items.length-1` (when "bottom" border is reached).
diff --git a/packages/base/src/delegate/ItemNavigation.js b/packages/base/src/delegate/ItemNavigation.js
@@ -37,16 +37,18 @@ class ItemNavigation extends EventProvider {
 
 		if (this.currentIndex >= items.length) {
 			if (!this.cyclic) {
+				this.currentIndex = items.length - 1;
 				this.fireEvent(ItemNavigation.BORDER_REACH, { start: false, end: true, offset: this.currentIndex });
+			} else {
+				this.currentIndex = this.currentIndex - items.length;
 			}
-
-			this.currentIndex = this.currentIndex - items.length;
 		} else if (this.currentIndex < 0) {
 			if (!this.cyclic) {
+				this.currentIndex = 0;
 				this.fireEvent(ItemNavigation.BORDER_REACH, { start: true, end: false, offset: this.currentIndex });
+			} else {
+				this.currentIndex = items.length + this.currentIndex;
 			}
-
-			this.currentIndex = items.length + this.currentIndex;
 		}
 
 		this.update();
diff --git a/packages/main/test/pages/ItemNavigation.html b/packages/main/test/pages/ItemNavigation.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<meta http-equiv="X-UA-Compatible" content="IE=edge">
+	<meta charset="utf-8">
+	<title>Item Navigation</title>
+
+	<script src="../../webcomponentsjs/webcomponents-loader.js"></script>
+	<script src="../../resources/bundle.esm.js" type="module"></script>
+	<script nomodule src="../../resources/bundle.es5.js"></script>
+</head>
+
+<body>
+	<h2>Focus does not cycle</h2>
+	<ui5-list>
+		<ui5-li id="item1">Option 1</ui5-li>
+		<ui5-li id="item2">Option 2</ui5-li>
+	</ui5-list>
+</body>
+</html>
diff --git a/packages/main/test/specs/ItemNavigation.spec.js b/packages/main/test/specs/ItemNavigation.spec.js
@@ -0,0 +1,22 @@
+const assert = require("assert");
+
+describe("Item Navigation Tests", () => {
+	before(() => {
+		browser.url("http://localhost:8080/test-resources/pages/ItemNavigation.html");
+	});
+
+	it("focus does not cycle", () => {
+		const firstItem = $("#item1");
+		const secondItem = $("#item2");
+
+		firstItem.click();
+		firstItem.keys("ArrowLeft");
+		firstItem.keys("ArrowUp");
+		assert.strictEqual(firstItem.isFocused(), true, "first item remains focused");
+
+		secondItem.click();
+		secondItem.keys("ArrowRight");
+		secondItem.keys("ArrowDown");
+		assert.strictEqual(secondItem.isFocused(), true, "second item remains focused");
+	});
+});
