support nested forms isolated from parent #10193

gonzaloruizdevilla · 2014-11-23T00:19:01Z

No description provided.

googlebot · 2014-11-23T00:19:02Z

CLAs look good, thanks!

thorn0 · 2014-11-23T00:52:11Z

ng-model-options is a different pattern. It's a separate directive, and this attribute mustn't be on the same element as ng-model. ng-model-options works for all the elements that are descendants of the element it's specified for. Having ng-something-options with a different behavior is inconsistent.

gonzaloruizdevilla · 2014-11-23T01:26:07Z

It is true that is inconsistent in the sense that no descendant inherits the behavior. In the use cases I've found, I only had on level of child forms, and I needed some of them to be isolated (I did it with a custom directive). Should we choose a different attribute name because of this inconsistency or complement the behavior to affect the descendant forms?

I think it is not important whether ng-form-options is a directive or not, provided that the behavior is what the user expects. Anyway, I think the implementation as a directive will be the way to go if we need to affect the descendants too.

It's a separate directive, and this attribute mustn't be on the same element as ng-model.

I'm not sure how to interpret these words. This how it's done most of the times, and even most of the AngularJS tests of ng-model-options combine it with ng-model

thorn0 · 2014-11-23T08:46:20Z

Sorry, I just meant "doesn't have to", not "mustn't". In my opinion, it's better to choose a different name for the attribute.

lgalfaso · 2014-11-23T10:00:26Z

src/ng/directive/form.js

+  form.$options = $scope.$eval(attrs.ngFormOptions) || {};
+
+  var parentForm = form.$$parentForm =
+        (!form.$options.isolated && element.parent().controller('form'))


I think that isolated is a bad name, it already has a meaning regarding scopes and would like not to cause any confusion

lgalfaso · 2014-11-23T10:03:49Z

Hi, thanks for the PR! I can see that this is solving a problem, would it be possible to have the following changes

Can you add more tests with higher nested forms
Please update the documentation reflecting this feature

gonzaloruizdevilla · 2014-12-01T15:49:54Z

@lgalfaso I've added a few more tests to illustrate the complete behavior.
Instead of isolated I'm using now root for the variable name. Does it sound better?

petebacondarwin · 2014-12-11T20:05:27Z

This looks interesting but the root is still not a clear name for me....

gonzaloruizdevilla · 2014-12-11T22:08:43Z

Maybe one of these?

isolatedForm
rootForm
topForm

Maybe with a 'Form' suffix is more clear, like in rootScope.

awerlang · 2015-03-13T18:56:03Z

Missing a test for submit event's, which would fail because HTML nested structure would eventually fire a submit event on topmost form (structurally speaking), even from a control inside isolated form.

petebacondarwin · 2015-03-13T21:02:08Z

RE naming: root and isolate already have other meanings in angular, and this case doesn't not map accurately to those meanings. I would be happy with topForm or better topLevelForm.

petebacondarwin · 2015-03-13T21:07:38Z

Also drop the ng-form-options since ng-model-options is actually a directive that can appear anywhere and applies to all descendents and this is a single attribute for a single form.

So something like:

<ng-form ng-form-top-level="true">

gonzaloruizdevilla · 2015-03-16T11:04:14Z

@awerlang good catch!
@petebacondarwin I will try to change the name and add the event test next weekend.

petebacondarwin · 2015-09-09T13:38:19Z

Let's see if we can get this in 1.5

gkalpak · 2015-09-09T13:50:00Z

This probably needs a rebase since f8a07dd, 290b504 and c6110e8.

petebacondarwin · 2015-09-09T15:10:35Z

Fancy moving this one forward @gkalpak ?

gkalpak · 2015-09-09T19:02:11Z

@petebacondarwin, sure thing !
_{Moving stuff forward is the new fancy :)}

petebacondarwin · 2016-06-06T10:56:06Z

@gonzaloruizdevilla I have landed your first two commits to close off their related issues.
The last commit is not passing on Travis. Can you take a look?

gonzaloruizdevilla · 2016-06-06T15:25:40Z

sure, I'll try to find some time tonight

Child forms propagate always their state to its parent form. A new optional attribute ngFormTopLevel is defined for forms that will allow to define now if the form should be considered as 'top level', therefore preventing the propagation of its state to its parent. I It maybe used like this: <ng:form name="parent"> <ng:form name="child" ng-form-top-level="true"> <input ng:model="modelA" name="inputA"> <input ng:model="modelB" name="inputB"> </ng:form> </ng:form> Closes: angular#5858

Narretz · 2016-06-07T14:06:46Z

src/ng/directive/form.js

@@ -497,18 +507,27 @@ var formDirectiveFactory = function(isNgForm) {
                event.preventDefault();
              };

+              var handleKeypress = function(event) {


That's too simple, see https://docs.angularjs.org/api/ng/directive/form#submitting-a-form-and-preventing-the-default-action
Maybe we should instead listen on the submit event and see if the first parent form of explicitOriginalTarget is 1) not the current submit target and 2) has the do-not-propagate attribute set

(a) Actually, the whole submit handling seems problematic. In ngForm, we don't emulate the submit behavior of regular forms. So in order to be able to "submit" a detached form, we would need to emulate the submit behavior (as requested in #2513). The current patch doesn't achieve this, and without it, I am not sure how useful the detached form is.

(b) What's more if you have form (with one input) -> ngForm (with x other inputs), and the ngForm is detached, you could expect that pressing "Enter" on the regular form input submits the form (see above rules), but obviously the browser doesn't know that we have "detached" the ngForm. So either we emulate the behavior for this case (horrible idea), or we advise that if you want to have nested forms with independent submission, you should only use ngForm.

(c) If you don't use form at all however, then you are missing out on other HTML features, such as autocomplete=off.

(d) Mixing ngForm with form gets worse when you have ngForm -> form -> ngForm. Then the event handling gets super complex, because the parent ngForm has no default submission behavior, but the grandchild form does because of its form parent.

(e) There's also the issue with handling the events - if we detect "submission" by listening on Enter, for detached forms, we then have to stop the event propagation to the next form that might also have a submit handler. This doesn't sit well with me, as there might be other code that does stuff with the event. We could try to create a synthetic submit event (that is propagation stopped etc) for this case.

If we have a parent form, then ngForm could theoretically try to listen on the submit even instead, but we'd need to register ours before the one in the form which is currently not possible because that's registered in preLink fn.

Overall, we would need:

basic requirement: a way to detach a form from its parent form. (Easiest way: simply call $removeControl on the parent), that stops validation, dirtiness, explicit submitted setting etc. from propagating

a way for ngForm to behave the same way as a form wrt to submission. See https://github.com/MarkPieszak/Angular1-ngForm-ngSubmit-fixes for an incomplete approach. Without this, it's impossible to have child forms that can be submitted individually with ng-submit on ngForm and a submit button, because ngForm does not have a raise a submit event (they could however be submitted with click handlers)

I think that theoretically both of these can be implemnted in user-land. Based on the original thread, it also seems that individual submission is not very important. #5858

form submission tests plnkr:
http://plnkr.co/edit/JARrreaOAg3LWvlVEQ2R?p=preview

Narretz · 2016-06-07T14:09:47Z

I would like to get this in, but I'm not a fan of the ngFormTopLevel attribute. It really doesn't convey what it does very well. I'd prefer something like ng-form-no-parent, or ng-form-no-child, ng-form-detached.
Or even more explcit, similar to #6095: ng-form-has-parent or ng-form-attach-to-parent.

Narretz · 2018-06-12T22:09:43Z

This is not gonna happen in core due to the reasons outlined in #10193 (comment)

googlebot added the cla: yes label Nov 23, 2014

lgalfaso reviewed Nov 23, 2014
View reviewed changes

lgalfaso added this to the Backlog milestone Nov 23, 2014

lgalfaso added 1.4-candidate component: forms frequency: moderate severity: inconvenient type: feature labels Nov 23, 2014

gonzaloruizdevilla force-pushed the formOptions branch 3 times, most recently from 75c719a to cca3f39 Compare December 1, 2014 15:49

petebacondarwin modified the milestones: Backlog, 1.4.x Dec 15, 2014

gonzaloruizdevilla force-pushed the formOptions branch 2 times, most recently from 582c576 to ede8135 Compare April 16, 2015 07:26

gonzaloruizdevilla mentioned this pull request Apr 22, 2015

browserTrigger doesn't support keydown with a specific keycode #8688

Closed

petebacondarwin mentioned this pull request Sep 9, 2015

Nested Forms validation #5858

Closed

petebacondarwin modified the milestones: 1.5.x - migration-facilitation, 1.4.x Sep 9, 2015

petebacondarwin assigned gkalpak Sep 9, 2015

gonzaloruizdevilla force-pushed the formOptions branch from b588a32 to 35d08c8 Compare September 13, 2015 21:40

petebacondarwin removed the 1.4-candidate label Sep 22, 2015

Narretz force-pushed the master branch from 70049ae to b77e14b Compare January 16, 2016 21:59

gonzaloruizdevilla force-pushed the formOptions branch from 35d08c8 to 52535c3 Compare June 7, 2016 10:25

Narretz reviewed Jun 7, 2016
View reviewed changes

Narretz mentioned this pull request Jun 7, 2016

feat(form): add parent parameter #6095

Closed

Narretz mentioned this pull request Nov 7, 2017

Fix: set $submitted on child forms when form is submitted #15778

Merged

3 tasks

Narretz modified the milestones: 1.5.x, 1.7.x Apr 12, 2018

Narretz added the resolution: not core label May 3, 2018

petebacondarwin self-assigned this May 14, 2018

gkalpak removed their assignment May 23, 2018

petebacondarwin removed their assignment May 23, 2018

Narretz modified the milestones: 1.7.x, 1.7.x - won't fix May 23, 2018

Narretz closed this Jun 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support nested forms isolated from parent #10193

support nested forms isolated from parent #10193

gonzaloruizdevilla commented Nov 23, 2014

googlebot commented Nov 23, 2014

thorn0 commented Nov 23, 2014

gonzaloruizdevilla commented Nov 23, 2014

thorn0 commented Nov 23, 2014

lgalfaso Nov 23, 2014

lgalfaso commented Nov 23, 2014

gonzaloruizdevilla commented Dec 1, 2014

petebacondarwin commented Dec 11, 2014

gonzaloruizdevilla commented Dec 11, 2014

awerlang commented Mar 13, 2015

petebacondarwin commented Mar 13, 2015

petebacondarwin commented Mar 13, 2015

gonzaloruizdevilla commented Mar 16, 2015

petebacondarwin commented Sep 9, 2015

gkalpak commented Sep 9, 2015

petebacondarwin commented Sep 9, 2015

gkalpak commented Sep 9, 2015

petebacondarwin commented Jun 6, 2016

gonzaloruizdevilla commented Jun 6, 2016

Narretz Jun 7, 2016

Narretz Jun 7, 2016 •

edited

Loading

Narretz commented Jun 7, 2016 •

edited

Loading

Narretz commented Jun 12, 2018

support nested forms isolated from parent #10193

support nested forms isolated from parent #10193

Conversation

gonzaloruizdevilla commented Nov 23, 2014

googlebot commented Nov 23, 2014

thorn0 commented Nov 23, 2014

gonzaloruizdevilla commented Nov 23, 2014

thorn0 commented Nov 23, 2014

lgalfaso Nov 23, 2014

Choose a reason for hiding this comment

lgalfaso commented Nov 23, 2014

gonzaloruizdevilla commented Dec 1, 2014

petebacondarwin commented Dec 11, 2014

gonzaloruizdevilla commented Dec 11, 2014

awerlang commented Mar 13, 2015

petebacondarwin commented Mar 13, 2015

petebacondarwin commented Mar 13, 2015

gonzaloruizdevilla commented Mar 16, 2015

petebacondarwin commented Sep 9, 2015

gkalpak commented Sep 9, 2015

petebacondarwin commented Sep 9, 2015

gkalpak commented Sep 9, 2015

petebacondarwin commented Jun 6, 2016

gonzaloruizdevilla commented Jun 6, 2016

Narretz Jun 7, 2016

Choose a reason for hiding this comment

Narretz Jun 7, 2016 • edited Loading

Choose a reason for hiding this comment

Narretz commented Jun 7, 2016 • edited Loading

Narretz commented Jun 12, 2018

Narretz Jun 7, 2016 •

edited

Loading

Narretz commented Jun 7, 2016 •

edited

Loading