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

[List / List items] Support custom filtering supporting a filter function with a prop #6544

Closed
nwhittaker opened this issue Feb 28, 2023 · 26 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. c-list Issues that pertain to the calcite-list component enhancement Issues tied to a new feature or request. estimate - 5 A few days of work, definitely requires updates to tests. impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone p - medium Issue is non core or affecting less that 60% of people using the library

Comments

@nwhittaker
Copy link
Contributor

nwhittaker commented Feb 28, 2023

Description

A way to opt in to an stripped-down behavior when clicking a list-item that contains nested list-items.

Originally included selection and filtering asks, now placed in two separate issues:

Acceptance Criteria

Filtering a list of items with nested list-items:

  • Parent list-item is visible when it does not match but any leaf items do match.
  • Parent list-item is not visible when it does match but no descendent leaf items match.
  • Parent list-item is not visible when no descendent leaf items match.
  • Consider [calcite-filter] Add support for custom filter function #3454 as part of the upcoming effort

Relevant Info

No response

Which Component

calcite-list-item

Example Use Case

The Field Maps web app would like to use the calcite-list element to display the layers in a map. For group layers, nested layers are shown as child list-items.

Selecting a leaf layer navigates the app for further handling of the layer. However, the app does not provide further handling for group layers. They are treated more like folders that organize sets of layers -- so clicking them in the list should have no effect other than to drill down to leaf layers.

Esri team

ArcGIS Field Apps

@nwhittaker nwhittaker added enhancement Issues tied to a new feature or request. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Feb 28, 2023
@github-actions github-actions bot added the ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. label Feb 28, 2023
@nwhittaker nwhittaker changed the title Alternative selection behavior for list-items with children Stripped-down selection behavior for list-items with children Feb 28, 2023
@nwhittaker nwhittaker added the impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone label Mar 8, 2023
@brittneytewks brittneytewks added the need more info Issues that are missing information and/or a clear, actionable description. label Mar 22, 2023
@brittneytewks
Copy link

brittneytewks commented Mar 22, 2023

Scheduling time with FieldApps to understand use case in more depth, will add priority and estimate after discussion

@nwhittaker nwhittaker changed the title Stripped-down selection behavior for list-items with children Stripped-down selection/filtering behavior for list-items with children Mar 22, 2023
@nwhittaker
Copy link
Contributor Author

Added some stripped-down filtering behavior to the issue description that should also be part of any discussion.

@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Mar 29, 2023
@geospatialem geospatialem added design Issues that need design consultation prior to development. and removed needs triage Planning workflow - pending design/dev review. need more info Issues that are missing information and/or a clear, actionable description. labels Apr 24, 2023
@geospatialem
Copy link
Member

Reallocating to the May release for design considerations.

@geospatialem geospatialem added the p - medium Issue is non core or affecting less that 60% of people using the library label May 2, 2023
@ashetland
Copy link
Contributor

Propose we model the changes to List selection behaviors after what is outlined in issue #6912 for Tree. This likely means adding a children selection mode and modifying the behaviors of single. I've started mocking this up today.

@ashetland
Copy link
Contributor

ashetland commented May 9, 2023

Selection modes

Final proposal for revising the existing single and multiple selection-modes and adding children and multichildren can be found in this Figma file. See also the image below. This would be consistent with changes to Tree in issue #6912 and pr #6926.

Filtering

Since the parent would still be selectable in children and multichildren, I propose the current filtering behavior be retained for these modes.

For the revised single and multiple, however, the behaviors outlined in the original post should be adopted:

Filtering a list of items with nested list-items:

Parent list-item is visible when it does not match but any leaf items do match.
Parent list-item is not visible when it does match but no descendent leaf items match.
Parent list-item is not visible when no descendent leaf items match.

Nested indentation levels

With these new/revised selection modes, nested indentation behavior should be reconsidered as part of issue #6632.

@nwhittaker, let me know if we've missed anything here.

@ashetland ashetland added ready for dev Issues ready for development implementation. and removed design Issues that need design consultation prior to development. labels May 10, 2023
@ashetland ashetland removed their assignment May 10, 2023
@geospatialem geospatialem added 0 - new New issues that need assignment. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels May 10, 2023
@geospatialem
Copy link
Member

@geospatialem @DitwanP what would you expect this custom filter prop on the list to be named?

Naming is hard 😅 - was trying to think of a way we could add filter... to the beginning part of the prop name for consistency with other filter props, but came up empty. @driskull Dig the direction you headed with customFilter. 🥇

@nwhittaker
Copy link
Contributor Author

@geospatialem @DitwanP what would you expect this custom filter prop on the list to be named?

Naming is hard 😅 - was trying to think of a way we could add filter... to the beginning part of the prop name for consistency with other filter props, but came up empty. @driskull Dig the direction you headed with customFilter. 🥇

A predicate function is one that always returns true or false. How about filterPredicate?

@driskull
Copy link
Member

driskull commented Dec 13, 2024

@geospatialem @DitwanP Should the custom filter function require that filterText be defined? (not empty string) or should we allow the custom filter function to worry about the filterText property? I lean towards not having a requirement that it be set. It could allow users do their own filtering independent of the UI.

The other question is should filterPredicate work even when filterEnabled is false?

@driskull
Copy link
Member

Actually, I think i like the filterPredicate, ill go with that.

@geospatialem
Copy link
Member

Should the custom filter function require that filterText be defined? (not empty string) or should we allow the custom filter function to worry about the filterText property? I lean towards not having a requirement that it be set. It could allow users do their own filtering independent of the UI.

Also lean towards not having a defined filterText be a requirement, otherwise it might make the story a bit confusing for devs to implement.

The other question is should filterPredicate work even when filterEnabled is false?

Maybe we should lean towards supporting it regardless of filterEnabled? If so, could this potentially accommodate #10258?

@driskull
Copy link
Member

Maybe we should lean towards supporting it regardless of filterEnabled? If so, could this potentially accommodate #10258?

Agreed.

In the future, we should still allow filterText to work without the filter UI. This would require decoupling filtering from the ui.

driskull added a commit that referenced this issue Dec 17, 2024
…11044)

**Related Issue:** #6544

## Summary

- add `filterPredicate` property.
  - `filterPredicate: (item: ListItem["el"]) => boolean;`
- add e2e test
- cleanup 

## Example

```js
list.filterPredicate = (item) => {
  if (list.filterText === "all") {
    return true;
  }

  return item.value === "item2";
};
```
@driskull driskull added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Dec 17, 2024
@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned driskull Dec 17, 2024
Copy link
Contributor

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Dec 18, 2024
@geospatialem
Copy link
Member

Verified with 3.0.0-next.76 and https://codepen.io/geospatialem/pen/vEBxZqy

const listEl = document.getElementById("custom-list");

listEl.filterPredicate = (item) => {
  if (listEl.filterText === "test") {
    return true;
  }

  return item.value === "item2";
};

driskull added a commit that referenced this issue Dec 19, 2024
…11109)

**Related Issue:** #6544

## Summary

- fix to not require filterEnabled to use filterPredicate
- It was basically the `(changes.has("filterPredicate") &&
this.hasUpdated)` that was the culprit.
- add test

BEGIN_COMMIT_OVERRIDE
END_COMMIT_OVERRIDE
driskull added a commit that referenced this issue Dec 20, 2024
**Related Issue:** #6544

## Summary

docs(list): make filterPredicate property optional.

BEGIN_COMMIT_OVERRIDE
END_COMMIT_OVERRIDE
benelan pushed a commit that referenced this issue Feb 8, 2025
…11044)

**Related Issue:** #6544

## Summary

- add `filterPredicate` property.
  - `filterPredicate: (item: ListItem["el"]) => boolean;`
- add e2e test
- cleanup 

## Example

```js
list.filterPredicate = (item) => {
  if (list.filterText === "all") {
    return true;
  }

  return item.value === "item2";
};
```
benelan pushed a commit that referenced this issue Feb 8, 2025
…11109)

**Related Issue:** #6544

## Summary

- fix to not require filterEnabled to use filterPredicate
- It was basically the `(changes.has("filterPredicate") &&
this.hasUpdated)` that was the culprit.
- add test

BEGIN_COMMIT_OVERRIDE
END_COMMIT_OVERRIDE
benelan pushed a commit that referenced this issue Feb 8, 2025
**Related Issue:** #6544

## Summary

docs(list): make filterPredicate property optional.

BEGIN_COMMIT_OVERRIDE
END_COMMIT_OVERRIDE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. c-list Issues that pertain to the calcite-list component enhancement Issues tied to a new feature or request. estimate - 5 A few days of work, definitely requires updates to tests. impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone p - medium Issue is non core or affecting less that 60% of people using the library
Projects
None yet
Development

No branches or pull requests

8 participants