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
[DataViz] Implement exact-match filter #32818
Conversation
expect( | ||
wrapper.instance().filterRecords(records, 'filterCol', '123') | ||
).to.deep.equal([ | ||
{id: 1, filterCol: 123, chartCol: 2}, | ||
{id: 3, filterCol: 123, chartCol: 5} | ||
]); |
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.
we should be able to simplify these all of the expect statements for this to something like:
expect(
wrapper.instance().filterRecords(records, 'filterCol', '123')
).to.deep.equal([records[0], records[2]]);
let records = [ | ||
{id: 1, filterCol: 123, chartCol: 2}, | ||
{id: 2, filterCol: 456, chartCol: 3}, | ||
{id: 3, filterCol: 123, chartCol: 5}, | ||
{id: 4, filterCol: '456', chartCol: 7}, | ||
{id: 5, filterCol: 0, chartCol: 5} | ||
]; |
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.
since we're using this array everywhere, should we hoist this to the top of this describe
block and merge all of the separate instances of records
into a single array?
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.
It has different values for different tests, though. I think I prefer having a shorter list of records for each test rather than one big list to cover all tests. IMO that has the added benefit to readability of being able to see the test data and expected result in one place, rather than having to scroll back and forth
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.
beautiful tests!
Currently, exact-match filter only explicitly supports strings, numbers, booleans,
null
, and,undefined
. It is possible to have other values (such as objects or arrays) in a data table, but filtering in those cases is currently intentionally unspecified behavior.#32693 populates the first dropdown with the column names, and the second with all values in the selected column.
If both dropdowns have a value selected, we perform an exact match filter on the records
If you select filter options such that there is nothing to chart, we show a blank chart:
Links
Testing story
Reviewer Checklist: