feat(directive): ngName

[sjbarker]

Request Type: feature

How to reproduce:

Component(s): forms

Impact: medium

Complexity: small

This issue is related to:

Detailed Description:

Handle evaluation of angular expressions for control names. Create a directive that sets the element's name as well as the control's name, prior to the control being added to the form.

Other Comments:

Currently there are multiple pull requests for an end result that it seems multiple people want - an evaluated control name. Doing so through binding causes issues with maintaining the adding and removing of controls to their parent. Most (if not, all) open issues on this subject make changes to the NgModelController. I approached it this way to avoid making any changes in existing controllers or directives (so this is a genuine "new" feature) and prevents changes on the expression from mucking up the controls.

The NgModelController is instantiated prior to the markup being compiled. So currently the following will not work either:

<input type="text" ng-model="something" name="{{ myControl }}">

The name attribute will be set on the element, however the $name property on the controller would be "{{ myControl }}".

This commit accomplishes the task through the use of a directive. Consider the following markup:

<input type="text" ng-model="something" ng-name="myControl">

Where the scope expressions may be:

scope.something = 123;
scope.myControl = "testControl";

The order we are concerned with is getting the $name property on the NgModelController for this control to be "testControl" and equally importantly, prior to the control being added to the parent form (if it exists).

This directive jumps in between the NgModelController instantiation and the ngModel linking function to set the name attribute and property and the $name controller property, prior to the control being added.

So, for a final example, if you wanted to build your form with ng-repeat using a data model:

<form name="thisForm">
  <div ng-repeat="c in controls">
    <input type="text" ng-model="c.val" ng-name="'control' + c.code" required><br>
    <span>True or false, am I $valid? - 
      <b ng-bind="thisForm.{{ 'control' + c.code }}.$valid"></b>
    </span>
  </div>
</form>
<div>
  thisForm.controlA1.$valid = {{ thisForm.controlA1.$valid }}
  ...

scope.controls = [
  {
    code: 'A1',
    val: ''
  },
  {
    code: 'B2',
    val: ''
  },
  {
    code: 'C1',
    val: ''
  }
];

[mary-poppins]

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

• Uses the issue template (#6569)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

[Burgov]

This issue seems to be the same as #5736 (but a slightly different approach)

[sjbarker]

@Burgov yes it seems so. Unfortunately, I gave up trying to find all of the issues related to this because there were so many. Perhaps this PR will help. What I definitely want the community to take into consideration is that this is a directive and does not alter the forms current functionality. That issue seems to want to change the way forms work at their core, which is much heavier than needed right now, IMHO. I build most of my forms by a data model. I also namespace my controls writing some forms. This is a directive that I have used to accomplish both tasks, without breaking anything else.

[caitp]

@sjbarker, both of the solutions for this (there are only two) are literally 1 line fixes, I don't think you're going to find a simpler fix.

That said, Matias is working on an alternative solution, so we have yet to decide between them. His version seems much more complicated for dubious benefits though, (based on my understanding of the description), so I'm still leaning towards "simple one-line fix"

[sjbarker]

@caitp, excellent point! A one-line fix is definitely the most attractive approach. I couldn't find any discussion on why this issue has been around for so long, so I took this approach.

Using a directive also allows for expansion on the fact that, currently, once the name is set you are done. ngModel adds the control and if the expression on name changes explicitly there is no response on the controller's part. And $name is the real reason why. I feel like using a directive will leave room for expanding on this idea of dynamic forms.

So, in the future if a control's name needs to bind to data and handle adding and removing controls, which I have seen a few use cases for (too few to commit right now), changes in the controller simply won't do. But expanding the functionality of the directive that handles that would.

My thoughts were "make the developer explicitly say, 'I want to eval/interpolate/parse this expression for name, and I understand that once I do it, it's done', instead of implicity doing it on the controller. To be honest, I guess I assumed that this is why this fix wasn't initially implemented and why a fix has yet to surface and be merged.

[sjbarker]

@caitp - I had found these open requests as well (one of which includes yours): #4086, #3135, and #4791. Are there any others that you know of?

[Rajat-Chowdhary]

@sjbarker : I couldn't get a few things. By running the directive at priority 100 do you ensure that it runs between NgModelController and Ngmodel Link function? Also, as it is a one time bind, as in there are no watchers or observe method on the expression, would it be worthy to have a observe function?

Thanks

[sjbarker]

@Rajat-Chowdhary : I am glad you brought this up! I can definitely see a later need for observing the attribute, however I didn't find it necessary immediately. If you do allow the name to change then you must remove the old control from it's form and add the new control with the new name (if a form exists). This exposed so much room for error when maintaining forms that I excluded it. Since the ngModelController doesn't handle adding controls to forms and the ngModelDirective linking function does, I found that having a directive to handle the interception was the best approach. This way someone else could add the functionality of form maintenance to the directive in the future.

And yes, both pre-linking and running at 100 ensures that the linking function runs before the ngModel's link function. Pre-linking alone may do it, but I haven't tested that and wanted to ensure that, if anything, I was explicit on the need for this to run before the control could be added to a form.

[Rajat-Chowdhary]

@sjbarker : Thanks so much for the detailed reply. i get it now and really appreciate the idea of why doing it through a directive is a better approach.

Like what you said, I guess pre-link alone should suffice to run it before NgModelDirective.

Thanks again.

[sjbarker]

No problem @Rajat-Chowdhary! Thank you.

[phuongnd08]

👍 Found this to be very useful. I'm currently stuck at trying to write a directive that wrap around an input and a span that will be displayed when the input run into validation issue. With regular name="{{name}}" inside the directive template, the form will try to validate {{name}} which is odd. Using the directive ngName that @sjbarker created, the form is validating the proper field that I passed to the custom directive initially.

[myitcv]

+1

[petebacondarwin]

This is not needed as the require property will throw if the ngModel is not there, unlike if you had made the require optional: require: '?ngModel'

[sjbarker]

Thanks @petebacondarwin! Good call!

[sjbarker]

This solution has become unstable and incompatible with the current build so I am closing this pull request. Thanks for the attention @caitp and @petebacondarwin and others.

[0x-r4bbit]

@caitp do you know if @matsko 's solution landed already? And if so, can you point me to the commit?

[shahata]

This was fixed in 729c238