Skip to content

Commit

Permalink
fix(ui5-multi-combobox): close popover & empty value on selection (#832)
Browse files Browse the repository at this point in the history
  • Loading branch information
fifoosid authored Oct 16, 2019
1 parent c1c1f26 commit 1b3e40d
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 42 deletions.
2 changes: 2 additions & 0 deletions packages/main/src/List.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ class List extends UI5Element {
selectedItems: this.getSelectedItems(),
previouslySelectedItems,
selectionComponentPressed: event.detail.selectionComponentPressed,
key: event.detail.key,
});
}
}
Expand Down Expand Up @@ -455,6 +456,7 @@ class List extends UI5Element {
item: pressedItem,
selectionComponentPressed: false,
selected: !pressedItem.selected,
key: event.detail.key,
},
});
}
Expand Down
10 changes: 5 additions & 5 deletions packages/main/src/ListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class ListItem extends ListItemBase {
}

if (isEnter(event)) {
this.fireItemPress();
this.fireItemPress(event);
}
}

Expand All @@ -153,7 +153,7 @@ class ListItem extends ListItemBase {
}

if (isSpace(event)) {
this.fireItemPress();
this.fireItemPress(event);
}
}

Expand All @@ -179,7 +179,7 @@ class ListItem extends ListItemBase {
if (event.isMarked === "button") {
return;
}
this.fireItemPress();
this.fireItemPress(event);
}

/*
Expand All @@ -204,8 +204,8 @@ class ListItem extends ListItemBase {
this.fireEvent("_selectionRequested", { item: this, selectionComponentPressed: false });
}

fireItemPress() {
this.fireEvent("_press", { item: this, selected: this.selected });
fireItemPress(event) {
this.fireEvent("_press", { item: this, selected: this.selected, key: event.key });
}

get placeSelectionElementBefore() {
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/MultiComboBox.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
</ui5-tokenizer>

<input id="ui5-multi-combobox-input"
value="{{value}}"
.value="{{value}}"
inner-input
placeholder="{{placeholder}}"
?disabled={{disabled}}
Expand Down
10 changes: 9 additions & 1 deletion packages/main/src/MultiComboBox.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js";
import ValueState from "@ui5/webcomponents-base/dist/types/ValueState.js";
import { isShow, isDown, isBackSpace } from "@ui5/webcomponents-base/dist/events/PseudoEvents.js";
import {
isShow, isDown, isBackSpace, isSpace,
} from "@ui5/webcomponents-base/dist/events/PseudoEvents.js";
import "./icons/slim-arrow-down.js";
import { getRTL } from "@ui5/webcomponents-base/dist/config/RTL.js";
import { isIE } from "@ui5/webcomponents-core/dist/sap/ui/Device.js";
Expand Down Expand Up @@ -402,6 +404,12 @@ class MultiComboBox extends UI5Element {
});

this.fireEvent("selectionChange", { items: this._getSelectedItems() });

if (!event.detail.selectionComponentPressed && !isSpace(event.detail)) {
this._getPopover().close();
this.value = "";
this.fireEvent("input");
}
}

_getPopover(isMorePopover) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
<ui5-li style="display: none" selected type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>

<div class="demo-section">
<span>Validating input and predefined value</span>

Expand All @@ -95,6 +96,7 @@
<ui5-li selected type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>

<div class="demo-section">
<span>Validating input and placeholder </span>

Expand All @@ -106,6 +108,7 @@
<ui5-li selected type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>

<div class="demo-section">
<span>disabled and placeholder </span>

Expand All @@ -117,39 +120,46 @@
<ui5-li selected type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>

<div class="demo-section">
<span>value state success </span>

<br>
<ui5-multi-combobox allow-custom-values style="width: 560px" placeholder="Some initial text" value-state="Success">
<ui5-multi-combobox allow-custom-values style="width: 560px" placeholder="Some initial text"
value-state="Success">
<ui5-li selected type="Active">Cosy</ui5-li>
<ui5-li type="Active">Compact</ui5-li>
<ui5-li selected type="Active">Condensed</ui5-li>
<ui5-li selected type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>

<div class="demo-section">
<span>value state warning </span>

<br>
<ui5-multi-combobox allow-custom-values style="width: 560px" placeholder="Some initial text" value-state="Warning">
<ui5-multi-combobox allow-custom-values style="width: 560px" placeholder="Some initial text"
value-state="Warning">
<ui5-li selected type="Active">Cosy</ui5-li>
<ui5-li type="Active">Compact</ui5-li>
<ui5-li selected type="Active">Condensed</ui5-li>
<ui5-li selected type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>

<div class="demo-section">
<span>value state error </span>

<br>
<ui5-multi-combobox allow-custom-values style="width: 560px" placeholder="Some initial text" value-state="Error">
<ui5-multi-combobox allow-custom-values style="width: 560px" placeholder="Some initial text"
value-state="Error">
<ui5-li selected type="Active">Cosy</ui5-li>
<ui5-li type="Active">Compact</ui5-li>
<ui5-li selected type="Active">Condensed</ui5-li>
<ui5-li selected type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>

<div class="demo-section">
<span>readonly </span>

Expand All @@ -175,27 +185,39 @@
</div>

<div class="demo-section">
<span>MultiComboBox n more</span> </br>
<span>MultiComboBox with validation</span>

<ui5-multi-combobox style="width: 320px" placeholder="Some initial text" id="more-mcb">
<ui5-li type="Active" selected>Cosy</ui5-li>
<ui5-li type="Active" selected>Compact</ui5-li>
<ui5-li type="Active">Condensed</ui5-li>
<ui5-li type="Active" selected>Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>
<br>
<ui5-multi-combobox allow-custom-values style="width: 560px" placeholder="Some initial text" id="another-mcb">
<ui5-li type="Active">Cosy</ui5-li>
<ui5-li type="Active">Compact</ui5-li>
<ui5-li type="Active">Condensed</ui5-li>
<ui5-li type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>

<div class="demo-section">
<span>MultiComboBox with validation</span>
<div class="demo-section">
<span>MultiComboBox n more</span> </br>

<br>
<ui5-multi-combobox style="width: 560px" placeholder="Some initial text" id="mcb-validation">
<ui5-li type="Active">Cosy</ui5-li>
<ui5-li type="Active">Compact</ui5-li>
<ui5-li type="Active">Condensed</ui5-li>
<ui5-li type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>
<ui5-multi-combobox style="width: 320px" placeholder="Some initial text" id="more-mcb">
<ui5-li type="Active" selected>Cosy</ui5-li>
<ui5-li type="Active" selected>Compact</ui5-li>
<ui5-li type="Active">Condensed</ui5-li>
<ui5-li type="Active" selected>Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>

<div class="demo-section">
<span>MultiComboBox with validation</span>

<br>
<ui5-multi-combobox style="width: 560px" placeholder="Some initial text" id="mcb-validation">
<ui5-li type="Active">Cosy</ui5-li>
<ui5-li type="Active">Compact</ui5-li>
<ui5-li type="Active">Condensed</ui5-li>
<ui5-li type="Active">Longest word in the world</ui5-li>
</ui5-multi-combobox>
</div>


<hr class="demo-section">
Expand All @@ -214,7 +236,7 @@
<br>

<span>Event call count: </span>
<input type="text" id="events-call-count" value="0"/>
<input type="text" id="events-call-count" value="0" />

<br>
<br>
Expand All @@ -224,19 +246,18 @@


<script>

const eventNameInput = document.getElementById("events-input");
const eventParamsInput = document.getElementById("events-parameters");
const callCountInput = document.getElementById("events-call-count");
const resetBtn = document.getElementById("reset-btn");

document.getElementById("mcb").addEventListener("ui5-selectionChange", function(event) {
document.getElementById("mcb").addEventListener("ui5-selectionChange", function (event) {
eventNameInput.value = "selectionChange";
eventParamsInput.value = event.detail.items.length;
callCountInput.value = parseInt(callCountInput.value) + 1;
});

resetBtn.addEventListener("click", function(event) {
resetBtn.addEventListener("click", function (event) {
eventNameInput.value = "";
eventParamsInput.value = "";
callCountInput.value = "";
Expand All @@ -245,4 +266,4 @@

</body>

</html>
</html>
52 changes: 43 additions & 9 deletions packages/main/test/specs/MultiComboBox.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ describe("MultiComboBox general interaction", () => {
const popover = browser.$("#multi1").shadow$(".ui5-multi-combobox-all-items-popover");

icon.click();
assert.ok(popover.isDisplayedInViewport(), "Popover should be displayed in the viewport");
assert.ok(popover.getProperty("opened"), "Popover should be displayed in the viewport");

icon.click();
assert.ok(!popover.isDisplayedInViewport(), "Popover should close");
assert.ok(!popover.getProperty("opened"), "Popover should close");
});
});

Expand All @@ -30,7 +30,7 @@ describe("MultiComboBox general interaction", () => {

icon.click();

assert.ok(popover.isDisplayedInViewport(), "Popover should be displayed in the viewport");
assert.ok(popover.getProperty("opened"), "Popover should be displayed in the viewport");
assert.equal(firstItem.getAttribute("selected"), null, "First item should not be selected");

firstItemCheckbox.click();
Expand Down Expand Up @@ -59,7 +59,7 @@ describe("MultiComboBox general interaction", () => {

const list = browser.$("#mcb").shadow$(".ui5-multi-combobox-all-items-list");

assert.ok(popover.isDisplayedInViewport(), "Popover should be displayed in the viewport");
assert.ok(popover.getProperty("opened"), "Popover should be displayed in the viewport");


assert.strictEqual(list.getProperty("items").length, 3, "3 items should be shown");
Expand Down Expand Up @@ -97,13 +97,47 @@ describe("MultiComboBox general interaction", () => {
}, 2500, "expect value state to be different after 2.5 seconds");
});

// it("tests if n more is applied and corresponding popover", () => {
// $("#more-mcb").scrollIntoView();
it("When item is clicked, the popover should be closed and the value in the input should be removed", () => {
const input = browser.$("#another-mcb").shadow$("#ui5-multi-combobox-input");
const popover = browser.$("#another-mcb").shadow$(".ui5-multi-combobox-all-items-popover");
const firstItem = browser.$("#another-mcb").shadow$(".ui5-multi-combobox-all-items-list > ui5-li");

// const nMoreText = browser.$("#more-mcb").shadow$("ui5-tokenizer").shadow$(".ui5-tokenizer-more-text");
input.click();
input.keys("c");

assert.strictEqual(popover.getProperty("opened"), true, "The popover should be opened");
assert.strictEqual(input.getValue(), "c", "Value is c (as typed)");

firstItem.click();

assert.strictEqual(popover.getProperty("opened"), false, "When the content is clicked, the popover should close");
assert.strictEqual(input.getValue(), "", "When the content is clicked, the value should be removed");
});

it("When item's checkbox is clicked, the popover should not be closed and the value in the input should be kept", () => {
const input = browser.$("#another-mcb").shadow$("#ui5-multi-combobox-input");
const popover = browser.$("#another-mcb").shadow$(".ui5-multi-combobox-all-items-popover");
const firstItemCheckbox = browser.$("#another-mcb").shadow$(".ui5-multi-combobox-all-items-list > ui5-li").shadow$("ui5-checkbox");

input.click();
input.keys("c");

assert.strictEqual(popover.getProperty("opened"), true, "The popover should be opened");
assert.strictEqual(input.getValue(), "c", "Value is c (as typed)");

// assert.ok(nMoreText.getText(), "1 More", "token 1 should be visible");
// });
firstItemCheckbox.click();

assert.strictEqual(popover.getProperty("opened"), true, "When the content is clicked, the popover should close");
assert.strictEqual(input.getValue(), "c", "When the content is clicked, the value should be removed");
});

it("tests if n more is applied and corresponding popover", () => {
$("#more-mcb").scrollIntoView();

const nMoreText = browser.$("#more-mcb").shadow$("ui5-tokenizer").shadow$(".ui5-tokenizer-more-text");

assert.ok(nMoreText.getText(), "1 More", "token 1 should be visible");
});
});

describe("keyboard handling", () => {
Expand Down

0 comments on commit 1b3e40d

Please sign in to comment.