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

fix(select): do not break when removing the element (e.g. via ngIf) #15468

Closed
wants to merge 2 commits into from
Closed

fix(select): do not break when removing the element (e.g. via ngIf) #15468

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50755af
fix(select): do not throw when removing the element (e.g. via `ngIf`)
gkalpak Dec 3, 2016
7dfa950
fixup! fix(select): do not throw when removing the element (e.g. via …
gkalpak Dec 6, 2016
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
4 changes: 4 additions & 0 deletions src/ng/directive/select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,11 @@ var SelectController =
}
};

var scopeDestroyed = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use the $$destroyed property of the scope rather than tracking the state with a sentinel var?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I could. But since we were already having a $destroy callback I opted for using that instead of the $$destroy as it seems unnecssary. Can go either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

If $$destroyed would work then I would prefer that. It is one less thing to store in memory and keep track of in maintenance of the code.

$scope.$on('$destroy', function() {
// disable unknown option so that we don't do work when the whole select is being destroyed
self.renderUnknownOption = noop;
scopeDestroyed = true;
});

// Read the value of the select control, the implementation of this changes depending
Expand Down Expand Up @@ -182,6 +184,8 @@ var SelectController =
updateScheduled = true;

$scope.$$postDigest(function() {
if (scopeDestroyed) return;

updateScheduled = false;
self.ngModelCtrl.$setViewValue(self.readValue());
if (renderAfter) self.ngModelCtrl.$render();
Expand Down
16 changes: 15 additions & 1 deletion test/ng/directive/selectSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('select', function() {
formElement = jqLite('<form name="form">' + html + '</form>');
element = formElement.find('select');
$compile(formElement)(scope);
scope.$apply();
scope.$digest();
}

function compileRepeatedOptions() {
Expand Down Expand Up @@ -767,6 +767,20 @@ describe('select', function() {
expect(element).toEqualSelect([unknownValue()], '1', '2', '3');
}
);


it('should not throw when removing the element and all its children', function() {
var template =
'<select ng-model="mySelect" ng-if="visible">' +
'<option value="">--- Select ---</option>' +
'</select>';
scope.visible = true;

compile(template);

// It should not throw when removing the element
scope.$apply('visible = false');
});
});


Expand Down