fix(ui5-icon): fix click event fired twice (#2858)

We used to fire a custom event, but did not stop the native one and end up with firing two "click" events.
Now, the native one is stopped properly (we need to fire the custom one, so the noConflict ui5-click is also fired). Also, one additional fix has been performed - the content no longer scrolls when the SPACE key is pressed over "interactive" icon.

FIXES: #2857

Author: ilhan007

packages/main/src/Icon.js

@@ -194,9 +194,17 @@ class Icon extends UI5Element {
 	}
 
 	_onkeydown(event) {
-		if (this.interactive && isEnter(event)) {
+		if (!this.interactive) {
+			return;
+		}
+
+		if (isEnter(event)) {
 			this.fireEvent("click");
 		}
+
+		if (isSpace(event)) {
+			event.preventDefault(); // prevent scrolling
+		}
 	}
 
 	_onkeyup(event) {
@@ -207,8 +215,8 @@ class Icon extends UI5Element {
 
 	_onclick(event) {
 		if (this.interactive) {
-			event.preventDefault();
-			// Prevent the native event and fire custom event because otherwise the noConfict event won't be thrown
+			// prevent the native event and fire custom event to ensure the noConfict "ui5-click" is fired
+			event.stopPropagation(); 
 			this.fireEvent("click");
 		}
 	}

packages/main/test/pages/Icon.html

@@ -37,6 +37,16 @@
 	<ui5-icon name="add-employee" class="icon-red icon-small"></ui5-icon>
 	<ui5-icon show-tooltip name="message-error"></ui5-icon>
 
+	<h3>Interactive Icon</h3>
+	<ui5-icon
+		id="myInteractiveIcon"
+		interactive
+		name="add-equipment"
+		style="width: 50px; height: 50px">
+	</ui5-icon>
+	<br>
+	<ui5-input id="click-event-2"></ui5-input>
+
 	<br>
 	<ui5-icon name="add-employee"></ui5-icon>
 	<br>
@@ -103,7 +113,9 @@
 		var icon = document.getElementById("interactive-icon"),
 			nonInteractiveIcon = document.getElementById("non-interactive-icon"),
 			input = document.getElementById("click-event"),
+			input2 = document.getElementById("click-event-2"),
 			inputValue = 0;
+			inputValue2 = 0;
 
 		icon.addEventListener("ui5-click", function() {
 			input.value = ++inputValue;
@@ -112,6 +124,10 @@
 		nonInteractiveIcon.addEventListener("ui5-click", function() {
 			input.value = ++inputValue;
 		});
+
+		myInteractiveIcon.addEventListener("click", function() {
+			input2.value = ++inputValue2;
+		});
 	</script>
 
 	<script type="module">

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

@@ -5,35 +5,48 @@ describe("Icon general interaction", () => {
 
 	it("Tests icon rendering", () => {
 		const iconRoot = browser.$("#interactive-icon").shadow$("ui5-icon-root");
+		const iconWithTooltip = browser.$("#iconWithTooltip");
+		const iconTooltip = iconWithTooltip.shadow$(`#${iconWithTooltip.getProperty("_id")}-tooltip`);
+		const ICON_TOOLTIP_TEXT = "Save";
 
 		assert.ok(iconRoot, "Icon is rendered");
-    });
+		assert.strictEqual(iconTooltip.getHTML(false).includes(ICON_TOOLTIP_TEXT), true,
+			"Built-in tooltip is correct");
+	});
+
+	it("Tests noConflict 'ui5-click' event is thrown for interactive icons", () => {
+		const iconRoot = browser.$("#interactive-icon").shadow$(".ui5-icon-root");
+		const input = browser.$("#click-event");
+
+		iconRoot.click();
+		assert.strictEqual(input.getAttribute("value"), "1", "Mouse click throws event");
 
-  it("Tests if clicked event is thrown for interactive icons", () => {
-    const iconRoot = browser.$("#interactive-icon").shadow$(".ui5-icon-root");
-    const input = browser.$("#click-event");
+		iconRoot.keys("Enter");
+		assert.strictEqual(input.getAttribute("value"), "2", "Enter throws event");
 
-    iconRoot.click();
-    assert.strictEqual(input.getAttribute("value"), "1", "Mouse click throws event");
+		iconRoot.keys("Space");
+		assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
+	});
+
+	it("Tests noConflict 'ui5-click' event is not thrown for non interactive icons", () => {
+		const iconRoot = browser.$("#non-interactive-icon");
+		const input = browser.$("#click-event");
 
-    iconRoot.keys("Enter");
-    assert.strictEqual(input.getAttribute("value"), "2", "Enter throws event");
+		iconRoot.click();
+		assert.strictEqual(input.getAttribute("value"), "3", "Mouse click throws event");
 
-    iconRoot.keys("Space");
-    assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
-  });
-  
-  it("Tests if clicked event is not thrown for non interactive icons", () => {
-    const iconRoot = browser.$("#non-interactive-icon");
-    const input = browser.$("#click-event");
+		iconRoot.keys("Enter");
+		assert.strictEqual(input.getAttribute("value"), "3", "Enter throws event");
 
-    iconRoot.click();
-    assert.strictEqual(input.getAttribute("value"), "3", "Mouse click throws event");
+		iconRoot.keys("Space");
+		assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
+	});
 
-    iconRoot.keys("Enter");
-    assert.strictEqual(input.getAttribute("value"), "3", "Enter throws event");
+	it("Tests native 'click' event thrown", () => {
+		const icon = browser.$("#myInteractiveIcon");
+		const input = browser.$("#click-event-2");
 
-    iconRoot.keys("Space");
-    assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
+		icon.click();
+		assert.strictEqual(input.getAttribute("value"), "1", "Mouse click throws event");
 	});
-});
+});