feat(limitTo): Add support for numbers in limitTo #8926
Conversation
|
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
|
I have signed the CLA. Let me know if there is anything else I need to do. |
|
You mean like this? http://plnkr.co/edit/iwmnK1NB5KgFNfodeINk?p=preview |
|
Ah, forget about it. I didn't understand your comment at first. |
|
No I mean like http://plnkr.co/edit/jQK9OuAP8TWVhL1vmkxY?p=preview |
|
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
|
So why not use https://docs.angularjs.org/api/ng/filter/number? |
| @@ -17,6 +19,8 @@ describe('Filter: limitTo', function() { | |||
| expect(limitTo(items, '3')).toEqual(['a', 'b', 'c']); | |||
| expect(limitTo(str, 3)).toEqual("tuv"); | |||
| expect(limitTo(str, '3')).toEqual("tuv"); | |||
| expect(limitTo(number, 3)).toEqual("100"); | |||
| expect(limitTo(number, '3')).toEqual("100"); | |||
|
I don't totally understand the use-case, but it looks basically okay to me --- just clean up the indentation and commit messages and squash (see https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit). I'll see what people think about this today I guess |
|
Awesome will do! My use case was for numerical values which you do not want to format with commas and/or for non-homogenous data sets (which have either a string or a number). For example barcode or ssn or id, etc.... |
feat (limitTo) : Add support for numbers in limitTo Added support for numbers as input in limitTo filter : - Will treat number as a string and limit number of places shown. Closes angular#8926
|
Squashed and committed with appropriate messaging. I am getting some strange errors on the TravisCI only in regards to jqlite (tests pass locally) -> Timed out waiting for Protractor to synchronize with the page after 0 seconds. Please see https://github.com/angular/protractor/blob/master/docs/faq.md. I can work on fixing this, but wanted to know if what people thought (so I don't take up time if it is a feature that people are against). Look forward to hearing your thoughts. |
|
Don't worry about the timeout, it's just flakes, most likely (will take a look). The commit message however still does not follow the message guidelines |
Added support for numbers as input in limitTo filter : - Will treat number as a string and limit number of places shown. Closes angular#8926
|
Sorry, had added the subject in the body. Fixed it. |
hjast
changed the title
| @@ -72,6 +72,7 @@ | |||
| */ | |||
| function limitToFilter(){ | |||
| return function(input, limit) { | |||
| if (typeof input == "number") input = input.toString(); | |||
There was a problem hiding this comment.
should use isString
wait nvm, misread that
Added support for numbers as input in limitTo filter : - Will treat number as a string and limit number of places shown. Closes angular#8926
| @@ -72,6 +72,7 @@ | |||
| */ | |||
| function limitToFilter(){ | |||
| return function(input, limit) { | |||
| if (isNumber(input)) input = input.toString(); | |||
There was a problem hiding this comment.
this PR is missing the type signature / doc update
Added support for numbers as input in limitTo filter : - Will treat number as a string and limit number of places shown. Closes angular#8926
Added support for numbers as input in limitTo filter : - Will treat number as a string and limit number of places shown. Closes angular#8926
Added support for numbers as input in limitTo filter : - Will treat number as a string and limit number of places shown. Closes angular#8926# Please enter the commit message for your changes. Lines starting
|
I have added documentation and end to end tests. Please let me know if what I added suffice. Thanks! |
|
landed as 1c8a745. Thanks! |
It would be nice if limitTo supports numbers. The expected behavior would change input to string and just run limitTo on that value. This becomes useful often when you have long decimal numbers, etc...