Skip to content

Commit

Permalink
fix(combobox): clear custom input value on blur (#8070)
Browse files Browse the repository at this point in the history
**Related Issue:** #2162 

## Summary

Clears input value on `blur` if the value has no matching
`combobox-item` when `allowCustomValue` property is set to `false`
  • Loading branch information
anveshmekala authored Oct 27, 2023
1 parent 7df7e1f commit 327ff06
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,7 @@ export class ComboboxItem implements ConditionalSlotComponent, InteractiveCompon
this.selected = !this.selected;
}

itemClickHandler = (event: MouseEvent): void => {
event.preventDefault();
private itemClickHandler = (): void => {
this.toggleSelected();
};

Expand Down
120 changes: 67 additions & 53 deletions packages/calcite-components/src/components/combobox/combobox.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { CSS as ComboboxItemCSS } from "../combobox-item/resources";
import { CSS as XButtonCSS } from "../functional/XButton";
import { getElementXY, skipAnimations } from "../../tests/utils";

const selectionModes = ["single", "single-persist", "ancestors", "multiple"];

describe("calcite-combobox", () => {
describe("renders", () => {
renders("calcite-combobox", { display: "block" });
Expand Down Expand Up @@ -490,6 +492,7 @@ describe("calcite-combobox", () => {

await selectItem(item1);
expect(await combobox.getProperty("value")).toBe("one");
expect(await combobox.getProperty("open")).toBe(true);
});

it("multiple-selection mode allows toggling selection once the selected item is selected", async () => {
Expand Down Expand Up @@ -1085,31 +1088,6 @@ describe("calcite-combobox", () => {
expect(eventSpy).toHaveReceivedEventTimes(2);
});

it("should clear the input on blur when filtered items are empty", async () => {
await page.waitForChanges();
const combobox = await page.find("calcite-combobox");
const inputEl = await page.find(`#myCombobox >>> input`);

await inputEl.focus();
await page.waitForChanges();
expect(await page.evaluate(() => document.activeElement.id)).toBe("myCombobox");
await page.keyboard.type("asdf");
await page.waitForChanges();
await page.keyboard.press("Tab");
await page.waitForChanges();
expect(await combobox.getProperty("value")).toBe("");

combobox.setProperty("selectionMode", "single");
await inputEl.focus();
await page.waitForChanges();
expect(await page.evaluate(() => document.activeElement.id)).toBe("myCombobox");
await page.keyboard.type("asdf");
await page.waitForChanges();
await page.keyboard.press("Tab");
await page.waitForChanges();
expect(await combobox.getProperty("value")).toBe("");
});

describe("keyboard interaction with chips", () => {
let element;
let chips;
Expand Down Expand Up @@ -1784,18 +1762,17 @@ describe("calcite-combobox", () => {
});
});

describe("clicked outside", () => {
describe("custom input value when clicked outside of the component", () => {
let page: E2EPage;

beforeEach(async () => {
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 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>
<calcite-button id="button">click me</calcite-button>
`
);
});
Expand All @@ -1805,42 +1782,79 @@ describe("calcite-combobox", () => {
combobox.setProperty("selectionMode", selectionMode);
combobox.setProperty("allowCustomValues", allowCustomValues);
const inputEl = await page.find(`#myCombobox >>> input`);
const buttonEl = await page.find("calcite-button");

await inputEl.focus();
await page.waitForChanges();
expect(await page.evaluate(() => document.activeElement.id)).toBe("myCombobox");
await inputEl.type("asdf");
await page.waitForChanges();

if (allowCustomValues) {
await inputEl.press("Enter");
await buttonEl.click();
await page.waitForChanges();
expect(await page.evaluate(() => document.activeElement.id)).toBe("button");
expect(await combobox.getProperty("value")).toBe("asdf");
} else {
await buttonEl.click();
await page.waitForChanges();
expect(await page.evaluate(() => document.activeElement.id)).toBe("button");
expect(await combobox.getProperty("value")).toBe("");
}
const comboboxRect = await page.evaluate(() => {
const comboboxEl = document.querySelector("#myCombobox");
return comboboxEl.getBoundingClientRect().toJSON();
});

await inputEl.type("three");
await page.waitForChanges();
await page.mouse.move(10, 2 * comboboxRect.bottom);
await page.mouse.down();
await page.waitForChanges();
await page.mouse.up();
await page.waitForChanges();
expect(await page.evaluate(() => document.activeElement.id)).not.toBe("myCombobox");
expect(await inputEl.getProperty("value")).toBe("");
expect(await combobox.getProperty("value")).toBe(allowCustomValues ? "three" : "");
}

it("should clear input value in single selectionMode", async () => {
await assertClickOutside("single");
});
selectionModes.forEach((mode) => {
it(`should clear input value when selectionMode=${mode} `, async () => {
await assertClickOutside(mode);
});

it("should not clear input value in single selectionMode with allowCustomValues", async () => {
await assertClickOutside("single", true);
it(`should not clear input value when selectionMode=${mode} with allowCustomValues`, async () => {
await assertClickOutside(mode, true);
});
});
});

describe("custom input value on blur using keyboard", () => {
let page: E2EPage;

it("should clear input value in multiple selectionMode", async () => {
await assertClickOutside();
beforeEach(async () => {
page = await newE2EPage();
await page.setContent(
html`
<calcite-combobox id="myCombobox">
<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>
`
);
});

it("should not clear input value in multiple selectionMode with allowCustomValues", async () => {
await assertClickOutside("multiple", true);
async function clearInputValueOnBlur(selectionMode = "multiple", allowCustomValues = false): Promise<void> {
const combobox = await page.find("calcite-combobox");
combobox.setProperty("selectionMode", selectionMode);
combobox.setProperty("allowCustomValues", allowCustomValues);
const inputEl = await page.find(`#myCombobox >>> input`);
await inputEl.focus();
await page.waitForChanges();
expect(await page.evaluate(() => document.activeElement.id)).toBe("myCombobox");
await page.keyboard.type("three");
await page.waitForChanges();
await page.keyboard.press("Tab");
await page.waitForChanges();
await page.keyboard.press("Tab");
await page.waitForChanges();
expect(await inputEl.getProperty("value")).toBe("");
expect(await combobox.getProperty("value")).toBe(allowCustomValues ? "three" : "");
}

selectionModes.forEach((mode) => {
it(`should clear the input on blur when selectionMode=${mode}`, async () => {
await clearInputValueOnBlur(mode);
});
it(`should not clear the input on blur when selectionMode=${mode} with allowCustomValues`, async () => {
await clearInputValueOnBlur(mode, true);
});
});
});
});
47 changes: 21 additions & 26 deletions packages/calcite-components/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,23 @@ export class Combobox
return;
}

this.setInactiveIfNotContained(event);
const composedPath = event.composedPath();

if (composedPath.includes(this.el) || composedPath.includes(this.referenceEl)) {
return;
}

if (!this.allowCustomValues && this.textInput.value) {
this.clearInputValue();
this.filterItems("");
this.updateActiveItemIndex(-1);
}

if (this.allowCustomValues && this.text.trim().length) {
this.addCustomChip(this.text);
}

this.open = false;
}

@Listen("calciteComboboxItemChange")
Expand Down Expand Up @@ -580,6 +596,10 @@ export class Combobox
} else if (this.open) {
this.open = false;
event.preventDefault();
} else if (!this.allowCustomValues && this.text) {
this.clearInputValue();
this.filterItems("");
this.updateActiveItemIndex(-1);
}
break;
case "ArrowLeft":
Expand Down Expand Up @@ -761,26 +781,6 @@ export class Combobox
this.updateActiveItemIndex(targetIndex);
}

private setInactiveIfNotContained = (event: Event): void => {
const composedPath = event.composedPath();

if (!this.open || composedPath.includes(this.el) || composedPath.includes(this.referenceEl)) {
return;
}

if (!this.allowCustomValues && this.textInput.value) {
this.clearInputValue();
this.filterItems("");
this.updateActiveItemIndex(-1);
}

if (this.allowCustomValues && this.text.trim().length) {
this.addCustomChip(this.text);
}

this.open = false;
};

setFloatingEl = (el: HTMLDivElement): void => {
this.floatingEl = el;
connectFloatingUI(this, this.referenceEl, this.floatingEl);
Expand Down Expand Up @@ -1146,10 +1146,6 @@ export class Combobox
this.textInput?.focus();
};

comboboxBlurHandler = (event: FocusEvent): void => {
this.setInactiveIfNotContained(event);
};

//--------------------------------------------------------------------------
//
// Render Methods
Expand Down Expand Up @@ -1225,7 +1221,6 @@ export class Combobox
disabled={disabled}
id={`${inputUidPrefix}${guid}`}
key="input"
onBlur={this.comboboxBlurHandler}
onFocus={this.comboboxFocusHandler}
onInput={this.inputHandler}
placeholder={placeholder}
Expand Down

0 comments on commit 327ff06

Please sign in to comment.