Skip to content

Commit 377c9bc

Browse files
authored
fix(ui5-avatar): fix click event fired twice (#2967)
We used to fire a custom event, but did not stop the native one and end up 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). FIXES: #2943
1 parent e35cc1a commit 377c9bc

File tree

5 files changed

+48
-9
lines changed

5 files changed

+48
-9
lines changed

packages/main/src/Avatar.js

+11-4
Original file line numberDiff line numberDiff line change
@@ -324,18 +324,25 @@ class Avatar extends UI5Element {
324324
}
325325

326326
_onclick(event) {
327-
event.isMarked = "avatar";
328327
if (this.interactive) {
329-
event.preventDefault();
330-
// Prevent the native event and fire custom event because otherwise the noConfict event won't be thrown
328+
// prevent the native event and fire custom event to ensure the noConfict "ui5-click" is fired
329+
event.stopPropagation();
331330
this.fireEvent("click");
332331
}
333332
}
334333

335334
_onkeydown(event) {
336-
if (this.interactive && isEnter(event)) {
335+
if (!this.interactive) {
336+
return;
337+
}
338+
339+
if (isEnter(event)) {
337340
this.fireEvent("click");
338341
}
342+
343+
if (isSpace(event)) {
344+
event.preventDefault(); // prevent scrolling
345+
}
339346
}
340347

341348
_onkeyup(event) {

packages/main/src/AvatarGroup.hbs

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
@keydown="{{_onkeydown}}"
66
@focusin="{{_onfocusin}}"
77
tabindex="{{_groupTabIndex}}"
8-
@click="{{_onGroupClick}}"
8+
@click="{{_onClick}}"
99
@ui5-click="{{_onUI5Click}}"
1010
>
1111
<slot></slot>

packages/main/src/AvatarGroup.js

+11-2
Original file line numberDiff line numberDiff line change
@@ -340,15 +340,24 @@ class AvatarGroup extends UI5Element {
340340
});
341341
}
342342

343-
_onGroupClick(event) {
343+
_onClick(event) {
344+
// no matter the value of noConflict, the ui5-button and the group container (div) always fire a native click event
345+
const isButton = event.target.hasAttribute("ui5-button");
344346
event.stopPropagation();
345-
if (event.isMarked === "avatar" || event.isMarked === "button" || this._isGroup) {
347+
348+
if (this._isGroup || isButton) {
346349
this._fireGroupEvent(event.target);
347350
}
348351
}
349352

350353
_onUI5Click(event) {
354+
// when noConflict=true only ui5-avatar will fire ui5-click event
355+
const isAvatar = event.target.hasAttribute("ui5-avatar");
351356
event.stopPropagation();
357+
358+
if (isAvatar) {
359+
this._fireGroupEvent(event.target);
360+
}
352361
}
353362

354363
/**

packages/main/test/pages/Avatar.html

+11
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,20 @@ <h3>Avatar - interactive</h3>
117117
<ui5-avatar id="interactive-avatar" interactive initials="XS" size="XS"></ui5-avatar>
118118
<ui5-avatar id="non-interactive-avatar" initials="S" size="S"></ui5-avatar>
119119
<ui5-input id="click-event" value="0"></ui5-input>
120+
121+
<br>
122+
<ui5-avatar id="myInteractiveAvatar" interactive initials="L" size="L"></ui5-avatar>
123+
<ui5-input id="click-event-2"></ui5-input>
120124
</section>
121125

122126
<script>
123127
var avatar = document.getElementById("interactive-avatar"),
128+
myInteractiveAvatar = document.getElementById("myInteractiveAvatar"),
124129
nonInteractiveAvatar = document.getElementById("non-interactive-avatar"),
125130
input = document.getElementById("click-event"),
131+
input2 = document.getElementById("click-event-2"),
126132
inputValue = 0;
133+
inputValue2 = 0;
127134

128135
avatar.addEventListener("ui5-click", function() {
129136
input.value = ++inputValue;
@@ -132,6 +139,10 @@ <h3>Avatar - interactive</h3>
132139
nonInteractiveAvatar.addEventListener("ui5-click", function() {
133140
input.value = ++inputValue;
134141
});
142+
143+
myInteractiveAvatar.addEventListener("click", function() {
144+
input2.value = ++inputValue2;
145+
});
135146
</script>
136147

137148
</body>

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

+14-2
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ describe("Avatar", () => {
4646
assert.ok(initials.isExisting(), "initials are rendered");
4747
});
4848

49-
it("Tests if clicked event is thrown for interactive avatars", () => {
49+
it("Tests noConflict 'ui5-click' event is thrown for interactive avatars", () => {
5050
const avatarRoot = browser.$("#interactive-avatar").shadow$(".ui5-avatar-root");
5151
const input = browser.$("#click-event");
5252

@@ -60,7 +60,7 @@ describe("Avatar", () => {
6060
assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
6161
});
6262

63-
it("Tests if clicked event is not thrown for non interactive avatars", () => {
63+
it("Tests noConflict 'ui5-click' event is not thrown for non interactive avatars", () => {
6464
const avatarRoot = browser.$("#non-interactive-avatar").shadow$(".ui5-avatar-root");;
6565
const input = browser.$("#click-event");
6666

@@ -72,5 +72,17 @@ describe("Avatar", () => {
7272

7373
avatarRoot.keys("Space");
7474
assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
75+
});
76+
77+
it("Tests native 'click' event thrown", () => {
78+
browser.execute(function() {
79+
window["sap-ui-webcomponents-bundle"].configuration.setNoConflict(false);
7580
});
81+
82+
const avatar = browser.$("#myInteractiveAvatar");
83+
const input = browser.$("#click-event-2");
84+
85+
avatar.click();
86+
assert.strictEqual(input.getAttribute("value"), "1", "Mouse click throws event");
87+
});
7688
});

0 commit comments

Comments
 (0)