-
Notifications
You must be signed in to change notification settings - Fork 77
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(list): include groups in filtering #10664
Changes from 31 commits
c679a94
becaffd
053a27f
1f02133
85a0e4e
204f9e4
363863a
c36591c
9c93432
cd3c545
246de6f
15cd55d
d6f7c24
8ea0e96
977e982
52dd50c
56cad5f
59a933e
beebb86
635385a
ca9c3b6
26f0d83
6aa62f7
bdd2543
23cd88d
4bfefb6
be935d9
40d2c6f
e107542
cab883a
bbfbc6b
723d02c
adef085
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,7 @@ import type { Filter } from "../filter/filter"; | |
import type { ListItemGroup } from "../list-item-group/list-item-group"; | ||
import { CSS, debounceTimeout, SelectionAppearance, SLOTS } from "./resources"; | ||
import T9nStrings from "./assets/t9n/messages.en.json"; | ||
import { ListElement } from "./interfaces"; | ||
import { ListDragDetail, ListDisplayMode, ListMoveDetail } from "./interfaces"; | ||
import { styles } from "./list.scss"; | ||
|
||
|
@@ -82,6 +83,8 @@ export class List | |
|
||
private listItems: ListItem["el"][] = []; | ||
|
||
private listItemGroups: ListItemGroup["el"][] = []; | ||
|
||
mutationObserver = createObserver("mutation", () => { | ||
this.willPerformFilter = true; | ||
this.updateListItems(); | ||
|
@@ -160,7 +163,7 @@ export class List | |
|
||
@state() assistiveText: string; | ||
|
||
@state() dataForFilter: ItemData = []; | ||
@state() dataForFilter: ItemData[] = []; | ||
|
||
@state() hasFilterActionsEnd = false; | ||
|
||
|
@@ -224,7 +227,7 @@ export class List | |
/** Placeholder text for the component's filter input field. */ | ||
@property({ reflect: true }) filterPlaceholder: string; | ||
|
||
/** Specifies the properties to match against when filtering. If not set, all properties will be matched (label, description, metadata, value). */ | ||
/** Specifies the properties to match against when filtering. If not set, all properties will be matched (label, description, metadata, value, group heading). */ | ||
@property() filterProps: string[]; | ||
|
||
/** Text for the component's filter input field. */ | ||
|
@@ -235,7 +238,7 @@ export class List | |
* | ||
* @readonly | ||
*/ | ||
@property() filteredData: ItemData = []; | ||
@property() filteredData: ItemData[] = []; | ||
|
||
/** | ||
* The currently filtered `calcite-list-item`s. | ||
|
@@ -408,6 +411,7 @@ export class List | |
this.updateListItems(); | ||
this.setUpSorting(); | ||
this.setParentList(); | ||
this.setListItemGroups(); | ||
} | ||
|
||
async load(): Promise<void> { | ||
|
@@ -649,6 +653,10 @@ export class List | |
} | ||
} | ||
|
||
private setListItemGroups(): void { | ||
this.listItemGroups = Array.from(this.el.querySelectorAll(listItemGroupSelector)); | ||
} | ||
|
||
private handleFilterActionsStartSlotChange(event: Event): void { | ||
this.hasFilterActionsStart = slotChangeHasAssignedElement(event); | ||
} | ||
|
@@ -685,17 +693,15 @@ export class List | |
filteredItems, | ||
visibleParents, | ||
}: { | ||
el: ListItem["el"] | ListItemGroup["el"]; | ||
el: ListElement; | ||
filteredItems: ListItem["el"][]; | ||
visibleParents: WeakSet<ListItem["el"] | ListItemGroup["el"]>; | ||
visibleParents: WeakSet<ListElement>; | ||
}): void { | ||
const filterHidden = !visibleParents.has(el) && !filteredItems.includes(el as ListItem["el"]); | ||
|
||
el.filterHidden = filterHidden; | ||
|
||
const closestParent = el.parentElement.closest<ListItem["el"] | ListItemGroup["el"]>( | ||
parentSelector, | ||
); | ||
const closestParent = el.parentElement.closest<ListElement>(parentSelector); | ||
|
||
if (!closestParent) { | ||
return; | ||
|
@@ -769,7 +775,7 @@ export class List | |
} | ||
|
||
if (filterEl.filteredItems) { | ||
this.filteredData = filterEl.filteredItems as ItemData; | ||
this.filteredData = filterEl.filteredItems as ItemData[]; | ||
} | ||
|
||
this.updateListItems(); | ||
|
@@ -782,7 +788,7 @@ export class List | |
|
||
private get effectiveFilterProps(): string[] { | ||
if (!this.filterProps) { | ||
return ["description", "label", "metadata"]; | ||
return ["description", "label", "metadata", "heading"]; | ||
} | ||
|
||
return this.filterProps.filter((prop) => prop !== "el"); | ||
|
@@ -813,15 +819,27 @@ export class List | |
this.updateFilteredData(); | ||
} | ||
|
||
private getItemData(): ItemData { | ||
private getItemData(): ItemData[] { | ||
return this.listItems.map((item) => ({ | ||
label: item.label, | ||
description: item.description, | ||
metadata: item.metadata, | ||
heading: this.getGroupHeading(item), | ||
el: item, | ||
})); | ||
} | ||
|
||
private getGroupHeading(item: ListItem["el"]): string { | ||
let heading = ""; | ||
this.listItemGroups.forEach((group) => { | ||
if (group.contains(item)) { | ||
heading += group.heading; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could also store heading of ancestor as Array object. Leaning towards string concatenation for performance gains with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I was thinking array because string concat would match two words from different headings combined. Can we change to use an array here? |
||
} | ||
}); | ||
|
||
return heading; | ||
} | ||
|
||
private updateGroupItems(): void { | ||
const { el, group, scale } = this; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other alternative would be something like this:
I'm not sure which would be more performant, calling closest() a few times on connectedCallback or calling
setListItemGroups
on each mutation. Thoughts @jcfranco?