From ef908217e9130467a23022b83f0af07754e1d2ee Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Fri, 7 Jun 2024 16:33:49 -0700 Subject: [PATCH 1/5] feat(list, filter): allow users to choose to match or ignore specific item properties #5063 --- .../calcite-components/src/components.d.ts | 8 +++ .../src/components/filter/filter.e2e.ts | 8 +++ .../src/components/filter/filter.tsx | 15 ++++- .../src/components/list/list.e2e.ts | 45 +++++++++++++ .../src/components/list/list.tsx | 1 + .../src/utils/filter.spec.ts | 66 +++++++++++++++++++ .../calcite-components/src/utils/filter.ts | 19 +++--- 7 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 packages/calcite-components/src/utils/filter.spec.ts diff --git a/packages/calcite-components/src/components.d.ts b/packages/calcite-components/src/components.d.ts index af91005d058..6ee50c7b64d 100644 --- a/packages/calcite-components/src/components.d.ts +++ b/packages/calcite-components/src/components.d.ts @@ -1805,6 +1805,10 @@ export namespace Components { * Defines the items to filter. The component uses the values as the starting point, and returns items that contain the string entered in the input, using a partial match and recursive search. This property is needed to conduct filtering. */ "items": object[]; + /** + * Specifies the fields to match against when filtering. + */ + "matchFields": string[]; /** * Use this property to override individual strings used by the component. */ @@ -9608,6 +9612,10 @@ declare namespace LocalJSX { * Defines the items to filter. The component uses the values as the starting point, and returns items that contain the string entered in the input, using a partial match and recursive search. This property is needed to conduct filtering. */ "items"?: object[]; + /** + * Specifies the fields to match against when filtering. + */ + "matchFields"?: string[]; /** * Use this property to override individual strings used by the component. */ diff --git a/packages/calcite-components/src/components/filter/filter.e2e.ts b/packages/calcite-components/src/components/filter/filter.e2e.ts index b38fce1cfda..b24f21cf53b 100644 --- a/packages/calcite-components/src/components/filter/filter.e2e.ts +++ b/packages/calcite-components/src/components/filter/filter.e2e.ts @@ -277,6 +277,14 @@ describe("calcite-filter", () => { await page.waitForTimeout(DEBOUNCE_TIMEOUT); assertMatchingItems(await filter.getProperty("filteredItems"), ["harry"]); }); + + it("should return no matching values", async () => { + const filter = await page.find("calcite-filter"); + filter.setProperty("matchFields", ["description"]); + await page.waitForChanges(); + await page.waitForTimeout(DEBOUNCE_TIMEOUT); + assertMatchingItems(await filter.getProperty("filteredItems"), []); + }); }); describe("filter method", () => { diff --git a/packages/calcite-components/src/components/filter/filter.tsx b/packages/calcite-components/src/components/filter/filter.tsx index 125c688c461..195bf218f4a 100644 --- a/packages/calcite-components/src/components/filter/filter.tsx +++ b/packages/calcite-components/src/components/filter/filter.tsx @@ -81,6 +81,16 @@ export class Filter */ @Prop({ mutable: true }) filteredItems: object[] = []; + /** + * Specifies the fields to match against when filtering. + */ + @Prop() matchFields: string[]; + + @Watch("matchFields") + matchFieldsHandler(): void { + this.filterDebounced(this.value); + } + /** * Specifies placeholder text for the input element. */ @@ -159,7 +169,7 @@ export class Filter async componentWillLoad(): Promise { setUpLoadableComponent(this); if (this.items.length) { - this.updateFiltered(filter(this.items, this.value)); + this.updateFiltered(filter(this.items, this.value, this.matchFields)); } await setUpMessages(this); } @@ -223,7 +233,8 @@ export class Filter private filterDebounced = debounce( (value: string, emit = false, onFilter?: () => void): void => - this.items.length && this.updateFiltered(filter(this.items, value), emit, onFilter), + this.items.length && + this.updateFiltered(filter(this.items, value, this.matchFields), emit, onFilter), DEBOUNCE_TIMEOUT, ); diff --git a/packages/calcite-components/src/components/list/list.e2e.ts b/packages/calcite-components/src/components/list/list.e2e.ts index 5296eb23fe1..9fd1d5b7bd9 100755 --- a/packages/calcite-components/src/components/list/list.e2e.ts +++ b/packages/calcite-components/src/components/list/list.e2e.ts @@ -373,6 +373,51 @@ describe("calcite-list", () => { expect(visibleItems.map((item) => item.id)).toEqual(["label-match", "description-match", "value-match"]); }); + it("filters initially with matchFields", async () => { + const page = await newE2EPage(); + await page.setContent(html` + + + + + + + `); + + await page.waitForChanges(); + const list = await page.find("calcite-list"); + list.setProperty("matchFields", ["label"]); + await page.waitForChanges(); + await page.waitForTimeout(listDebounceTimeout); + + expect(await list.getProperty("filteredItems")).toHaveLength(1); + expect(await list.getProperty("filteredData")).toHaveLength(1); + + const visibleItems = await page.findAll("calcite-list-item:not([filter-hidden])"); + + expect(visibleItems.map((item) => item.id)).toEqual(["label-match"]); + }); + it("should support shift click to select multiple items", async () => { const clickItemContent = (item: HTMLCalciteListItemElement, selector: string) => { item.shadowRoot.querySelector(selector).dispatchEvent(new MouseEvent("click", { bubbles: true, shiftKey: true })); diff --git a/packages/calcite-components/src/components/list/list.tsx b/packages/calcite-components/src/components/list/list.tsx index 06e4c43b583..34b97bc1ca2 100755 --- a/packages/calcite-components/src/components/list/list.tsx +++ b/packages/calcite-components/src/components/list/list.tsx @@ -558,6 +558,7 @@ export class List aria-label={filterPlaceholder} disabled={disabled} items={dataForFilter} + matchFields={["label", "description", "metadata"]} onCalciteFilterChange={this.handleFilterChange} placeholder={filterPlaceholder} ref={this.setFilterEl} diff --git a/packages/calcite-components/src/utils/filter.spec.ts b/packages/calcite-components/src/utils/filter.spec.ts new file mode 100644 index 00000000000..bbcf81fc2fe --- /dev/null +++ b/packages/calcite-components/src/utils/filter.spec.ts @@ -0,0 +1,66 @@ +import { filter } from "./filter"; + +describe("filter function", () => { + it("warns and returns empty array for empty data", () => { + const originalWarn = console.warn; + console.warn = jest.fn(); + const result = filter([], "test"); + expect(console.warn).toHaveBeenCalled(); + expect(result).toEqual([]); + console.warn = originalWarn; + }); + + it("returns empty array when no objects match", () => { + const data = [{ name: "John" }, { name: "Jane" }]; + const result = filter(data, "Doe"); + expect(result).toEqual([]); + }); + + it("returns matching objects", () => { + const data = [{ name: "John" }, { name: "Jane" }]; + const result = filter(data, "Jane"); + expect(result).toEqual([{ name: "Jane" }]); + }); + + it("considers only specified fields for matching", () => { + const data = [ + { name: "John", age: 30 }, + { name: "Jane", age: 25 }, + ]; + const result = filter(data, "25", ["age"]); + expect(result).toEqual([{ name: "Jane", age: 25 }]); + + const result2 = filter(data, "John", ["age"]); + expect(result2).toEqual([]); + }); + + it("ignores functions and null values in objects", () => { + const data = [{ name: "John", action: () => {}, value: null }]; + const result = filter(data, "John"); + expect(result).toEqual([{ name: "John", action: expect.any(Function), value: null }]); + }); + + it("searches nested objects correctly", () => { + const data = [{ name: "John", details: { age: 30 } }]; + const result = filter(data, "30"); + expect(result).toEqual([{ name: "John", details: { age: 30 } }]); + }); + + it("searches arrays in objects correctly", () => { + const data = [{ name: "John", tags: ["developer", "tester"] }]; + const result = filter(data, "tester"); + expect(result).toEqual([{ name: "John", tags: ["developer", "tester"] }]); + }); + + it("always includes objects with filterDisabled set to true", () => { + const data = [{ name: "John" }, { name: "Jane", filterDisabled: true }]; + const result = filter(data, "Doe"); + expect(result).toEqual([{ name: "Jane", filterDisabled: true }]); + }); + + it("always includes objects with constant set to true", () => { + const data = [{ name: "John" }, { name: "Jane", constant: true }]; + const result = filter(data, "Doe"); + expect(result).toEqual([{ name: "Jane", constant: true }]); + }); +}); diff --git a/packages/calcite-components/src/utils/filter.ts b/packages/calcite-components/src/utils/filter.ts index d94fda4bce3..e8c595ce5be 100644 --- a/packages/calcite-components/src/utils/filter.ts +++ b/packages/calcite-components/src/utils/filter.ts @@ -1,6 +1,6 @@ import { escapeRegExp, forIn } from "lodash-es"; -export const filter = (data: Array, value: string): Array => { +export const filter = (data: Array, value: string, matchFields?: string[]): Array => { const escapedValue = escapeRegExp(value); const regex = new RegExp(escapedValue, "i"); @@ -9,16 +9,22 @@ export const filter = (data: Array, value: string): Array => { The data argument should be an array of objects`); } - const find = (input: object, RE: RegExp) => { + const find = (input: object, RE: RegExp, fields?: string[]) => { if ((input as any)?.constant || (input as any)?.filterDisabled) { return true; } + let found = false; - forIn(input, (val) => { + forIn(input, (val: any, key) => { if (typeof val === "function" || val == null /* intentional == to catch undefined */) { return; } + + if (fields && !fields.includes(key)) { + return; + } + if (Array.isArray(val) || (typeof val === "object" && val !== null)) { if (find(val, RE)) { found = true; @@ -27,12 +33,9 @@ export const filter = (data: Array, value: string): Array => { found = true; } }); + return found; }; - const result = data.filter((item) => { - return find(item, regex); - }); - - return result; + return data.filter((item) => find(item, regex, matchFields)); }; From 5a39e5916bd5d04e0329a3fc19aca2f96181ab31 Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Fri, 7 Jun 2024 16:49:47 -0700 Subject: [PATCH 2/5] fix test --- .../src/components/list/list.e2e.ts | 49 +------------------ 1 file changed, 2 insertions(+), 47 deletions(-) diff --git a/packages/calcite-components/src/components/list/list.e2e.ts b/packages/calcite-components/src/components/list/list.e2e.ts index 9fd1d5b7bd9..edabe6524e1 100755 --- a/packages/calcite-components/src/components/list/list.e2e.ts +++ b/packages/calcite-components/src/components/list/list.e2e.ts @@ -365,59 +365,14 @@ describe("calcite-list", () => { const list = await page.find("calcite-list"); await page.waitForTimeout(listDebounceTimeout); - expect(await list.getProperty("filteredItems")).toHaveLength(3); - expect(await list.getProperty("filteredData")).toHaveLength(3); + expect(await list.getProperty("filteredItems")).toHaveLength(2); + expect(await list.getProperty("filteredData")).toHaveLength(2); const visibleItems = await page.findAll("calcite-list-item:not([filter-hidden])"); expect(visibleItems.map((item) => item.id)).toEqual(["label-match", "description-match", "value-match"]); }); - it("filters initially with matchFields", async () => { - const page = await newE2EPage(); - await page.setContent(html` - - - - - - - `); - - await page.waitForChanges(); - const list = await page.find("calcite-list"); - list.setProperty("matchFields", ["label"]); - await page.waitForChanges(); - await page.waitForTimeout(listDebounceTimeout); - - expect(await list.getProperty("filteredItems")).toHaveLength(1); - expect(await list.getProperty("filteredData")).toHaveLength(1); - - const visibleItems = await page.findAll("calcite-list-item:not([filter-hidden])"); - - expect(visibleItems.map((item) => item.id)).toEqual(["label-match"]); - }); - it("should support shift click to select multiple items", async () => { const clickItemContent = (item: HTMLCalciteListItemElement, selector: string) => { item.shadowRoot.querySelector(selector).dispatchEvent(new MouseEvent("click", { bubbles: true, shiftKey: true })); From a44149063925b0f0479694b15b4bc0f6d1e7457a Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Fri, 7 Jun 2024 17:03:44 -0700 Subject: [PATCH 3/5] fix test --- packages/calcite-components/src/components/list/list.e2e.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/calcite-components/src/components/list/list.e2e.ts b/packages/calcite-components/src/components/list/list.e2e.ts index edabe6524e1..fea84942044 100755 --- a/packages/calcite-components/src/components/list/list.e2e.ts +++ b/packages/calcite-components/src/components/list/list.e2e.ts @@ -370,7 +370,7 @@ describe("calcite-list", () => { const visibleItems = await page.findAll("calcite-list-item:not([filter-hidden])"); - expect(visibleItems.map((item) => item.id)).toEqual(["label-match", "description-match", "value-match"]); + expect(visibleItems.map((item) => item.id)).toEqual(["label-match", "description-match"]); }); it("should support shift click to select multiple items", async () => { From 2dc67aecc93182f212048167d258d6b62f89c269 Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Wed, 12 Jun 2024 14:21:26 -0700 Subject: [PATCH 4/5] review fixes --- .../src/components/filter/filter.tsx | 2 +- .../calcite-components/src/utils/filter.spec.ts | 13 +++++++++---- packages/calcite-components/src/utils/filter.ts | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/calcite-components/src/components/filter/filter.tsx b/packages/calcite-components/src/components/filter/filter.tsx index 195bf218f4a..7f473ce7f30 100644 --- a/packages/calcite-components/src/components/filter/filter.tsx +++ b/packages/calcite-components/src/components/filter/filter.tsx @@ -82,7 +82,7 @@ export class Filter @Prop({ mutable: true }) filteredItems: object[] = []; /** - * Specifies the fields to match against when filtering. + * Specifies the fields to match against when filtering. This will only apply when `value` is an object. If not set, all fields will be matched. */ @Prop() matchFields: string[]; diff --git a/packages/calcite-components/src/utils/filter.spec.ts b/packages/calcite-components/src/utils/filter.spec.ts index bbcf81fc2fe..ac5f45cc64e 100644 --- a/packages/calcite-components/src/utils/filter.spec.ts +++ b/packages/calcite-components/src/utils/filter.spec.ts @@ -2,12 +2,11 @@ import { filter } from "./filter"; describe("filter function", () => { it("warns and returns empty array for empty data", () => { - const originalWarn = console.warn; - console.warn = jest.fn(); + const spy = jest.spyOn(console, "warn"); const result = filter([], "test"); - expect(console.warn).toHaveBeenCalled(); + expect(spy).toHaveBeenCalledTimes(1); expect(result).toEqual([]); - console.warn = originalWarn; + spy.mockRestore(); }); it("returns empty array when no objects match", () => { @@ -40,6 +39,12 @@ describe("filter function", () => { expect(result).toEqual([{ name: "John", action: expect.any(Function), value: null }]); }); + it("returns empty array when searching with 'null'", () => { + const data = [{ name: "John", action: () => {}, value: null }]; + const result = filter(data, "null"); + expect(result).toEqual([]); + }); + it("searches nested objects correctly", () => { const data = [{ name: "John", details: { age: 30 } }]; const result = filter(data, "30"); diff --git a/packages/calcite-components/src/utils/filter.ts b/packages/calcite-components/src/utils/filter.ts index e8c595ce5be..8ab36c624f4 100644 --- a/packages/calcite-components/src/utils/filter.ts +++ b/packages/calcite-components/src/utils/filter.ts @@ -16,7 +16,7 @@ export const filter = (data: Array, value: string, matchFields?: string[ let found = false; - forIn(input, (val: any, key) => { + forIn(input, (val, key) => { if (typeof val === "function" || val == null /* intentional == to catch undefined */) { return; } From c66218314cfcc6b938016cf7edeb6a216595d50c Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Wed, 12 Jun 2024 14:52:24 -0700 Subject: [PATCH 5/5] revert list changes --- packages/calcite-components/src/components/list/list.e2e.ts | 6 +++--- packages/calcite-components/src/components/list/list.tsx | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/calcite-components/src/components/list/list.e2e.ts b/packages/calcite-components/src/components/list/list.e2e.ts index fea84942044..5296eb23fe1 100755 --- a/packages/calcite-components/src/components/list/list.e2e.ts +++ b/packages/calcite-components/src/components/list/list.e2e.ts @@ -365,12 +365,12 @@ describe("calcite-list", () => { const list = await page.find("calcite-list"); await page.waitForTimeout(listDebounceTimeout); - expect(await list.getProperty("filteredItems")).toHaveLength(2); - expect(await list.getProperty("filteredData")).toHaveLength(2); + expect(await list.getProperty("filteredItems")).toHaveLength(3); + expect(await list.getProperty("filteredData")).toHaveLength(3); const visibleItems = await page.findAll("calcite-list-item:not([filter-hidden])"); - expect(visibleItems.map((item) => item.id)).toEqual(["label-match", "description-match"]); + expect(visibleItems.map((item) => item.id)).toEqual(["label-match", "description-match", "value-match"]); }); it("should support shift click to select multiple items", async () => { diff --git a/packages/calcite-components/src/components/list/list.tsx b/packages/calcite-components/src/components/list/list.tsx index 34b97bc1ca2..06e4c43b583 100755 --- a/packages/calcite-components/src/components/list/list.tsx +++ b/packages/calcite-components/src/components/list/list.tsx @@ -558,7 +558,6 @@ export class List aria-label={filterPlaceholder} disabled={disabled} items={dataForFilter} - matchFields={["label", "description", "metadata"]} onCalciteFilterChange={this.handleFilterChange} placeholder={filterPlaceholder} ref={this.setFilterEl}