Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(select): add basic track by and select as support #9533

Closed
wants to merge 1 commit into from

Conversation

jeffbcross
Copy link
Contributor

Instead of throwing an error when using "track by" and "select as" expressions,
ngOptions will assume that the track by expression is valid, and will use it to
compare values.

@jeffbcross
Copy link
Contributor Author

TODO: add tests for multiple select, and object data source

@jeffbcross jeffbcross added this to the 1.3.0-rc.6 milestone Oct 9, 2014
scope.obj = {'10': {score: 10, label: 'ten'}, '20': {score: 20, label: 'twenty'}};
});


it('should throw a helpful minerr', function() {
expect(function() {
it('should bind to scope value through experession while tracking/identifying objects', function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"It should use the "value" variable to represent items in the array as well as for the selected values in track by expression"

Instead of throwing an error when using "track by" and "select as" expressions,
ngOptions will assume that the track by expression is valid, and will use it to
compare values.

Closes angular#6564
@jeffbcross
Copy link
Contributor Author

@tbosch I added more test cases, for single/multiple and object/array. I also added a note to the error page and opened #9538 as a reminder to remove the error page later.

@@ -3,6 +3,9 @@
@fullName Comprehension expression cannot contain both `select as` and `track by` expressions.
@description

NOTE: This error was introduced in 1.3.0-rc.5, and was removed for 1.3.0-rc.6 in order to
accommodate existing apps that write complex enough `track by` expressions to work with `select as`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"to not break existing apps" and remove the "complex enough" part onward.

@jeffbcross
Copy link
Contributor Author

Landed addfff3

@jeffbcross jeffbcross closed this Oct 10, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants