docs(filter): clarify the comparator parameter #15827
Conversation
The previous description does not make it clear how this parameter is used with other parameters and its purpose.
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
1 similar comment
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed it! |
|
CLAs look good, thanks! |
1 similar comment
|
CLAs look good, thanks! |
src/ng/filter/filter.js
Outdated
| * the object in the array) should be considered a match. | ||
| * @param {function(actual, expected)|true|false} [comparator] A predicate to further refine the | ||
| * search results of an object or string expression. It should be used to determine if the | ||
| * actual value from the array is equal to the expected value from the filter expression. |
There was a problem hiding this comment.
TBH, I feel this change is more confusing. E.g. comparator is not used to "further refine the search results". And "should be used" sounds more like the user should do something (vs AngularJS using the function under the hood). And also actual is not necessarily a "value from the array", but can be a property or sub-property of an item in the collection/array.
src/ng/filter/filter.js
Outdated
| @@ -45,8 +45,9 @@ | |||
| * The final result is an array of those elements that the predicate returned true for. | |||
| * | |||
| * @param {function(actual, expected)|true|false} [comparator] Comparator which is used in | |||
| * determining if the expected value (from the filter expression) and actual value (from | |||
| * the object in the array) should be considered a match. | |||
| * determining if matches returned by the argument expression should be considered a match based | |||
There was a problem hiding this comment.
The matches are not returned by the expression argument. How about something like:
determining if values retrieved using
expressionshould be...
Explain that comparator is only called when expression is not a function.
The previous description does not make it clear how this parameter is used with other parameters
and its purpose.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
This is a doc update.
What is the current behavior? (You can also link to an open issue here)
N/A
What is the new behavior (if this is a feature change)?
N/A
Does this PR introduce a breaking change?
This PR does not introduce a breaking change.
Please check if the PR fulfills these requirements
Other information: