-
Notifications
You must be signed in to change notification settings - Fork 69
[Data filter] Trim whitespace #7592
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
base: master
Are you sure you want to change the base?
Conversation
1b52dee to
a52fd15
Compare
hokolomopo
left a comment
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 values shown in the filter menu are now correct, but the values actually filtered when selecting a trimmed value are not. You should apply the same normalization in the evaluation of filters 🙂
(also tests are missing)
a52fd15 to
e48889f
Compare
| const cellValues = cells.map((val) => val.cellValue); | ||
| const filterValues = filterValue?.filterType === "values" ? filterValue.hiddenValues : []; | ||
| const normalizedFilteredValues = new Set(filterValues.map(toLowerCase)); | ||
| const normalizedFilteredValues = new Set(filterValues.map(normalize)); |
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.
Not sure about this, but maybe we can store the normalized values in filterValue.hiddenValues instead of applying the normalize function each time we reevaluate a filter ?
| }; | ||
| setGrid(model, grid); | ||
| createTableWithFilter(model, "A1:A5"); | ||
| updateFilterCriterion(model, "A1", { |
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.
this test passes even without this updateFilterCriterion, so I'm not sure what's the point
[Data filter] Trim whitespace and don't bother of caps vs low case when filtering Changes done in the filters by values and by criterion Task:5375214
e48889f to
953fc39
Compare

Description:
[Data filter] Trim whitespace and don't bother of caps vs low case when filtering
Task: 5375214
review checklist