-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(select): keep ngModel when selected option is recreated with ngRe… #15632
Conversation
Looks like my test passes without the patch 😨 Let's see |
Test has been fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (with one optional suggestion).
expect(scope.obj.value).toBe('B'); | ||
|
||
scope.$apply(function() { | ||
// Only when new objects are used, ngRepeat re-creates the element from scratch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make it more explicit, I would add an expactation that optionElements[1] (before) !== optionElements[0] (after)
.
Updated, PTAL |
var previouslySelectedOptionElement = optionElements[1]; | ||
optionElements = element.find('option'); | ||
expect(optionElements.length).toEqual(3); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misplaced newline?
f1866fe
to
4e0501b
Compare
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
1 similar comment
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
4e0501b
to
cba1178
Compare
CLAs look good, thanks! |
1 similar comment
CLAs look good, thanks! |
This is a really bad breakage, why isn't this in 1.6 already? Is it waiting for something I can help with? |
@Narretz right, wasn't looking properly, but I'm still having issues with |
@odedniv I don't know of any other issues and since my crystal ball is still in repair, I can't say what might be going on with your selects. Please open a new issue with a minimal reproduction of the problem. Thanks! |
…peat
Fixes #15630