Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(combobox): clear custom input value on blur #8070

Merged
merged 3 commits into from
Oct 27, 2023
Merged

fix(combobox): clear custom input value on blur #8070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9564238
fix(combobox): clear custom input value on blur
anveshmekala Oct 27, 2023
28a1e6c
refactor and update test
anveshmekala Oct 27, 2023
4a1db7d
feedback changes
anveshmekala Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/calcite-components/src/components/combobox-item/combobox-item.tsx
View file
Open in desktop
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
View file
Open in desktop
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) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: for readability, can you add a newline above this to make it easier to scan?

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
View file
Open in desktop
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