Skip to content

Commit

Permalink
fix(ui5-list): fire itemClick after the selection (#1618)
Browse files Browse the repository at this point in the history
We used to fire the "itemClick" and "itemPress" before the selection finishes and if someone gets the "event.detail.item.selected" - it is will be the old one. 
Now, they are fired after the selection finishes and provide the correct "item" state, although it is not common to listen for  "itemClick" in selection modes, but rather for "selectionChange".

FIXES: #1615
  • Loading branch information
ilhan007 committed May 15, 2020
1 parent ec5a9cf commit 28326c5
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 6 deletions.
6 changes: 3 additions & 3 deletions packages/main/src/List.js
Original file line number Diff line number Diff line change
Expand Up @@ -564,9 +564,6 @@ class List extends UI5Element {
onItemPress(event) {
const pressedItem = event.detail.item;

this.fireEvent("itemPress", { item: pressedItem });
this.fireEvent("itemClick", { item: pressedItem });

if (!this._selectionRequested && this.mode !== ListMode.Delete) {
this._selectionRequested = true;
this.onSelectionRequested({
Expand All @@ -579,6 +576,9 @@ class List extends UI5Element {
});
}

this.fireEvent("itemPress", { item: pressedItem });
this.fireEvent("itemClick", { item: pressedItem });

this._selectionRequested = false;
}

Expand Down
30 changes: 28 additions & 2 deletions packages/main/test/pages/List_test_page.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,26 @@
<input id="loadMoreResult" value="0"/>
<br/>
<ui5-list id="listEvents" mode="SingleSelectEnd">
<ui5-li id="country1" >Argentina</ui5-li>
<ui5-li id="country1">Argentina</ui5-li>
<ui5-li id="country2" selected >Bulgaria</ui5-li>
<ui5-li id="country3" >China</ui5-li>
<ui5-li id="country3">China</ui5-li>
<ui5-li id="country4" type="Inactive">Portugal</ui5-li>
</ui5-list>
<ui5-input id="itemPressResultField" placeholder="itemPress result"></ui5-input>
<ui5-input id="itemPressSelectedResultField" placeholder="itemPress selected result"></ui5-input>
<ui5-input id="selectionChangeResultField" placeholder="selectionChange result"></ui5-input>
<ui5-input id="selectionChangeResultPreviousItemsParameter" placeholder="selectionChange previousSelection result"></ui5-input>

<ui5-list id="listEvents2" mode="MultiSelect">
<ui5-li id="country11">Argentina</ui5-li>
<ui5-li id="country22" selected >Bulgaria</ui5-li>
<ui5-li id="country33">China</ui5-li>
<ui5-li id="country44">Portugal</ui5-li>
</ui5-list>
<ui5-input id="itemPressResultField2" placeholder="itemPress result"></ui5-input>
<ui5-input id="itemPressSelectedResultField2" placeholder="itemPress selected result"></ui5-input>
<ui5-input id="selectionChangeResultField2" placeholder="selectionChange result"></ui5-input>

<ui5-list id="list1" header-text="API: GroupHeaderListItem">
<ui5-li-groupheader>New Items</ui5-li-groupheader>
<ui5-li image="./img/HT-1000.jpg" >Laptop HP</ui5-li>
Expand Down Expand Up @@ -126,7 +137,9 @@

var itemDeleteCounter = 0;
var itemPressCounter = 0;
var itemPressCounter2 = 0;
var selectionChangeCounter = 0;
var selectionChangeCounter2 = 0;

function deleteItemHandler(e) {
document.querySelector("#lblResult").innerHTML = e.detail.item.innerHTML + ": " + ++itemDeleteCounter;
Expand All @@ -137,6 +150,7 @@
listEvents.addEventListener("ui5-itemPress", function (event) {
itemPressCounter += 1;
itemPressResultField.value = itemPressCounter;
itemPressSelectedResultField.value = event.detail.item.selected;
});

listEvents.addEventListener("ui5-selectionChange", function (event) {
Expand All @@ -148,6 +162,18 @@
selectionChangeResultPreviousItemsParameter.value = event.detail.previouslySelectedItems[0].id;
})

listEvents2.addEventListener("ui5-itemPress", function (event) {
itemPressCounter2 += 1;
itemPressResultField2.value = itemPressCounter2;
itemPressSelectedResultField2.value = event.detail.item.selected;
});

listEvents2.addEventListener("ui5-selectionChange", function (event) {
selectionChangeCounter2 += 1;
selectionChangeResultField2.value = selectionChangeCounter2;
});


listMultiSel.addEventListener("ui5-selectionChange", function(event) {
fieldMultiSelResult.value = event.detail.selectionComponentPressed;
})
Expand Down
17 changes: 16 additions & 1 deletion packages/main/test/specs/List.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,32 @@ describe("List Tests", () => {
assert.ok(list, "List is rendered");
});

it("itemPress and selectionChange events are fired", () => {
it("itemPress and selectionChange events are fired in Single selection", () => {
const itemPressResultField = $("#itemPressResultField");
const itemPressSelectedResultField = $("#itemPressSelectedResultField");
const selectionChangeResultField = $("#selectionChangeResultField");
const firstItem = $("#listEvents #country1");

firstItem.click();

assert.strictEqual(itemPressResultField.getProperty("value"), "1", "itemPress event has been fired once");
assert.strictEqual(itemPressSelectedResultField.getProperty("value"), "true", "itemPress detail 'item' has correct value.");
assert.strictEqual(selectionChangeResultField.getProperty("value"), "1", "selectionChange event has been fired.");
});

it("itemPress and selectionChange events are fired in Multi selection", () => {
const itemPressResultField2 = $("#itemPressResultField2");
const itemPressSelectedResultField2 = $("#itemPressSelectedResultField2");
const selectionChangeResultField2 = $("#selectionChangeResultField2");
const firstItem = $("#listEvents2 #country11");

firstItem.click();

assert.strictEqual(itemPressResultField2.getProperty("value"), "1", "itemPress event has been fired once");
assert.strictEqual(itemPressSelectedResultField2.getProperty("value"), "true", "itemPress detail 'item' has correct value.");
assert.strictEqual(selectionChangeResultField2.getProperty("value"), "1", "selectionChange event has been fired.");
});

it("selectionChange events provides previousSelection item", () => {
const selectionChangeResultPreviousItemsParameter = $("#selectionChangeResultPreviousItemsParameter");
const firstItem = $("#listEvents #country1");
Expand Down

0 comments on commit 28326c5

Please sign in to comment.