fix(form): allow dynamic form names which initially evaluate to blank #11096
Conversation
|
LGTM after taking a quick look. Would be cool if someone more familiar with the form stuff have another look, though. @Narretz / @petebacondarwin ? |
| setter(scope, controller.$name, controller, controller.$name); | ||
| attr.$observe(nameAttr, function(newValue) { | ||
| if (controller.$name === newValue) return; | ||
| setter(scope, controller.$name, undefined, controller.$name); |
There was a problem hiding this comment.
I realize this line was there before - but as far as I can tell, it doesn't do anything, because two lines after the setter is called again, this time with the correct value (also, all tests pass if I remove this)
There was a problem hiding this comment.
It is to remove the controller from the scope under the old name (although it doesn't actually remove it but sets it to undefined). I've added this expectation to the form renaming tests.
There was a problem hiding this comment.
Ah, yeah, that makes a lot of sense.
jbedard
force-pushed
the
form-name-blank
branch
from
86618c2 to
0ab25bf
Compare
Conflicts: src/ng/directive/form.js Closes #11096
|
Closing this since it was merged. |
|
Was this supposed to go into master too? It looks like @Narretz only merged it into 1.3.x |
|
It's here: 410f7c6 I had to make adjustments for 1.3.x, as the setter takes different arguments. Not sure why the PR wasn't closed from the commit, maybe because the branches didn't match? |
|
It didn't close it because the commit doesn't have a |
|
But this one has :) 190ea88 |
|
Yeah but that one wasn't merged to master (the default branch for this repo) :-P |
Previously the form name watching was initialized based on the initial name value instead of the attribute being set.