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
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/content/error/ngOptions/trkslct.ngdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.


This error occurs when 'ngOptions' is passed a comprehension expression that contains both a
`select as` expression and a `track by` expression. These two expressions are fundamentally
incompatible.
Expand Down
18 changes: 6 additions & 12 deletions src/ng/directive/select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,6 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
//re-usable object to represent option's locals
locals = {};

if (trackFn && selectAsFn) {
throw ngOptionsMinErr('trkslct',
"Comprehension expression cannot contain both selectAs '{0}' " +
"and trackBy '{1}' expressions.",
selectAs, track);
}

if (nullOption) {
// compile the element since there might be bindings in it
$compile(nullOption)(scope);
Expand Down Expand Up @@ -460,7 +453,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
function createIsSelectedFn(viewValue) {
var selectedSet;
if (multiple) {
if (!selectAs && trackFn && isArray(viewValue)) {
if (trackFn && isArray(viewValue)) {

selectedSet = new HashMap([]);
for (var trackIndex = 0; trackIndex < viewValue.length; trackIndex++) {
Expand All @@ -470,15 +463,16 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
} else {
selectedSet = new HashMap(viewValue);
}
} else if (!selectAsFn && trackFn) {
} else if (trackFn) {
viewValue = callExpression(trackFn, null, viewValue);
}

return function isSelected(key, value) {
var compareValueFn;
if (selectAsFn) {
compareValueFn = selectAsFn;
} else if (trackFn) {
if (trackFn) {
compareValueFn = trackFn;
} else if (selectAsFn) {
compareValueFn = selectAsFn;
} else {
compareValueFn = valueFn;
}
Expand Down
170 changes: 161 additions & 9 deletions test/ng/directive/selectSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -817,22 +817,174 @@ describe('select', function() {
});


/**
* This behavior is broken and should probably be cleaned up later as track by and select as
* aren't compatible.
*/
describe('selectAs+trackBy expression', function() {
beforeEach(function() {
scope.arr = [{id: 10, label: 'ten'}, {id:'20', label: 'twenty'}];
scope.obj = {'10': {score: 10, label: 'ten'}, '20': {score: 20, label: 'twenty'}};
scope.arr = [{subItem: {label: 'ten', id: 10}}, {subItem: {label: 'twenty', id: 20}}];
scope.obj = {'10': {subItem: {id: 10, label: 'ten'}}, '20': {subItem: {id: 20, label: 'twenty'}}};
});


it('should throw a helpful minerr', function() {
expect(function() {
it('It should use the "value" variable to represent items in the array as well as for the ' +
'selected values in track by expression (single&array)', function() {
createSelect({
'ng-model': 'selected',
'ng-options': 'item.subItem as item.subItem.label for item in arr track by (item.id || item.subItem.id)'
});

// First test model -> view

scope.$apply(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.

Add comment model -> view

scope.selected = scope.arr[0].subItem;
});
expect(element.val()).toEqual('0');

scope.$apply(function() {
scope.selected = scope.arr[1].subItem;
});
expect(element.val()).toEqual('1');

// Now test view -> model

element.val('0');
browserTrigger(element, 'change');
expect(scope.selected).toBe(scope.arr[0].subItem);

// Now reload the array
scope.$apply(function() {
scope.arr = [{
subItem: {label: 'new ten', id: 10}
},{
subItem: {label: 'new twenty', id: 20}
}];
});
expect(element.val()).toBe('0');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add expectation that scope.selected id is 10.

});


it('It should use the "value" variable to represent items in the array as well as for the ' +
'selected values in track by expression (multiple&array)', function() {
createSelect({
'ng-model': 'selected',
'multiple': true,
'ng-options': 'item.subItem as item.subItem.label for item in arr track by (item.id || item.subItem.id)'
});

// First test model -> view

scope.$apply(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.

Add comment before "Now reload the array". And actually replace the whole array

scope.selected = [scope.arr[0].subItem];
});
expect(element.val()).toEqual(['0']);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add expect that scope.selected.label == 'new item'

scope.$apply(function() {
scope.selected = [scope.arr[1].subItem];
});
expect(element.val()).toEqual(['1']);

// Now test view -> model

element.find('option')[0].selected = true;
element.find('option')[1].selected = false;
browserTrigger(element, 'change');
expect(scope.selected).toEqual([scope.arr[0].subItem]);

// Now reload the array
scope.$apply(function() {
scope.arr = [{
subItem: {label: 'new ten', id: 10}
},{
subItem: {label: 'new twenty', id: 20}
}];
});
expect(element.val()).toEqual(['0']);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same change as above

});

createSelect({
'ng-model': 'selected',
'ng-options': 'item.id as item.name for item in values track by item.id'
});

}).toThrowMinErr('ngOptions', 'trkslct', "Comprehension expression cannot contain both selectAs 'item.id' and trackBy 'item.id' expressions.");
it('It should use the "value" variable to represent items in the array as well as for the ' +
'selected values in track by expression (multiple&object)', function() {
createSelect({
'ng-model': 'selected',
'multiple': true,
'ng-options': 'val.subItem as val.subItem.label for (key, val) in obj track by (val.id || val.subItem.id)'
});

// First test model -> view

scope.$apply(function() {
scope.selected = [scope.obj['10'].subItem];
});
expect(element.val()).toEqual(['10']);


scope.$apply(function() {
scope.selected = [scope.obj['10'].subItem];
});
expect(element.val()).toEqual(['10']);

// Now test view -> model

element.find('option')[0].selected = true;
element.find('option')[1].selected = false;
browserTrigger(element, 'change');
expect(scope.selected).toEqual([scope.obj['10'].subItem]);

// Now reload the object
scope.$apply(function() {
scope.obj = {
'10': {
subItem: {label: 'new ten', id: 10}
},
'20': {
subItem: {label: 'new twenty', id: 20}
}
};
});
expect(element.val()).toEqual(['10']);
});


it('It should use the "value" variable to represent items in the array as well as for the ' +
'selected values in track by expression (single&object)', function() {
createSelect({
'ng-model': 'selected',
'ng-options': 'val.subItem as val.subItem.label for (key, val) in obj track by (val.id || val.subItem.id)'
});

// First test model -> view

scope.$apply(function() {
scope.selected = scope.obj['10'].subItem;
});
expect(element.val()).toEqual('10');


scope.$apply(function() {
scope.selected = scope.obj['10'].subItem;
});
expect(element.val()).toEqual('10');

// Now test view -> model

element.find('option')[0].selected = true;
browserTrigger(element, 'change');
expect(scope.selected).toEqual(scope.obj['10'].subItem);

// Now reload the object
scope.$apply(function() {
scope.obj = {
'10': {
subItem: {label: 'new ten', id: 10}
},
'20': {
subItem: {label: 'new twenty', id: 20}
}
};
});
expect(element.val()).toEqual('10');
});
});

Expand Down