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

Conversation

@booleanbetrayal
Copy link
Contributor

Closes #11447

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wish I had a better understanding of unknownValue usage in select + options. Had to revert the spec back to its form in ef894c8.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so the reason for this is that, if the model is null then element.val() is empty but if the model contains a value that doesn't match any option then element.val() is '?'.

When we remove the option the value becomes invalid so we get '?' but then the model is cleared out and becomes null. If we have a watch on the model then this will cause the ngModel to run $render, which update the element value to ''.

I think that what we need is to trigger a new call to $render when the selected option is removed...

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually what we need to do is fix the bit of code inside updateOptions (https://github.com/angular/angular.js/pull/11448/files#diff-0a9e86a58b2663074f51454f7a18a0efR646) that checks if the model has changed

@petebacondarwin
Copy link
Contributor

I'll take a better look over the weekend but I think this is good.

@booleanbetrayal
Copy link
Contributor Author

👍

Copy link
Member

Choose a reason for hiding this comment

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

I might add deep (equality) or something, to make it more clear.
(Technically, even in the default case, it is checking for equality (by reference).)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point! updated in rebased commit below.

@Narretz
Copy link
Contributor

Narretz commented Mar 28, 2015

There's a also regression with circular references in models which was introduced by 6a03ca2 here: #11372

@barillax
Copy link

👍

1 similar comment
@nbrustein
Copy link

👍

@petebacondarwin
Copy link
Contributor

Thanks @booleanbetrayal for this

@booleanbetrayal
Copy link
Contributor Author

np ... thanks @petebacondarwin !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ngSelect + ngOptions performs deep equality check unnecessarily in certain scenarios

7 participants