-
Notifications
You must be signed in to change notification settings - Fork 431
case insensitive text filter #190
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
case insensitive text filter #190
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.
@makenova how about we make it configurable ?! Keep current behavior as default and add case insensitive props, if ok, I can show you how to change or I can handle it, thanks
return cellStr === filterVal; | ||
} | ||
return cellStr.indexOf(filterVal) > -1; | ||
const re = RegExp(filterVal, 'i'); |
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 that danger? for example \test\
or \\test\\
, maybe throw some err?
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.
Like if the filterVal
is \test\
?
I’ll do some testing to see if that can cause an error.
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.
yes, thanks !!
Making it configureable is a great idea! |
@makenova I think we can allow the text filter to have caseInsensitive via add a new else {
// select default comparator is EQ, others are LIKE
const { comparator = (filterType === FILTER_TYPE.SELECT ? EQ : LIKE) } = filter.props;
const caseSensitive = filter.props.caseSensitive || false;
currFilters[dataField] = { filterVal, filterType, comparator, caseSensitive };
}
store.filters = currFilters; then in https://github.com/react-bootstrap-table/react-bootstrap-table2/blob/master/packages/react-bootstrap-table2-filter/src/filter.js#L19, depends on the Let me know if you got any question, thank you |
I've updated it with |
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.
LGTM
@makenova thanks your contribution 👍 |
For issue #189