feat(filterFilter): support filterFilter for object literal collection #6508
Conversation
|
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
| } | ||
| angular.forEach(collection, function(item){ | ||
| if (predicates.check(item)) { | ||
| filtered.push(item); |
There was a problem hiding this comment.
Is it good to return an array if we're iterating over an object? I'm not totally sure here.
There was a problem hiding this comment.
I think it's okay, since it's just regrouping the object's property values as iterating over a normal array would. It think it would also allow ngRepeat to pick up a speed boost on the filtered return since it's generally quicker to loop over an array than an object.
|
Okay, I'll rework the tests - thanks @caitp |
|
@caitp I thought on it more and agree with you that the filtered return should match the initial typing of the collection. I've adjusted my code and tests to account for this. It especially matters when using (key, value) notation in ngRepeat, as it previously would no longer be an object after filtering. |
| if (predicates.check(value)) { | ||
| filtered.push(value); | ||
| } | ||
| if(collectionType === 'array' && Array.prototype.filter){ |
There was a problem hiding this comment.
This is an unrelated fix, we probably don't want to do this here. It's possible that there may be a perf benefit, but often ES5 stuff like this will perform slower than a for-loop, so I think we'd want to measure this before making this change.
|
The diff is still pretty huge, because of the re-organized tests. I think that's okay though... I can see the benefit of this and most of the actual code changes look okay, so I'll see what people think about shipping this for 1.3 |
|
Totally understand the hesitation there. I you'd prefer, I can keep the object test structure entirely separate instead of just forked. I went ahead and removed the ES5 optimization for the purpose of this PR. |
filterFilter was previously only available to array/array-like collections, but ngRepeat supports iterating over object literals. This PR enables filtering functionality over object literal properties. Fixes #6490
|
Looks like there are some other filterFilter PRs appearing (#6623) - is this one being considered for 1.3? I'd like to try and keep all the tests in sync if so. |
The docs for the `flush()` method contained a few grammatical errors and were awkwardly worded. Change the explanation of the method to remove errors and read more naturally. Closes #4886
- According to https://github.com/angular/angular.js/blob/5bf81bc111a866ec65ef86c01336911e577df5df/docs/content/guide/controller.ngdoc#L166 Ctrl should be the suffix for a controller Closes #5817
This will make the following commit clearer when the update is run.
PR #5547 introduced conversion of all 0 status codes to 404 for cases where no response was recieved (previously this was done for the file:// protocol only). But this mechanism is too eager and masks legitimate cases where status 0 should be returned. This commits reverts to the previous mechanism of handling 0 status code for the file:// protocol (converting 0 to 404) while retaining the returned status code 0 for all the protocols other than file:// Fixes #6074 Fixes #6155
Now the SHA can be short/long, whateva.
This reverts commit d5294eb. It turned out to be more work and I don't wanna deal with it right now.
from our experiements it appears that the presense or absense of the from and resolved properties makes no difference on the behavior of but updates these properties with different values depending on different state of the cache and node_modules. So in order to get clean diffs during updates, we are just going to drop these properties and have a script to do this automatically. Long term this should be fixed in npm: npm/npm#3581
This is to deal with npm/npm#3581 See the previous commit for more info. Closes #6672
|
I made the mistake of not putting this PR on a separate branch, so things are getting tangled. I'm going to close this and clean up my end. |
Request Type: feature
How to reproduce: Feed an object literal into an ngRepeat to loop over its properties with a filter in use, as shown here: http://plnkr.co/edit/mMvAgQgILcBVkbT2q4z4?p=preview
Component(s): misc core
Impact: small
Complexity: small
This issue is related to:
Detailed Description:
I switched the primary filterFilter iteration method to one that supports both array and object literal collections and added tests for the object version.
Other Comments:
filterFilter was previously only available to array/array-like collections,
but ngRepeat supports iterating over object literals. This PR enables
filtering functionality over object literal properties.
Fixes #6490