-
Notifications
You must be signed in to change notification settings - Fork 27.4k
docs(ngModel.NgModelController): describe your change... #15272
Conversation
If there's no else, then the same will happen in both inputs (the model value is overwritten).
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
1 similar comment
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
1 similar comment
CLAs look good, thanks! |
Although the example is broken, this fix is not correct. As described above the example, what it tries to showcase is setting the model (i.e. So, in both cases, it must set the model to an empty string and the difference is that when your don't cancel the pending update, it will overwrite the empty string you set once the field is blurred. In order to see that, the model value has to already be the empty string, which is not the case in this example. That's why it seems/is broken. So, the proper fix is initializing both models to an empty string inside the controller: $scope.model = {value1: '', value2: ''}; @juboba, would you like to update your PR with the above change? |
@gkalpak yes, I'd like to update my PR, but I'm checking in this in plnkr: https://plnkr.co/edit/ZtqpkoOhHWdJQwOTF9mY?p=preview You can see the value is being reset to the previous one, but the model is then set to '', so the viewValue is also changed to '' |
If you remove the |
@juboba Are you planning on completing the changes for this PR? |
Hi Martin, I'm confused with Georgios Kalpakas' reply. Do you agree that the expected "Injustice anywhere is a threat to justice everywhere." -- Martin Luther On Mon, Oct 31, 2016 at 6:00 PM, Martin Staffa notifications@github.com
|
Yes, I think for the purpose of this example, @gkalpak's explanation is correct. It's a very specific scenario that shows that you need $rollBackViewValue in some cases when you have non-immediate scope updates. |
Update commit to match @gkalpak 's observations.
I updated my commit according to @gkalpak's explanation. |
For the example to work correctly, the initial model values have to be empty strings. Closes #15272
For the example to work correctly, the initial model values have to be empty strings. Closes angular#15272
For the example to work correctly, the initial model values have to be empty strings. Closes angular#15272
For the example to work correctly, the initial model values have to be empty strings. Closes angular#15272
For the example to work correctly, the initial model values have to be empty strings. Closes angular#15272
For the example to work correctly, the initial model values have to be empty strings. Closes angular#15272
For the example to work correctly, the initial model values have to be empty strings. Closes #15272
For the example to work correctly, the initial model values have to be empty strings. Closes #15272
For the example to work correctly, the initial model values have to be empty strings. Closes angular#15272
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix: The model should be cleared out only if we don't allow rollback.
What is the current behavior? (You can also link to an open issue here)
Currently, the model is always being cleared out to ''
What is the new behavior (if this is a feature change)?
The new behaviour matches the docs.
Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Other information:
If there's no else, then the same will happen in both inputs (the model value is overwritten).