-
Notifications
You must be signed in to change notification settings - Fork 1k
Data Array Enhancement #6704
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
Data Array Enhancement #6704
Conversation
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.
Functionality looks good! Couple small cleanup comments.
Can you enhance the story so that searching matches on the color names, in addition to the filtering? |
I think we should consider using |
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.
Nice job!
Complex Story Deploy Preview
What does this PR do?
Add functionality to Data's
filter()
for when a property is alist
, rather than astring
,bool
, or nested object. Allow the user to select filter values that filter upon list objects.Where should the reviewer start?
src/js/components/Data/filter.js
src/js/components/Data/stories/Complex.js
What testing has been done on this PR?
Manual and visual testing to ensure filtering upon list elements works as intended.
How should this be manually tested?
Check that existing filtering works as expected in addition to the new list filtering capabilities.
Do Jest tests follow these best practices?
screen
is used for querying.userEvent
is used in place offireEvent
.asFragment()
is used for snapshot testing.Any background context you want to provide?
What are the relevant issues?
Closes #6701
Screenshots (if appropriate)
Do the grommet docs need to be updated?
Yes - update after initial code review to include example case.
Should this PR be mentioned in the release notes?
Is this change backwards compatible or is it a breaking change?