Skip to content

Commit 6fd6a5e

Browse files
authored
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
1 parent deb173a commit 6fd6a5e

File tree

3 files changed

+37
-5
lines changed

3 files changed

+37
-5
lines changed

packages/main/src/Icon.js

+11-3
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,17 @@ class Icon extends UI5Element {
199199
}
200200

201201
_onkeydown(event) {
202-
if (this.interactive && isEnter(event)) {
202+
if (!this.interactive) {
203+
return;
204+
}
205+
206+
if (isEnter(event)) {
203207
this.fireEvent("click");
204208
}
209+
210+
if (isSpace(event)) {
211+
event.preventDefault(); // prevent scrolling
212+
}
205213
}
206214

207215
_onkeyup(event) {
@@ -212,8 +220,8 @@ class Icon extends UI5Element {
212220

213221
_onclick(event) {
214222
if (this.interactive) {
215-
event.preventDefault();
216-
// Prevent the native event and fire custom event because otherwise the noConfict event won't be thrown
223+
// prevent the native event and fire custom event to ensure the noConfict "ui5-click" is fired
224+
event.stopPropagation();
217225
this.fireEvent("click");
218226
}
219227
}

packages/main/test/pages/Icon.html

+16
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@
3737
<ui5-icon name="add-employee" class="icon-red icon-small"></ui5-icon>
3838
<ui5-icon show-tooltip name="message-error"></ui5-icon>
3939

40+
<h3>Interactive Icon</h3>
41+
<ui5-icon
42+
id="myInteractiveIcon"
43+
interactive
44+
name="add-equipment"
45+
style="width: 50px; height: 50px">
46+
</ui5-icon>
47+
<br>
48+
<ui5-input id="click-event-2"></ui5-input>
49+
4050
<br>
4151
<ui5-icon name="add-employee"></ui5-icon>
4252
<br>
@@ -89,7 +99,9 @@
8999
var icon = document.getElementById("interactive-icon"),
90100
nonInteractiveIcon = document.getElementById("non-interactive-icon"),
91101
input = document.getElementById("click-event"),
102+
input2 = document.getElementById("click-event-2"),
92103
inputValue = 0;
104+
inputValue2 = 0;
93105

94106
icon.addEventListener("ui5-click", function() {
95107
input.value = ++inputValue;
@@ -98,6 +110,10 @@
98110
nonInteractiveIcon.addEventListener("ui5-click", function() {
99111
input.value = ++inputValue;
100112
});
113+
114+
myInteractiveIcon.addEventListener("click", function() {
115+
input2.value = ++inputValue2;
116+
});
101117
</script>
102118

103119
<script type="module">

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

+10-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ describe("Icon general interaction", () => {
1414
"Built-in tooltip is correct");
1515
});
1616

17-
it("Tests if clicked event is thrown for interactive icons", () => {
17+
it("Tests noConflict 'ui5-click' event is thrown for interactive icons", () => {
1818
const iconRoot = browser.$("#interactive-icon").shadow$(".ui5-icon-root");
1919
const input = browser.$("#click-event");
2020

@@ -28,7 +28,7 @@ describe("Icon general interaction", () => {
2828
assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
2929
});
3030

31-
it("Tests if clicked event is not thrown for non interactive icons", () => {
31+
it("Tests noConflict 'ui5-click' event is not thrown for non interactive icons", () => {
3232
const iconRoot = browser.$("#non-interactive-icon");
3333
const input = browser.$("#click-event");
3434

@@ -41,4 +41,12 @@ describe("Icon general interaction", () => {
4141
iconRoot.keys("Space");
4242
assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
4343
});
44+
45+
it("Tests native 'click' event thrown", () => {
46+
const icon = browser.$("#myInteractiveIcon");
47+
const input = browser.$("#click-event-2");
48+
49+
icon.click();
50+
assert.strictEqual(input.getAttribute("value"), "1", "Mouse click throws event");
51+
});
4452
});

0 commit comments

Comments
 (0)