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.

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): include groups in filtering #10511

Merged
merged 12 commits into from
Oct 31, 2024
2 changes: 1 addition & 1 deletion packages/calcite-components/.stylelintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const customFunctions = [
"medium-modular-scale",
"modular-scale",
"scale-duration",
"small-modular-scale"
"small-modular-scale",
];
// ⚠️ END OF AUTO-GENERATED CODE

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,62 @@ describe("calcite-combobox", () => {
expect(await visibleItems[0].getProperty("value")).toBe("1");
});

it("should display group and its items when filter matches group label", async () => {
const page = await newE2EPage();
await page.setContent(
html` <calcite-combobox placeholder="typing 'group1' or 'group2' should show group with all items">
<calcite-combobox-item-group id="group1" label="group1">
<calcite-combobox-item id="value1" value="value1" text-label="value1"></calcite-combobox-item>
<calcite-combobox-item id="value2" value="value2" text-label="value2"></calcite-combobox-item>
<calcite-combobox-item id="value3" value="value3" text-label="value3"></calcite-combobox-item>
</calcite-combobox-item-group>
<calcite-combobox-item-group id="group2" label="group2">
<calcite-combobox-item id="value4" value="value4" text-label="value4"></calcite-combobox-item>
<calcite-combobox-item id="value5" value="value5" text-label="value5"></calcite-combobox-item>
<calcite-combobox-item id="value6" value="value6" text-label="value6"></calcite-combobox-item>
</calcite-combobox-item-group>
</calcite-combobox>`,
);

const combobox = await page.find("calcite-combobox");
await combobox.click();
await page.waitForChanges();
await combobox.type("group");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

Copy link
Member

Choose a reason for hiding this comment

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

Would waiting for calciteComboboxFilterChange help avoid the debounce timeout? We can revisit this separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree a refactor PR would be beneficial considering it is repeated multiple times.

const one = await page.find("#value1");
const two = await page.find("#value2");
const three = await page.find("#value3");
const four = await page.find("#value4");
const five = await page.find("#value5");
const six = await page.find("#value6");
const group1 = await page.find("#group1");
const group2 = await page.find("#group2");

expect(await group1.isVisible()).toBeTruthy();
expect(await one.isVisible()).toBeTruthy();
expect(await two.isVisible()).toBeTruthy();
expect(await three.isVisible()).toBeTruthy();
expect(await group2.isVisible()).toBeTruthy();
expect(await four.isVisible()).toBeTruthy();
expect(await five.isVisible()).toBeTruthy();
expect(await six.isVisible()).toBeTruthy();

await combobox.type("1");
await page.waitForChanges();
await page.waitForTimeout(DEBOUNCE.filter);

expect(await group1.isVisible()).toBeTruthy();
expect(await one.isVisible()).toBeTruthy();
expect(await two.isVisible()).toBeTruthy();
expect(await three.isVisible()).toBeTruthy();
expect(await group2.isVisible()).toBeFalsy();
expect(await four.isVisible()).toBeFalsy();
expect(await five.isVisible()).toBeFalsy();
expect(await six.isVisible()).toBeFalsy();
});

it("should restore filter text when no items are filtered", async () => {
const page = await newE2EPage();
await page.setContent(html`
Expand All @@ -555,17 +611,17 @@ describe("calcite-combobox", () => {
await page.waitForChanges();
await combobox.type("an");
await page.waitForChanges();
await new Promise((res) => setTimeout(() => res(true), DEBOUNCE.filter));
await page.waitForTimeout(DEBOUNCE.filter);

const one = await page.find("#one");
const two = await page.find("#two");
const three = await page.find("#three");

expect(await one.isVisible()).toBeFalsy();
expect(await two.isVisible()).toBeFalsy();
expect(await three.isVisible()).toBeTruthy();

await combobox.type("m");
await new Promise((res) => setTimeout(() => res(true), DEBOUNCE.filter));
await page.waitForTimeout(DEBOUNCE.filter);
await page.waitForChanges();
expect(await one.isVisible()).toBeFalsy();
expect(await two.isVisible()).toBeFalsy();
Expand Down
21 changes: 11 additions & 10 deletions packages/calcite-components/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ import { componentOnReady, getIconScale } from "../../utils/component";
import { Validation } from "../functional/Validation";
import { IconNameOrString } from "../icon/interfaces";
import { ComboboxMessages } from "./assets/combobox/t9n";
import { ComboboxChildElement, SelectionDisplay } from "./interfaces";
import { ComboboxChildElement, SelectionDisplay, GroupData, ItemData } from "./interfaces";
import { ComboboxChildSelector, ComboboxItem, ComboboxItemGroup, CSS, IDS } from "./resources";
import {
getItemAncestors,
Expand All @@ -79,14 +79,6 @@ import {
isSingleLike,
} from "./utils";

interface ItemData {
description: string;
label: string;
metadata: Record<string, unknown>;
shortHeading: string;
value: string;
}

const isGroup = (el: ComboboxChildElement): el is HTMLCalciteComboboxItemGroupElement =>
el.tagName === ComboboxItemGroup;

Expand Down Expand Up @@ -598,6 +590,8 @@ export class Combobox

private data: ItemData[];

private groupData: GroupData[];

mutationObserver = createObserver("mutation", () => this.updateItems());

private resizeObserver = createObserver("resize", () => {
Expand Down Expand Up @@ -1087,7 +1081,7 @@ export class Combobox
);

return debounce((text: string, setOpenToEmptyState = false, emit = true): void => {
const filteredData = filter(this.data, text);
const filteredData = filter([...this.data, ...this.groupData], text);
const itemsAndGroups = this.getItemsAndGroups();

const matchAll = text === "";
Expand Down Expand Up @@ -1222,6 +1216,7 @@ export class Combobox
this.items = this.getItems();
this.groupItems = this.getGroupItems();
this.data = this.getData();
this.groupData = this.getGroupData();
this.selectedItems = this.getSelectedItems();
this.filteredItems = this.getFilteredItems();
this.needsIcon = this.getNeedsIcon();
Expand Down Expand Up @@ -1259,6 +1254,12 @@ export class Combobox
}));
}

getGroupData(): GroupData[] {
return this.groupItems.map((groupItem: HTMLCalciteComboboxItemGroupElement) => ({
label: groupItem.label,
}));
}

getNeedsIcon(): boolean {
return isSingleLike(this.selectionMode) && this.items.some((item) => item.icon);
}
Expand Down
12 changes: 12 additions & 0 deletions packages/calcite-components/src/components/combobox/interfaces.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,14 @@
export type ComboboxChildElement = HTMLCalciteComboboxItemElement | HTMLCalciteComboboxItemGroupElement;
export type SelectionDisplay = "all" | "fit" | "single";

export interface ItemData {
description: string;
label: string;
metadata: Record<string, unknown>;
shortHeading: string;
value: string;
}

export interface GroupData {
label: string;
}
Loading