From d46be1f99e9ac20e9c2fbbded16fec89600a3e14 Mon Sep 17 00:00:00 2001
From: ilhan orhan <ilhan.myumyun@sap.com>
Date: Tue, 17 Dec 2019 12:19:18 +0200
Subject: [PATCH] fix(ui5-select): prevent selection from cycling (#1066)

---
 packages/main/src/Select.js             |  4 +--
 packages/main/test/pages/Select.html    | 13 +++++++++
 packages/main/test/specs/Select.spec.js | 35 ++++++++++++++++++++++---
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/packages/main/src/Select.js b/packages/main/src/Select.js
index 1c1d20d13042..f3ff3fde97f0 100644
--- a/packages/main/src/Select.js
+++ b/packages/main/src/Select.js
@@ -363,11 +363,11 @@ class Select extends UI5Element {
 	}
 
 	_getNextOptionIndex() {
-		return this._selectedIndex === (this.options.length - 1) ? 0 : (this._selectedIndex + 1);
+		return this._selectedIndex === (this.options.length - 1) ? this._selectedIndex : (this._selectedIndex + 1);
 	}
 
 	_getPreviousOptionIndex() {
-		return this._selectedIndex === 0 ? (this.options.length - 1) : (this._selectedIndex - 1);
+		return this._selectedIndex === 0 ? this._selectedIndex : (this._selectedIndex - 1);
 	}
 
 	_beforeOpen() {
diff --git a/packages/main/test/pages/Select.html b/packages/main/test/pages/Select.html
index 68adc4b943f4..58568cc0b286 100644
--- a/packages/main/test/pages/Select.html
+++ b/packages/main/test/pages/Select.html
@@ -46,6 +46,19 @@ <h2>Disabled Select</h2>
 	<h2> Input with suggestions</h2>
 	<ui5-input id="myInput" show-suggestions placeholder="Search for a country ..."></ui5-input>
 
+	<h2>Selection not cycling</h2>
+	<ui5-select id="selectionNotCycling">
+		<ui5-option>Opt1</ui5-option>
+		<ui5-option>Opt2</ui5-option>
+		<ui5-option selected>Opt3</ui5-option>
+	</ui5-select>
+
+	<ui5-select id="selectionNotCycling2">
+		<ui5-option selected>Opt1</ui5-option>
+		<ui5-option>Opt2</ui5-option>
+		<ui5-option>Opt3</ui5-option>
+	</ui5-select>
+
 	<h2> Change event counter holder</h2>
 	<ui5-input id="inputResult"></ui5-input>
 </body>
diff --git a/packages/main/test/specs/Select.spec.js b/packages/main/test/specs/Select.spec.js
index 1200d18eb253..17caa53e5944 100644
--- a/packages/main/test/specs/Select.spec.js
+++ b/packages/main/test/specs/Select.spec.js
@@ -73,10 +73,41 @@ describe("Select general interaction", () => {
 		assert.strictEqual(inputResult.getProperty("value"), "5", "Change event should have fired twice");
 	});
 
+	it("tests selection does not cycle with ArrowDown", () => {
+		const select = $("#selectionNotCycling");
+		const EXPECTED_SELECTION_TEXT = "Opt3";
+		const selectOptionText = select.shadow$("ui5-label");
+
+		select.click();
+		assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text is " + EXPECTED_SELECTION_TEXT);
+
+		// The last item is already selected - pressing ArrowDown should not change the focus or the selection
+		select.keys("ArrowDown"); 
+		assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text remains " + EXPECTED_SELECTION_TEXT);
+
+		// Close the select not to cover other components that tests would try to click
+		select.keys("Escape");
+
+	});
+
+	it("tests selection does not cycle with ArrowUp", () => {
+		const select = $("#selectionNotCycling2");
+		const EXPECTED_SELECTION_TEXT = "Opt1";
+		const selectOptionText = select.shadow$("ui5-label");
+
+		select.click();
+		assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text is " + EXPECTED_SELECTION_TEXT);
+
+		// The last item is already selected - pressing ArrowUp should not change the focus or the selection
+		select.keys("ArrowUp"); 
+		assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text remains " + EXPECTED_SELECTION_TEXT);
+
+		// Close the select not to cover other components that tests would try to click
+		select.keys("Escape");
+	});
 
 	it("opens upon space", () => {
 		const btn = $("#myBtn2");
-		const select = $("#mySelect");
 		const popover = browser.$("#mySelect").shadow$("ui5-popover");
 
 		btn.click();
@@ -88,7 +119,6 @@ describe("Select general interaction", () => {
 
 	it("toggles upon F4", () => {
 		const btn = $("#myBtn2");
-		const select = $("#mySelect");
 		const popover = browser.$("#mySelect").shadow$("ui5-popover");
 
 		btn.click();
@@ -103,7 +133,6 @@ describe("Select general interaction", () => {
 
 	it("toggles upon ALT + UP", () => {
 		const btn = $("#myBtn2");
-		const select = $("#mySelect");
 		const popover = browser.$("#mySelect").shadow$("ui5-popover");
 
 		btn.click();