Skip to content

Commit

Permalink
fix(combobox): only allow deleting visible chips with the keyboard (#…
Browse files Browse the repository at this point in the history
…8603)

**Related Issue:** #8469 

## Summary

This PR fixes a bug where pressing the Delete/Backspace key in combobox
resulted in hidden chips being removed, which wasn't intuitive or
expected. The new behavior is deleting items is only allowed in `all`
selection-display mode or `fit` when there are no overflowed chips.

---------

Co-authored-by: Erik Harper <[email protected]>
  • Loading branch information
eriklharper and eriklharper authored Jan 17, 2024
1 parent 4964491 commit 2d38241
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 9 deletions.
100 changes: 91 additions & 9 deletions packages/calcite-components/src/components/combobox/combobox.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ describe("calcite-combobox", () => {
}
});

describe("keyboard navigation", () => {
describe("keyboard navigation in all selection-display mode", () => {
let page: E2EPage;
const scrollablePageSizeInPx = 2400;
// PageUp/Down scroll test fails without the delay
Expand All @@ -898,10 +898,10 @@ describe("calcite-combobox", () => {
page = await newE2EPage();
await page.setContent(html`
<calcite-combobox id="myCombobox">
<calcite-combobox-item id="one" value="one" label="one"></calcite-combobox-item>
<calcite-combobox-item id="two" value="two" label="two"></calcite-combobox-item>
<calcite-combobox-item-group label="Last Item">
<calcite-combobox-item id="three" value="three" label="three"></calcite-combobox-item>
<calcite-combobox-item id="one" value="one" text-label="one"></calcite-combobox-item>
<calcite-combobox-item id="two" value="two" text-label="two"></calcite-combobox-item>
<calcite-combobox-item-group text-label="Last Item">
<calcite-combobox-item id="three" value="three" text-label="three"></calcite-combobox-item>
</calcite-combobox-item-group>
</calcite-combobox>
`);
Expand Down Expand Up @@ -1121,16 +1121,98 @@ describe("calcite-combobox", () => {
expect(chips.length).toEqual(2);
});

it("should delete last chip on Delete", async () => {
it("should delete last item on Delete", async () => {
expect((await element.getProperty("selectedItems")).length).toBe(3);
await element.click();

await element.press("Backspace");
chips = await page.findAll("#myCombobox >>> calcite-chip");
expect(chips.length).toEqual(2);
expect((await element.getProperty("selectedItems")).length).toBe(2);
});
});
});

describe("deleting items with the keyboard in single and fit selection-display modes", () => {
it("should not delete any items on Delete in single selection-display mode", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-combobox id="myCombobox" selection-display="single">
<calcite-combobox-item id="one" value="one" label="one"></calcite-combobox-item>
<calcite-combobox-item id="two" value="two" label="two"></calcite-combobox-item>
<calcite-combobox-item-group label="Last Item">
<calcite-combobox-item id="three" value="three" label="three"></calcite-combobox-item>
</calcite-combobox-item-group>
</calcite-combobox>
`);

const element = await page.find("#myCombobox");
await element.click();

const item1 = await page.find("calcite-combobox-item#one");
const item2 = await page.find("calcite-combobox-item#two");
const item3 = await page.find("calcite-combobox-item:last-child");
await item1.click();
await item2.click();
await item3.click();

await element.click();
await element.press("Backspace");
expect((await element.getProperty("selectedItems")).length).toBe(3);
});

it("should not delete any items on Delete in fit selection-display mode when there are overflowed chips", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-combobox id="myCombobox" selection-display="fit" style="width:350px">
<calcite-combobox-item id="one" value="one" text-label="one"></calcite-combobox-item>
<calcite-combobox-item id="two" value="two" text-label="two"></calcite-combobox-item>
<calcite-combobox-item-group text-label="Last Item">
<calcite-combobox-item id="three" value="three" text-label="three"></calcite-combobox-item>
</calcite-combobox-item-group>
</calcite-combobox>
`);

const element = await page.find("#myCombobox");
await element.click();

const item1 = await page.find("calcite-combobox-item#one");
const item2 = await page.find("calcite-combobox-item#two");
const item3 = await page.find("calcite-combobox-item:last-child");
await item1.click();
await item2.click();
await item3.click();

await element.click();
await element.press("Backspace");
expect((await element.getProperty("selectedItems")).length).toBe(3);
});

it("should delete last item on Delete in fit selection-display mode when there are no overflowed chips", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-combobox id="myCombobox" selection-display="fit" style="width:400px">
<calcite-combobox-item id="one" value="one" text-label="one"></calcite-combobox-item>
<calcite-combobox-item id="two" value="two" text-label="two"></calcite-combobox-item>
<calcite-combobox-item-group text-label="Last Item">
<calcite-combobox-item id="three" value="three" text-label="three"></calcite-combobox-item>
</calcite-combobox-item-group>
</calcite-combobox>
`);

const element = await page.find("#myCombobox");
await element.click();

const item1 = await page.find("calcite-combobox-item#one");
const item2 = await page.find("calcite-combobox-item#two");
const item3 = await page.find("calcite-combobox-item:last-child");
await item1.click();
await item2.click();
await item3.click();

await element.click();
await element.press("Backspace");
expect((await element.getProperty("selectedItems")).length).toBe(2);
});
});

describe("calciteComboboxChange", () => {
it("should have 1 selectedItem when single select", async () => {
const page = await newE2EPage();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,12 @@ export class Combobox
break;
case "Delete":
case "Backspace":
const notDeletable =
this.selectionDisplay === "single" ||
(this.selectionDisplay === "fit" && this.selectedHiddenChipsCount > 0);
if (notDeletable) {
return;
}
if (this.activeChipIndex > -1) {
event.preventDefault();
this.removeActiveChip();
Expand Down

0 comments on commit 2d38241

Please sign in to comment.