Skip to content

Commit

Permalink
fix(ui5-avatar): fix click event fired twice (#2967)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dobrinyonkov authored and ilhan007 committed Mar 23, 2021
1 parent 4124e56 commit fed6acd
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 10 deletions.
15 changes: 11 additions & 4 deletions packages/main/src/Avatar.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,18 +323,25 @@ class Avatar extends UI5Element {
}

_onclick(event) {
event.isMarked = "avatar";
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");
}
}

_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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/AvatarGroup.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
@keydown="{{_onkeydown}}"
@focusin="{{_onfocusin}}"
tabindex="{{_groupTabIndex}}"
@click="{{_onGroupClick}}"
@click="{{_onClick}}"
@ui5-click="{{_onUI5Click}}"
>
<slot></slot>
Expand Down
13 changes: 11 additions & 2 deletions packages/main/src/AvatarGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -336,15 +336,24 @@ class AvatarGroup extends UI5Element {
});
}

_onGroupClick(event) {
_onClick(event) {
// no matter the value of noConflict, the ui5-button and the group container (div) always fire a native click event
const isButton = event.target.hasAttribute("ui5-button");
event.stopPropagation();
if (event.isMarked === "avatar" || event.isMarked === "button" || this._isGroup) {

if (this._isGroup || isButton) {
this._fireGroupEvent(event.target);
}
}

_onUI5Click(event) {
// when noConflict=true only ui5-avatar will fire ui5-click event
const isAvatar = event.target.hasAttribute("ui5-avatar");
event.stopPropagation();

if (isAvatar) {
this._fireGroupEvent(event.target);
}
}

/**
Expand Down
11 changes: 11 additions & 0 deletions packages/main/test/pages/Avatar.html
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,20 @@ <h3>Avatar - interactive</h3>
<ui5-avatar id="interactive-avatar" interactive initials="XS" size="XS"></ui5-avatar>
<ui5-avatar id="non-interactive-avatar" initials="S" size="S"></ui5-avatar>
<ui5-input id="click-event" value="0"></ui5-input>

<br>
<ui5-avatar id="myInteractiveAvatar" interactive initials="L" size="L"></ui5-avatar>
<ui5-input id="click-event-2"></ui5-input>
</section>

<script>
var avatar = document.getElementById("interactive-avatar"),
myInteractiveAvatar = document.getElementById("myInteractiveAvatar"),
nonInteractiveAvatar = document.getElementById("non-interactive-avatar"),
input = document.getElementById("click-event"),
input2 = document.getElementById("click-event-2"),
inputValue = 0;
inputValue2 = 0;

avatar.addEventListener("ui5-click", function() {
input.value = ++inputValue;
Expand All @@ -132,6 +139,10 @@ <h3>Avatar - interactive</h3>
nonInteractiveAvatar.addEventListener("ui5-click", function() {
input.value = ++inputValue;
});

myInteractiveAvatar.addEventListener("click", function() {
input2.value = ++inputValue2;
});
</script>

</body>
Expand Down
18 changes: 15 additions & 3 deletions packages/main/test/specs/Avatar.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe("Avatar", () => {
assert.ok(initials.isExisting(), "initials are rendered");
});

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

Expand All @@ -57,8 +57,8 @@ describe("Avatar", () => {
avatarRoot.keys("Space");
assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
});
it("Tests if clicked event is not thrown for non interactive avatars", () => {

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

Expand All @@ -70,5 +70,17 @@ describe("Avatar", () => {

avatarRoot.keys("Space");
assert.strictEqual(input.getAttribute("value"), "3", "Space throws event");
});

it("Tests native 'click' event thrown", () => {
browser.execute(function() {
window["sap-ui-webcomponents-bundle"].configuration.setNoConflict(false);
});

const avatar = browser.$("#myInteractiveAvatar");
const input = browser.$("#click-event-2");

avatar.click();
assert.strictEqual(input.getAttribute("value"), "1", "Mouse click throws event");
});
});

0 comments on commit fed6acd

Please sign in to comment.