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
Fixed DataFilters to show all filter options #6943
Conversation
// build from a piece of data, ignore object values | ||
filtersFor = Object.keys(data[0]).filter( | ||
(k) => typeof data[0][k] !== 'object', | ||
filtersFor = Object.keys(unfilteredData[0]).filter( |
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.
Is it reasonable for us to only be looking at the first object to build up the default options? This assumes the first object is representative of all objects in the data set and has all properties. What if other objects have properties not represented?
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.
I think it's reasonable given they can specify properties if they have sparse objects for some reason
// build from a piece of data, ignore object values | ||
filtersFor = Object.keys(data[0]).filter( | ||
(k) => typeof data[0][k] !== 'object', | ||
filtersFor = Object.keys(unfilteredData[0]).filter( |
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.
I think it's reasonable given they can specify properties if they have sparse objects for some reason
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.
Looks good!
What does this PR do?
Closes #6940.
Uses
unfilteredData
instead ofdata
as source to build up default filter options. Previously, the logic was built up fromdata
, however ifdata
reached a length of zero, then no object was available to build up the options.Where should the reviewer start?
What testing has been done on this PR?
How should this be manually tested?
Use the following stories:
properties
is not specified, data provides default filters.properties
is specified,properties
becomes the schema definition for which properties are filterable by filters.For each:
Story w/out
properties
prop:Story with
properties
prop: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?
#6940
Screenshots (if appropriate)
Do the grommet docs need to be updated?
No.
Should this PR be mentioned in the release notes?
Yes.
Is this change backwards compatible or is it a breaking change?
Backwards compatible.