fix(ui5-select): preselect first item if none is selected (#358)

MapTo0 · web-flow · commit 3d1842090357 · 2019-04-24T11:41:01.000+03:00
- pressing escape now reverts the selection before opening the picker - focusing out the popover now triggers a change event (if selection is changed) Fixes: #339
diff --git a/packages/main/src/Select.js b/packages/main/src/Select.js
@@ -5,6 +5,7 @@ import {
 	isUp,
 	isDown,
 	isEnter,
+	isEscape,
 } from "@ui5/webcomponents-base/src/events/PseudoEvents.js";
 import KeyCodes from "@ui5/webcomponents-core/dist/sap/ui/events/KeyCodes.js";
 import ValueState from "@ui5/webcomponents-base/src/types/ValueState.js";
@@ -154,6 +155,10 @@ class Select extends UI5Element {
 		super();
 
 		this._closing = false; // Flag for handling open/close on space
+		this._selectedItemBeforeOpen = null; // Stores the selected item before opening the picker
+		this._escapePressed = false; // Identifies if the escape is pressed when picker is open
+
+
 		this._setSelectedItem(null);
 		this._setPreviewedItem(null);
 		this.Suggestions = new Suggestions(this, "items", true /* move focus with arrow keys */);
@@ -201,6 +206,15 @@ class Select extends UI5Element {
 			this.Suggestions.onEnter(event);
 		}
 
+		if (isEscape(event) && this._opened && this._selectedItemBeforeOpen) {
+			this.items.forEach(item => {
+				item.selected = false;
+			});
+
+			this._select(this._selectedItemBeforeOpen, this.items.indexOf(this._selectedItemBeforeOpen));
+			this._escapePressed = true;
+		}
+
 		const key = event.which;
 
 		if (key === KeyCodes.F4 || (event.altKey && Select.ARROWS.includes(key))) {
@@ -232,7 +246,6 @@ class Select extends UI5Element {
 		}
 
 		this._select(item);
-		this._fireChange(item);
 	}
 
 	onItemPreviewed(item) {
@@ -241,21 +254,30 @@ class Select extends UI5Element {
 	}
 
 	onOpen() {
-		this._opened = true;// invalidating property
+		this._opened = true; // invalidating property
+
+		const selectedItem = this._getSelectedItem();
+
+		if (selectedItem) {
+			this._selectedItemBeforeOpen = selectedItem;
+			selectedItem.focus();
+		}
 	}
 
 	onClose() {
-		this._opened = false;// invalidating property
+		this._opened = false; // invalidating property
 
-		if (this._isSelectionChanged()) {
-			const previewedItem = this._getPreviewedItem();
+		if ((this._getSelectedItem() !== this._selectedItemBeforeOpen) && !this._escapePressed) {
+			const previewedItem = this._getSelectedItem();
 			this._fireChange(previewedItem);
 		}
+
+		this._escapePressed = false;
 	}
 
 	/* Private methods */
 	_validateSelection() {
-		if (this._isOpened()) {
+		if (this._isOpened() || !this.items.length) {
 			return;
 		}
 
@@ -272,6 +294,11 @@ class Select extends UI5Element {
 			}
 		});
 
+		if (!selectedItem) {
+			selectedItem = this.items[0];
+			selectedItemPos = 0;
+		}
+
 		if (this._getSelectedItem() !== selectedItem) {
 			this._select(selectedItem, selectedItemPos);
 		}
diff --git a/packages/main/test/sap/ui/webcomponents/main/pages/Select.html b/packages/main/test/sap/ui/webcomponents/main/pages/Select.html
@@ -15,8 +15,11 @@
 
 <body>
 	<ui5-togglebutton id="myBtn">turn lights</ui5-togglebutton>
+	<ui5-button id="add-items-btn">Add new Items</ui5-button>
+	<ui5-button id="restore-items-btn">Restore Items</ui5-button>
 	<ui5-button id="myBtn2">click</ui5-button>
 
+
 	<h2>Content size</h2>
 	<ui5-label id="lblResult"></ui5-label>
 	<ui5-select id="mySelect">
@@ -50,6 +53,8 @@ <h2> Change event counter holder</h2>
 	var sap_database_entries = [{ key: "Afg", text: "Afghanistan" }, { key: "Arg", text: "Argentina" }, { key: "Alb", text: "Albania" }, { key: "Arm", text: "Armenia" }, { key: "Alg", text: "Algeria" }, { key: "And", text: "Andorra" }, { key: "Ang", text: "Angola" }, { key: "Ast", text: "Austria" }, { key: "Aus", text: "Australia" }, { key: "Aze", text: "Azerbaijan" }, { key: "Aruba", text: "Aruba" }, { key: "Antigua", text: "Antigua and Barbuda" }, { key: "Bel", text: "Belarus" }, { key: "Bel", text: "Belgium" }, { key: "Bg", text: "Bulgaria" }, { key: "Bra", text: "Brazil" }, { key: "Ch", text: "China" }, { key: "Cub", text: "Cuba" }, { key: "Chil", text: "Chili" }, { key: "Lat", text: "Latvia" }, { key: "Lit", text: "Litva" }, { key: "Prt", text: "Portugal" }, { key: "Sen", text: "Senegal" }, { key: "Ser", text: "Serbia" }, { key: "Afg", text: "Seychelles" }, { key: "Sierra", text: "Sierra Leone" }, { key: "Sgp", text: "Singapore" }, { key: "Sint", text: "Sint Maarten" }, { key: "Slv", text: "Slovakia" }, { key: "Slo", text: "Slovenia" }];
 
 	var btn = document.getElementById('myBtn');
+	var addItemsBtn = document.getElementById('add-items-btn');
+	var restoreItemsBtn = document.getElementById('restore-items-btn');
 	var input = document.getElementById('myInput');
 	var inputResult = document.getElementById('inputResult');
 	var select = document.getElementById('mySelect');
@@ -117,6 +122,24 @@ <h2> Change event counter holder</h2>
 		li.textContent = item.text;
 		select3.appendChild(li);
 	});
+
+	var initialItemsHTML = "";
+
+	addItemsBtn.addEventListener("press", event => {
+		initialItemsHTML = select.innerHTML;
+
+		var li = document.createElement('ui5-li');
+
+		li.type = "Active";
+		li.textContent = "New";
+
+		select.innerHTML = "";
+		select.appendChild(li);
+	});
+
+	restoreItemsBtn.addEventListener("press", event => {
+		select.innerHTML = initialItemsHTML;
+	});
 </script>
 </body>
 </html>
diff --git a/packages/main/test/specs/Select.spec.js b/packages/main/test/specs/Select.spec.js
@@ -111,7 +111,6 @@ describe("Select general interaction", () => {
 
 	it("toggles upon ALT + DOWN", () => {
 		const btn = $("#myBtn2");
-		const select = $("#mySelect");
 		const popover = browser.findElementDeep("#mySelect >>> ui5-popover >>> .sapMPopover");
 
 		btn.click();
@@ -123,4 +122,66 @@ describe("Select general interaction", () => {
 		browser.keys(["Alt", "ArrowDown", "NULL"]);
 		assert.ok(!popover.isDisplayedInViewport(), "Select is closed.");
 	});
+
+	it("adds unselected only items to select", () => {
+		const addItemsBtn = $("#add-items-btn");
+		const restoreItemsBtn = $("#restore-items-btn");
+
+		addItemsBtn.click();
+
+		const firstItem = $("#mySelect ui5-li[slot=items-1]");
+
+		assert.ok(firstItem.getProperty("selected"), "First Item should be selected");
+
+		restoreItemsBtn.click();
+	});
+
+	it("reverts value before open after clicking on escape", () => {
+		const select = $("#mySelect");
+		const selectText = browser.findElementDeep("#mySelect >>> ui5-label").getHTML(false);
+		const inputResult = browser.findElementDeep("#inputResult >>> input");
+
+		select.click();
+		select.keys("ArrowDown");
+		select.keys("Escape");
+
+		const selectedItem = $("#mySelect ui5-li[selected]");
+		const selectTextAfterEscape = browser.findElementDeep("#mySelect >>> ui5-label").getHTML(false);
+
+		assert.ok(selectedItem.getProperty("selected"), "Initially selected item should remain selected");
+		assert.strictEqual(inputResult.getProperty("value"), "5", "Change event should not be fired");
+		assert.strictEqual(selectTextAfterEscape, selectText, "Initially selected item should remain selected");
+	});
+
+	it("fires change event after selection is change and picker if focussed out", () => {
+		const select = $("#mySelect");
+		const inputResult = browser.findElementDeep("#inputResult >>> input");
+		const btn = $("#myBtn2");
+
+		select.click();
+		select.keys("ArrowDown");
+		select.keys("ArrowDown");
+		
+		// focus out select
+		btn.click();
+		
+		assert.strictEqual(inputResult.getProperty("value"), "6", "Change event should be fired");
+	});
+
+	it("fires change event after selecting a previewed item", () => {
+		const select = $("#mySelect");
+		const inputResult = browser.findElementDeep("#inputResult >>> input");
+
+		select.click();
+		select.keys("ArrowDown");
+
+		select.keys("Escape");
+
+		select.click();
+		const firstItem = $("#mySelect ui5-li[slot=items-1]");
+
+		firstItem.click();
+
+		assert.strictEqual(inputResult.getProperty("value"), "7", "Change event should be fired");
+	});
 });
