fix(ui5-select): prevent selection from cycling (#1066)

SAP · Dec 17, 2019 · d46be1f · d46be1f
1 parent e4ed295
commit d46be1f
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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();