Skip to content

Commit 5e4a664

Browse files
authored
fix(ui5-shellbar): fire "click" by the correct abstract ui5-shellbar-item (#4303)
- Fix an issue with the following code, although no longer throws an error as before the recently introduced [fix](https://github.com/SAP/ui5-webcomponents/pull/4287/files) is not complete: ```js const refItemId = event.target.getAttribute("data-ui5-external-action-item-id"); const shellbarItem = this.items.find(item => { return this.shadowRoot.querySelector(`#${refItemId}`); }); ``` As this is always true (the clicked item is among all items): ```js return this.shadowRoot.querySelector(`#${refItemId}`); ``` the result will be always the same - the first ui5-shellbar-item in the array will be returned and the"click" event will be always fired by the first ui5-shellbar-item, although we might click on another custom icon within the bar - the 2nd, 3rd, etc. - Add a test to avoid degradations
1 parent 8553293 commit 5e4a664

File tree

3 files changed

+29
-4
lines changed

3 files changed

+29
-4
lines changed

packages/fiori/src/ShellBar.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,7 @@ class ShellBar extends UI5Element {
726726

727727
if (refItemId) {
728728
const shellbarItem = this.items.find(item => {
729-
return this.shadowRoot.querySelector(`#${refItemId}`);
729+
return item._id === refItemId;
730730
});
731731

732732
const prevented = !shellbarItem.fireEvent("click", { targetRef: event.target }, true);

packages/fiori/test/pages/ShellBar.html

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@
5555
id="shellbarwithitems"
5656
>
5757
<ui5-shellbar-item icon="accelerated" text="Hello World!" title="Schedule"></ui5-shellbar-item>
58-
<ui5-shellbar-item icon="accept" text="Hello World!" title="Approve"></ui5-shellbar-item>
59-
<ui5-shellbar-item icon="alert" text="Hello World!" title="Warning"></ui5-shellbar-item>
58+
<ui5-shellbar-item id="accept" icon="accept" text="Hello World!" title="Approve"></ui5-shellbar-item>
59+
<ui5-shellbar-item id="warning" icon="alert" text="Hello World!" title="Warning"></ui5-shellbar-item>
6060
<ui5-shellbar-item icon="discussion" text="Hello World!" count="42" title="42 Messages"></ui5-shellbar-item>
6161
<ui5-shellbar-item icon="error" text="Hello World!" title="Attention"></ui5-shellbar-item>
6262
<ui5-shellbar-item icon="hello-world" text="UI5 Webcomponents" title="hello world"></ui5-shellbar-item>
@@ -65,6 +65,8 @@
6565
<ui5-shellbar-item icon="sound-loud" text="UI5 Webcomponents" title="Sound"></ui5-shellbar-item>
6666
</ui5-shellbar>
6767

68+
<ui5-input id="press-input3" class="shellbar3auto"></ui5-input>
69+
6870
<ui5-shellbar
6971
class="shellbar-example"
7072
secondary-title="No primary title"
@@ -284,6 +286,14 @@ <h3>ShellBar in Compact</h3>
284286
window["custom-item-popover"].showAt(event.detail.targetRef);
285287
});
286288
});
289+
290+
["accept", "warning"].forEach(function(id) {
291+
var currenItem = window[id][0] || window[id];
292+
currenItem.addEventListener("ui5-click", function(event) {
293+
event.preventDefault();
294+
window["press-input3"].value = event.target.id;
295+
});
296+
});
287297
</script>
288298
</body>
289299
</html>

packages/fiori/test/specs/ShellBar.spec.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,22 @@ describe("Component Behavior", () => {
6060

6161
assert.strictEqual(await shellbar.shadow$(".ui5-shellbar-custom-item").getAttribute("data-count"), "3");
6262

63-
})
63+
});
64+
65+
it("tests 'click' on custom action", async () => {
66+
const shellbar = await browser.$("#shellbarwithitems");
67+
const resultInput = await browser.$("#press-input3");
68+
const customActionIcon1 = await shellbar.shadow$(`.ui5-shellbar-custom-item[icon="accept"]`);
69+
const customActionIcon2 = await shellbar.shadow$(`.ui5-shellbar-custom-item[icon="alert"]`);
70+
71+
await customActionIcon1.click();
72+
assert.strictEqual(await resultInput.getProperty("value"), "accept",
73+
"click, fired by the first item");
74+
75+
await customActionIcon2.click();
76+
assert.strictEqual(await resultInput.getProperty("value"), "warning",
77+
"click, fired by the second item");
78+
});
6479
});
6580

6681
describe("Responsiveness", () => {

0 commit comments

Comments
 (0)