Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

support nested forms isolated from parent #10193

Closed
wants to merge 1 commit into from
+197 −1
Closed

support nested forms isolated from parent #10193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/ng/directive/form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
var form = this,
controls = [];

var topLevel = $scope.$eval(attrs.ngFormTopLevel) || false;

// init state
form.$error = {};
form.$$success = {};
Expand All @@ -76,6 +78,7 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
form.$invalid = false;
form.$submitted = false;
form.$$parentForm = nullFormCtrl;
form.$$topLevel = topLevel;

/**
* @ngdoc method
Expand Down Expand Up @@ -318,6 +321,9 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
*
* @param {string=} ngForm|name Name of the form. If specified, the form controller will be published into
* related scope, under this name.
* @param {boolean} ngFormTopLevel Value which indicates that the form should be considered as a top level
* and that it should not propagate its state to its parent form (if there is one). By default,
* child forms propagate their state ($dirty, $pristine, $valid, ...) to its parent form.
*
*/

Expand Down Expand Up @@ -416,6 +422,10 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
angular.module('formExample', [])
.controller('FormController', ['$scope', function($scope) {
$scope.userType = 'guest';
$scope.submitted = false;
$scope.submit = function (){
$scope.submitted = true;
}
}]);
</script>
<style>
Expand Down Expand Up @@ -497,18 +507,27 @@ var formDirectiveFactory = function(isNgForm) {
event.preventDefault();
};

var handleKeypress = function(event) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@Narretz Narretz Jun 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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

if (controller.$$topLevel && event.keyCode === 13 && event.target.nodeName === "INPUT") {
event.stopPropagation();
event.preventDefault();
}
};

formElement[0].addEventListener('submit', handleFormSubmission);
formElement[0].addEventListener('keypress', handleKeypress);

// unregister the preventDefault listener so that we don't not leak memory but in a
// way that will achieve the prevention of the default action.
formElement.on('$destroy', function() {
$timeout(function() {
formElement[0].removeEventListener('submit', handleFormSubmission);
formElement[0].removeEventListener('keypress', handleKeypress);
}, 0, false);
});
}

var parentFormCtrl = ctrls[1] || controller.$$parentForm;
var parentFormCtrl = (!controller.$$topLevel && ctrls[1]) || controller.$$parentForm;
parentFormCtrl.$addControl(controller);

var setter = nameAttr ? getSetter(controller.$name) : noop;
Expand Down
177 changes: 177 additions & 0 deletions test/ng/directive/formSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1176,6 +1176,183 @@ describe('form', function() {
expect(scope.form.$submitted).toBe(false);
});
});

describe('ngFormTopLevel attribute', function() {
it('should allow define a form as top level form', function() {
doc = jqLite(
'<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>');
$compile(doc)(scope);

var parent = scope.parent,
child = scope.child,
inputA = child.inputA,
inputB = child.inputB;

inputA.$setValidity('MyError', false);
inputB.$setValidity('MyError', false);
expect(parent.$error.MyError).toBeFalsy();
expect(child.$error.MyError).toEqual([inputA, inputB]);

inputA.$setValidity('MyError', true);
expect(parent.$error.MyError).toBeFalsy();
expect(child.$error.MyError).toEqual([inputB]);

inputB.$setValidity('MyError', true);
expect(parent.$error.MyError).toBeFalsy();
expect(child.$error.MyError).toBeFalsy();

child.$setDirty();
expect(parent.$dirty).toBeFalsy();

child.$setSubmitted();
expect(parent.$submitted).toBeFalsy();
});



it('should stop enter triggered submit from propagating to parent forms', function() {
var form = $compile(
'<form name="parent">' +
'<ng-form name="topLevelForm" ng-form-top-level="true">' +
'<input type="text" name="i"/>' +
'</ng-form>' +
'</form>')(scope);
scope.$digest();

var inputElm = form.find('input').eq(0);
var topLevelFormElm = form.find('ng-form').eq(0);

var parentFormKeypress = jasmine.createSpy('parentFormKeypress');
var topLevelFormKeyPress = jasmine.createSpy('topLevelFormKeyPress');

form.on('keypress', parentFormKeypress);
topLevelFormElm.on('keypress', topLevelFormKeyPress);

browserTrigger(inputElm[0], 'keypress', {bubbles: true, keyCode:13});

expect(parentFormKeypress).not.toHaveBeenCalled();
expect(topLevelFormKeyPress).toHaveBeenCalled();

dealoc(form);
});


it('should chain nested forms as default behaviour', function() {
doc = jqLite(
'<ng:form name="parent">' +
'<ng:form name="child" >' +
'<input ng:model="modelA" name="inputA">' +
'<input ng:model="modelB" name="inputB">' +
'</ng:form>' +
'</ng:form>');
$compile(doc)(scope);

var parent = scope.parent,
child = scope.child,
inputA = child.inputA,
inputB = child.inputB;

inputA.$setValidity('MyError', false);
inputB.$setValidity('MyError', false);
expect(parent.$error.MyError).toEqual([child]);
expect(child.$error.MyError).toEqual([inputA, inputB]);

inputA.$setValidity('MyError', true);
expect(parent.$error.MyError).toEqual([child]);
expect(child.$error.MyError).toEqual([inputB]);

inputB.$setValidity('MyError', true);
expect(parent.$error.MyError).toBeFalsy();
expect(child.$error.MyError).toBeFalsy();

child.$setDirty();
expect(parent.$dirty).toBeTruthy();

child.$setSubmitted();
expect(parent.$submitted).toBeTruthy();
});

it('should chain nested forms when "ng-form-top-level" is false', function() {
doc = jqLite(
'<ng:form name="parent">' +
'<ng:form name="child" ng-form-top-level="false">' +
'<input ng:model="modelA" name="inputA">' +
'<input ng:model="modelB" name="inputB">' +
'</ng:form>' +
'</ng:form>');
$compile(doc)(scope);

var parent = scope.parent,
child = scope.child,
inputA = child.inputA,
inputB = child.inputB;

inputA.$setValidity('MyError', false);
inputB.$setValidity('MyError', false);
expect(parent.$error.MyError).toEqual([child]);
expect(child.$error.MyError).toEqual([inputA, inputB]);

inputA.$setValidity('MyError', true);
expect(parent.$error.MyError).toEqual([child]);
expect(child.$error.MyError).toEqual([inputB]);

inputB.$setValidity('MyError', true);
expect(parent.$error.MyError).toBeFalsy();
expect(child.$error.MyError).toBeFalsy();

child.$setDirty();
expect(parent.$dirty).toBeTruthy();

child.$setSubmitted();
expect(parent.$submitted).toBeTruthy();
});

it('should maintain the default behavior for children of a root form', function() {
doc = jqLite(
'<ng:form name="parent">' +
'<ng:form name="child" ng-form-top-level="true">' +
'<ng:form name="grandchild">' +
'<input ng:model="modelA" name="inputA">' +
'<input ng:model="modelB" name="inputB">' +
'</ng:form>' +
'</ng:form>' +
'</ng:form>');
$compile(doc)(scope);

var parent = scope.parent,
child = scope.child,
grandchild = scope.grandchild,
inputA = grandchild.inputA,
inputB = grandchild.inputB;

inputA.$setValidity('MyError', false);
inputB.$setValidity('MyError', false);
expect(parent.$error.MyError).toBeFalsy();
expect(child.$error.MyError).toEqual([grandchild]);
expect(grandchild.$error.MyError).toEqual([inputA, inputB]);

inputA.$setValidity('MyError', true);
expect(parent.$error.MyError).toBeFalsy();
expect(child.$error.MyError).toEqual([grandchild]);
expect(grandchild.$error.MyError).toEqual([inputB]);

inputB.$setValidity('MyError', true);
expect(parent.$error.MyError).toBeFalsy();
expect(child.$error.MyError).toBeFalsy();
expect(grandchild.$error.MyError).toBeFalsy();

child.$setDirty();
expect(parent.$dirty).toBeFalsy();

child.$setSubmitted();
expect(parent.$submitted).toBeFalsy();
});
});
});

describe('form animations', function() {
Expand Down