Skip to content

fix(list): changing filterText property will now update filtered items #7133

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

Merged
merged 24 commits into from
Jun 28, 2023
Merged

fix(list): changing filterText property will now update filtered items #7133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4f794d5
feat(filter): Add filter method. #6633
driskull Jun 5, 2023
c8a2518
Merge branch 'master' into dris0000/filter-method
driskull Jun 5, 2023
63c510b
fix(list): changing filterText property will now update filtered item…
driskull Jun 6, 2023
4d1f1c0
cleanup
driskull Jun 6, 2023
17527cc
Merge branch 'dris0000/filter-method' into dris0000/list-filter-fix
driskull Jun 6, 2023
07aeeea
cleanup
driskull Jun 6, 2023
9620145
Merge branch 'master' into dris0000/filter-method
driskull Jun 6, 2023
9f54ead
cleanup
driskull Jun 6, 2023
8b4d8a8
Merge branch 'dris0000/filter-method' of github.com:Esri/calcite-comp…
driskull Jun 6, 2023
8bc9322
Merge branch 'dris0000/filter-method' into dris0000/list-filter-fix
driskull Jun 6, 2023
3a515e7
cleanup
driskull Jun 6, 2023
55669b7
cleanup
driskull Jun 6, 2023
48d23bd
cleanup
driskull Jun 6, 2023
4592095
Merge branch 'master' into dris0000/filter-method
driskull Jun 6, 2023
9414f6b
fix filteredItems
driskull Jun 6, 2023
1ca7937
Merge branch 'dris0000/filter-method' of github.com:Esri/calcite-comp…
driskull Jun 6, 2023
fa0a54a
Merge branch 'dris0000/filter-method' into dris0000/list-filter-fix
driskull Jun 6, 2023
aa0fe22
Merge branch 'master' into dris0000/list-filter-fix
driskull Jun 6, 2023
fd4f7e2
Merge branch 'master' into dris0000/filter-method
driskull Jun 8, 2023
77c7609
Merge branch 'dris0000/filter-method' into dris0000/list-filter-fix
driskull Jun 8, 2023
42ff266
review fixes
driskull Jun 9, 2023
96ec73d
review fixes
driskull Jun 16, 2023
04e92c9
Merge branch 'dris0000/filter-method' into dris0000/list-filter-fix
driskull Jun 16, 2023
67a2aff
Merge branch 'master' into dris0000/list-filter-fix
driskull Jun 28, 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
123 changes: 60 additions & 63 deletions packages/calcite-components/src/components/list/list.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,28 +105,28 @@ describe("calcite-list", () => {
);
});

it.skip("navigating items after filtering", async () => {
const page = await newE2EPage({
html: html`
<calcite-list filter-enabled>
<calcite-list-item value="one" label="One" description="hello world"></calcite-list-item>
<calcite-list-item value="two" label="Two" description="hello world"></calcite-list-item>
</calcite-list>
`
});
it("navigating items after filtering", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-list filter-enabled>
<calcite-list-item value="one" label="One" description="hello world"></calcite-list-item>
<calcite-list-item value="two" label="Two" description="hello world"></calcite-list-item>
</calcite-list>
`);
await page.waitForChanges();
const list = await page.find("calcite-list");
const filter = await page.find(`calcite-list >>> calcite-filter`);
await page.waitForTimeout(listDebounceTimeout);
expect(await list.getProperty("filteredItems")).toHaveLength(2);
expect(await list.getProperty("filteredData")).toHaveLength(2);
expect(await list.getProperty("filterText")).toBeUndefined();

await filter.callMethod("setFocus");

const calciteListFilterEvent = page.waitForEvent("calciteListFilter");
const calciteListFilterEvent = list.waitForEvent("calciteListFilter");
Copy link
Member

Choose a reason for hiding this comment

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

✨🧹✨🏆

await page.keyboard.type("one");
await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);
await calciteListFilterEvent;
expect(await list.getProperty("filteredItems")).toHaveLength(1);
expect(await list.getProperty("filteredData")).toHaveLength(1);
Expand All @@ -137,80 +137,77 @@ describe("calcite-list", () => {
await page.keyboard.press("Backspace");
await page.waitForChanges();

const calciteListFilterEvent2 = page.waitForEvent("calciteListFilter");
const calciteListFilterEvent2 = list.waitForEvent("calciteListFilter");
await page.keyboard.type("two");
await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);
await calciteListFilterEvent2;
expect(await list.getProperty("filteredItems")).toHaveLength(1);
expect(await list.getProperty("filteredData")).toHaveLength(1);
expect(await list.getProperty("filterText")).toBe("two");

const calciteListFilterEvent3 = page.waitForEvent("calciteListFilter");
const calciteListFilterEvent3 = list.waitForEvent("calciteListFilter");
await page.keyboard.type("blah");
await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);
await calciteListFilterEvent3;
expect(await list.getProperty("filteredItems")).toHaveLength(0);
expect(await list.getProperty("filteredData")).toHaveLength(0);
expect(await list.getProperty("filterText")).toBe("twoblah");
});

it.skip("filters initially", async () => {
const page = await newE2EPage({
html: html`
<calcite-list filter-enabled filter-text="match">
<calcite-list-item
id="label-match"
label="match"
description="description-1"
value="value-1"
></calcite-list-item>
<calcite-list-item
id="description-match"
label="label-2"
description="match"
value="value-1"
></calcite-list-item>
<calcite-list-item
id="value-match"
label="label-3"
description="description-3"
value="match"
></calcite-list-item>
<calcite-list-item
id="no-match"
label="label-4"
description="description-4"
value="value-4"
></calcite-list-item>
</calcite-list>
`
});
it("filters initially", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-list filter-enabled filter-text="match">
<calcite-list-item
id="label-match"
label="match"
description="description-1"
value="value-1"
></calcite-list-item>
<calcite-list-item
id="description-match"
label="label-2"
description="match"
value="value-1"
></calcite-list-item>
<calcite-list-item
id="value-match"
label="label-3"
description="description-3"
value="match"
></calcite-list-item>
<calcite-list-item
id="no-match"
label="label-4"
description="description-4"
value="value-4"
></calcite-list-item>
</calcite-list>
`);

await page.waitForChanges();
const list = await page.find("calcite-list");
await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();

expect(await list.getProperty("filteredData")).toHaveLength(3);
expect(await list.getProperty("filteredItems")).toHaveLength(3);
expect(await list.getProperty("filteredData")).toHaveLength(3);

const visibleItems = await page.findAll("calcite-list-item:not([hidden])");

expect(visibleItems.map((item) => item.id)).toEqual(["label-match", "description-match", "value-match"]);
});

it("should update active item on init and click", async () => {
const page = await newE2EPage({
html: html`<calcite-list selection-mode="none">
<calcite-list-item id="item-1" label="hello" description="world"></calcite-list-item>
<calcite-list-item id="item-2" label="hello 2" description="world 2"></calcite-list-item>
<calcite-list-item id="item-3" label="hello 3" description="world 3"></calcite-list-item>
</calcite-list>`
});

await page.waitForTimeout(listDebounceTimeout);
const page = await newE2EPage();
await page.setContent(html`<calcite-list selection-mode="none">
<calcite-list-item id="item-1" label="hello" description="world"></calcite-list-item>
<calcite-list-item id="item-2" label="hello 2" description="world 2"></calcite-list-item>
<calcite-list-item id="item-3" label="hello 3" description="world 3"></calcite-list-item>
</calcite-list>`);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);

const items = await page.findAll("calcite-list-item");

Expand All @@ -222,8 +219,8 @@ describe("calcite-list", () => {

await items[1].click();

await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);
expect(eventSpy).toHaveReceivedEventTimes(1);

expect(await items[0].getProperty("active")).toBe(false);
Expand All @@ -240,8 +237,8 @@ describe("calcite-list", () => {
</calcite-list>`
});

await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);

const items = await page.findAll("calcite-list-item");

Expand All @@ -253,8 +250,8 @@ describe("calcite-list", () => {

await items[2].click();

await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);
expect(eventSpy).toHaveReceivedEventTimes(1);

expect(await items[0].getProperty("selected")).toBe(false);
Expand All @@ -271,8 +268,8 @@ describe("calcite-list", () => {
</calcite-list>`
});

await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);

const items = await page.findAll("calcite-list-item");

Expand All @@ -284,8 +281,8 @@ describe("calcite-list", () => {

await items[2].click();

await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);
expect(eventSpy).toHaveReceivedEventTimes(1);

expect(await items[0].getProperty("selected")).toBe(false);
Expand All @@ -294,8 +291,8 @@ describe("calcite-list", () => {

await items[0].click();

await page.waitForTimeout(listDebounceTimeout);
await page.waitForChanges();
await page.waitForTimeout(listDebounceTimeout);
expect(eventSpy).toHaveReceivedEventTimes(2);

expect(await items[0].getProperty("selected")).toBe(true);
Expand Down
69 changes: 48 additions & 21 deletions packages/calcite-components/src/components/list/list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ export class List implements InteractiveComponent, LoadableComponent {
*/
@Prop({ reflect: true, mutable: true }) filterText: string;

@Watch("filterText")
async handleFilterTextChange(): Promise<void> {
this.performFilter();
}

/**
* Specifies an accessible name for the component.
*/
Expand Down Expand Up @@ -224,14 +229,6 @@ export class List implements InteractiveComponent, LoadableComponent {

componentDidLoad(): void {
setComponentLoaded(this);
const { filterEl } = this;
const filteredItems = filterEl?.filteredItems as ItemData;

if (filteredItems) {
this.filteredData = filteredItems;
}

this.updateListItems();
}

// --------------------------------------------------------------------------
Expand Down Expand Up @@ -299,11 +296,11 @@ export class List implements InteractiveComponent, LoadableComponent {
aria-label={filterPlaceholder}
disabled={loading || disabled}
items={dataForFilter}
onCalciteFilterChange={this.handleFilter}
onCalciteFilterChange={this.handleFilterChange}
placeholder={filterPlaceholder}
value={filterText}
// eslint-disable-next-line react/jsx-sort-props
ref={(el) => (this.filterEl = el)}
ref={this.setFilterEl}
/>
</th>
</tr>
Expand All @@ -323,11 +320,11 @@ export class List implements InteractiveComponent, LoadableComponent {
//
// --------------------------------------------------------------------------

handleDefaultSlotChange = (event: Event): void => {
private handleDefaultSlotChange = (event: Event): void => {
updateListItemChildren(getListItemChildren(event));
};

setActiveListItem = (): void => {
private setActiveListItem = (): void => {
const { enabledListItems } = this;

if (!enabledListItems.some((item) => item.active)) {
Expand Down Expand Up @@ -402,15 +399,45 @@ export class List implements InteractiveComponent, LoadableComponent {
}
};

handleFilter = (event: CustomEvent): void => {
private updateFilteredData(emit = false): void {
const { filterEl } = this;

if (!filterEl) {
return;
}

if (filterEl.filteredItems) {
this.filteredData = filterEl.filteredItems as ItemData;
}

this.updateListItems(emit);
}

private async performFilter(): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

Some naming suggestions: filter(), runFilter, refreshFilter(), updateFilter? Up to you.

const { filterEl, filterText } = this;

if (!filterEl) {
return;
}

filterEl.value = filterText;
await filterEl.filter(filterText);
this.updateFilteredData();
}

private setFilterEl = (el: HTMLCalciteFilterElement): void => {
this.filterEl = el;
this.performFilter();
};

private handleFilterChange = (event: CustomEvent): void => {
event.stopPropagation();
const { filteredItems, value } = event.currentTarget as HTMLCalciteFilterElement;
this.filteredData = filteredItems as ItemData;
const { value } = event.currentTarget as HTMLCalciteFilterElement;
this.filterText = value;
this.updateListItems(true);
this.updateFilteredData(true);
};

getItemData = (): ItemData => {
private getItemData = (): ItemData => {
return this.listItems.map((item) => ({
label: item.label,
description: item.description,
Expand Down Expand Up @@ -439,11 +466,11 @@ export class List implements InteractiveComponent, LoadableComponent {
this.updateSelectedItems(emit);
}, debounceTimeout);

queryListItems = (): HTMLCalciteListItemElement[] => {
private queryListItems = (): HTMLCalciteListItemElement[] => {
return Array.from(this.el.querySelectorAll(listItemSelector));
};

focusRow = (focusEl: HTMLCalciteListItemElement): void => {
private focusRow = (focusEl: HTMLCalciteListItemElement): void => {
const { enabledListItems } = this;

if (!focusEl) {
Expand All @@ -455,7 +482,7 @@ export class List implements InteractiveComponent, LoadableComponent {
focusEl.setFocus();
};

isNavigable = (listItem: HTMLCalciteListItemElement): boolean => {
private isNavigable = (listItem: HTMLCalciteListItemElement): boolean => {
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for making these private. This reminds me to look for a rule to help enforce access modifiers on class members (these can affect whether they are exposed in Stencil's generated interfaces 🕵️).

const parentListItemEl = listItem.parentElement?.closest(listItemSelector);

if (!parentListItemEl) {
Expand All @@ -465,7 +492,7 @@ export class List implements InteractiveComponent, LoadableComponent {
return parentListItemEl.open && this.isNavigable(parentListItemEl);
};

handleListKeydown = (event: KeyboardEvent): void => {
private handleListKeydown = (event: KeyboardEvent): void => {
if (event.defaultPrevented) {
return;
}
Expand Down