-
Notifications
You must be signed in to change notification settings - Fork 405
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
#10488: 'ilike' as default operator for text field in attribute table quick filter #10496
#10488: 'ilike' as default operator for text field in attribute table quick filter #10496
Conversation
…attribute table quick filter Description: - handling specifying ilike as the default operator in DD operators in attribute table for the text fields
@@ -50,7 +50,7 @@ class AttributeFilter extends React.PureComponent { | |||
booleanOperators: ["="], | |||
defaultOperators: ["=", ">", "<", ">=", "<=", "<>", "isNull"], | |||
timeDateOperators: ["=", ">", "<", ">=", "<=", "<>", "><", "isNull"], | |||
operator: this.props.isWithinAttrTbl ? "=" : "", | |||
operator: this.props.isWithinAttrTbl ? (this.props.operator || '=') : "", |
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.
@mahmoudadel54 The this.props.operator
was introduced in a previous PR #9743 and in this new PR is used to apply a default value. Few questions:
- What was the role of the prop
operator
before this change? It was used? - If the prop
operator
was existing before why do we are using it to apply a default value?
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.
@allyoucanmap
this.props.operator ---> it is used in BaseDateTimeFilter component - which inherits from AttributeFilter class component - in onChange handlers but after double checking I think I can remove it and just use the operator state
for the second point, I think again I can just use initiate the state operator based on this.props.type to implement ilike in case of string field
If Ok, I will push the code for that, thanks
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.
@mahmoudadel54 I would suggest to introduce a specific prop for the default value such as defaultOperator, we cannot use type because the current request is a change just to the attribute table while the usage of type could effect also the query builder.
About the removal of this.props.operetor
we should do it only if we are 100% sure it's not really used
@@ -50,7 +50,7 @@ class AttributeFilter extends React.PureComponent { | |||
booleanOperators: ["="], | |||
defaultOperators: ["=", ">", "<", ">=", "<=", "<>", "isNull"], | |||
timeDateOperators: ["=", ">", "<", ">=", "<=", "<>", "><", "isNull"], | |||
operator: this.props.isWithinAttrTbl ? "=" : "", | |||
operator: this.props.isWithinAttrTbl ? (this.props.operator || '=') : "", |
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.
@mahmoudadel54 I would suggest to introduce a specific prop for the default value such as defaultOperator, we cannot use type because the current request is a change just to the attribute table while the usage of type could effect also the query builder.
About the removal of this.props.operetor
we should do it only if we are 100% sure it's not really used
…attribute table quick filter [resolve review comments] Description: - replace operator prop with defaultOperator - remove unused prop operator from BaseDateTimeFilter component
@ElenaGallo please test this fix on dev, thanks |
Description
In this PR, handling specifying 'ilike' as the default operator in DD operators in attribute table for the text fields is implemented and adding a unit test for that.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
#10488
What is the current behavior?
#10488
What is the new behavior?
Now the attribute table shows 'ilike' operator for the text fields as a default operator for quick filter.
Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)
Other useful information