-
Notifications
You must be signed in to change notification settings - Fork 27.5k
fix(orderBy): support string predicates containing non-ident characters #6144
Conversation
} | ||
return reverseComparator(function(a,b){ | ||
return compare(get(a),get(b)); | ||
return compare(get(a, {'item': a}),get(b, {'item': b})); |
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.
could we reuse this object so that we don't produce unnecessary garbage?
also please leave a note with explanation of what's going on here since it's not obvious.
otherwise looks good to me. please make the changes and merge it in.
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.
Actually, I think there might be a problem with this...
As it is, a predicate string containing periods should traverse the object as expected, but this would break it from doing that, similar to the filterFilter issue... But I'll see if I can recycle the locals.
This version is a bit more long-winded, but doesn't introduce the breaking change... Now, I'm not sure if the original behaviour of traversing nested properties was intended, as there are no tests covering it. But in either case, I'm not sure this is a terrible trade. It's just a few extra lines. |
@@ -62,17 +62,30 @@ | |||
*/ | |||
orderByFilter.$inject = ['$parse']; | |||
function orderByFilter($parse){ | |||
var QUOTED_STRING_REGEXP = /^\s*(['"])([^'"]*)(['"])\s*$/; |
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.
Could you use a back reference here for matching the closing quote to the opening one? If it is of any benefit to cut the check match[1] === match[3]
:
/^\s*(['"])([^'"]*)(['"])\s*$/
could be rewritten as /^\s*(['"])([^'"]*)(\1)\s*$/
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.
I'll try that, didn't know js regexps supported backreferences
can you try something like
|
The orderBy filter now allows string predicates passed to the orderBy filter to make use property name predicates containing non-ident strings, such as spaces or percent signs, or non-latin characters. This behaviour requires the predicate string to be double-quoted. In markup, this might look like so: ```html <div ng-repeat="item in items | orderBy:'\"Tip %\"'"> ... </div> ``` Or in JS: ```js var sorted = $filter('orderBy')(array, ['"Tip %"', '-"Subtotal $"'], false); ``` Closes angular#6143
The orderBy filter now allows string predicates passed to the orderBy filter to make use property name predicates containing non-ident strings, such as spaces or percent signs, or non-latin characters. This behaviour requires the predicate string to be double-quoted. In markup, this might look like so: ```html <div ng-repeat="item in items | orderBy:'\"Tip %\"'"> ... </div> ``` Or in JS: ```js var sorted = $filter('orderBy')(array, ['"Tip %"', '-"Subtotal $"'], false); ``` Closes #6143 Closes #6144
The orderBy filter now allows string predicates passed to the orderBy filter to make use property name predicates containing non-ident strings, such as spaces or percent signs, or non-latin characters. This behaviour requires the predicate string to be double-quoted. In markup, this might look like so: ```html <div ng-repeat="item in items | orderBy:'\"Tip %\"'"> ... </div> ``` Or in JS: ```js var sorted = $filter('orderBy')(array, ['"Tip %"', '-"Subtotal $"'], false); ``` Closes angular#6143 Closes angular#6144
The orderBy filter now allows string predicates passed to the orderBy filter
to make use property name predicates containing non-ident strings, such as
spaces or percent signs, or non-latin characters.
Closes #6143