angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.8k stars 27.49k forks source link

Form control is overridden when nesting with ng-form and ng-repeat #10165

Closed hanneskaeufler closed 8 years ago

hanneskaeufler commented 9 years ago

Say I have a form which creates nested forms via ng-repeat and ng-form.

  <form name="categoryForm">
    <div ng-repeat="item in items">
      <ng-form name="tags">
        Tag Name: <input name="name" ng-model="item.name" required>
        <div ng-if="tags.name.$invalid">This is great from the inside!</div>
      </ng-form>
    </div>
  </form>

This is great for accessing the form control tags.name inside the ng-form block. But from the outside, say in the controller, there is no way to get the same control.

$scope.categoryForm.tags is always set to the last control created by the repeated ng-form.

See http://plnkr.co/edit/SvBAxJFioTIgfs7Wvvk1?p=preview

It seems $scope.categoryForm.tags should rather be an array of FormControllers or something instead of only the last FormController.

Currently this has me creating ugly workarounds like

<ng-repeat="...">
     <ng-form name="tags[{{ $index }}]">...</ng-form>
</ng-repeat>

which is possible since https://github.com/angular/angular.js/issues/1404 got closed.

Could we support something like an array of FormControllers or is this creating any other problems I´m not thinking of?

Narretz commented 9 years ago

It would definitely make the api / code more complex. Why are you creating a new form anyway if you only have one input in it?

hanneskaeufler commented 9 years ago

Just to keep the example short :-) In a recent case I stumbled across I have two selects inside the ng-form. Handling the display of validation errors in this <ng-if="..."> style you usually see in examples is surely going to end up in a maintenance hell. To prevent that we attach validation errors from the server side to the angular controls. Another directive then inspects the form controllers $valid state and handles display / highlighting of invalid form fields / errors.

jrmiddleton commented 9 years ago

Looking at both w3c documentation (http://www.w3.org/MarkUp/html3/forms.html) and the HTML5 proposal (http://www.w3.org/TR/2011/WD-html5-20110525/forms.html#the-form-element). Both state that a form element should not have child elements that are forms.

HTML 3: There can be several forms in a single document, but the FORM element can't be nested. HTML 5: Flow content, but with no form element descendants.

So I guess you can expect odd behavior from this as it isn't really an expected use case.

gkalpak commented 9 years ago

@jrmiddleton: ngForm's purpose is to overcome that exact limitation of HTML. So, this is an expected use-case and there shouldn't be any issues.

@hanneskaeufler: That said, I don't find it that ugly to use the $index property to name your nested forms. Using arrays would also require using $index to target a specific ngForm and thay would add extra complexity just to accomodate a limited number of use-cases.

Maybe, you could keep things more readable by using ngInit:

<form name="categoryForm">
    <div ng-repeat="item in items" ng-init="subForm='tags'+$index">
        <ng-form name="{{subForm}}">
            Tag Name:
            <input name="name" ng-model="item.name" required />
            <div ng-if="this[subForm].name.$invalid">This is great from the inside!</div>
            <!-- OR -->
            <div ng-if="categoryForm[subForm].name.$invalid">This is great from the inside!</div>
        </ng-form>
    </div>
</form>

Demo fiddle

hanneskaeufler commented 9 years ago

Hey @gkalpak, I like the ngInit idea, thanks. I guess what I don´t like about manually indexing the sub-forms is that multiple parts of my app have to know about the way I fake-serialize the subforms. Was it tags[0], tags_0 or `tags0? is now encoded in the template and any directive that performs upon that knowledge. Having something in the angular-core would kind of get that knowledge away from me haha :)

philBrown commented 9 years ago

I'd love to see nested forms used in ng-repeat added to an array as well.

We use server-side validation in our app and the validation contexts reference the controls using something like resource.collection[0].field. As it stands, I'm using the ng-init idea above but this means I need to transform the context paths into something like resource.collection_0.field when resolving form controls. It would be much cleaner if I could simply run the response string through $parse to resolve the form control without having to transform it.

SriramaDanturthi commented 9 years ago

I Updated the Sample I Added a Add Button and i am adding new item to the collection then the form is not creating http://jsfiddle.net/2ewe3uct/5/ It has tags 0 to 9 but its not adding tag10,tag11 by clicking on add button.

abobwhite commented 9 years ago

I've just run across this issue. It appears this worked at one point as recently as last March via http://stackoverflow.com/questions/22248733/angularjs-form-validation-within-ng-repeat.

The ng-init solution isn't relevant to my use-case because the ng-form needs to be on the same element as the ng-repeat because it is a table and using <ng-form> inside a <tr> gets tossed by the browser since it only allows <td>. A workaround I'm using is to actually ng-repeat on <tbody> and then use ng-form on my <tr>. It's not valid but it works.

gkalpak commented 9 years ago

@abobwhite, can you provide a demo of what is not working ?

dohomi commented 9 years ago

+1 I'm looking for this for a while now. Would be great if a syntax like this would be supported:

<div ng-repeat="exercise in training.exercises">
<input  max="60" min="0" type="number"
       ng-model="training.exercises[$index].duration"
       name="duration">
<div ng-messages="form.duration.$error"
     ng-if="trainingForm.$submitted || form.duration.$touched">
    <div ng-message="number">
        <span translate="MESSAGE_NUMBER"></span>
    </div>
    <div ng-message="min">
        <span translate="MESSAGE_MIN"></span>
    </div>
    <div ng-message="max">
        <span translate="MESSAGE_MAX"></span>
    </div>
</div>
</div>
dohomi commented 9 years ago

I just found a good temporary solution in here: https://github.com/angular/angular.js/issues/1404 Now it works as expected.

piyushchauhan2011 commented 8 years ago

Is it possible that nesting ng-form inside ng-form can override the properties of mainForm control ? Not sure, but had the problem of $validity being updated of ng-form with sub ng-form inside of md-autocomplete

gkalpak commented 8 years ago

I'm going to close this, since we are not likely to change the current behavior and it is already possible to have something very similar to the requested.

In addition to the above mentioned solutions, you could even have a array-ish object by using a wrapper ngForm like this:

<form name="categoryForm">
  ...
  <ng-form name="tags"><!-- Wrapper `ngForm` to collect all tags -->
    <div ng-repeat="item in items">
      <ng-form name="{{ $index }}"><!-- Use `$index` as the key, so tags can be accessed by index -->
        Tag name: <input name="name" ng-model="item.name" required />
        <div ng-if="categoryForm.tags[$index].name.$invalid">This is great from the inside !</div>
      </ng-form>
    </div>
  </ng-form>
</form>

Demo fiddle

Caveats:

@piyushchauhan2011, yours sounds like a general support question. Please, use one of the appropriate support channels for these types of questions. GitHub issues are reserved for bug reports and feature requests.