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

feat(form): $submitted state #8056

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 27 additions & 6 deletions src/ng/directive/form.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
'use strict';

/* global -nullFormCtrl */
/* global
-nullFormCtrl,
-SUBMITTED_CLASS
*/
var nullFormCtrl = {
$addControl: noop,
$removeControl: noop,
$setValidity: noop,
$setDirty: noop,
$setPristine: noop
};
$setPristine: noop,
$setSubmitted: noop
},
SUBMITTED_CLASS = 'ng-submitted';

/**
* @ngdoc type
Expand Down Expand Up @@ -60,6 +65,7 @@ function FormController(element, attrs, $scope, $animate) {
form.$pristine = true;
form.$valid = true;
form.$invalid = false;
form.$submitted = false;

parentForm.$addControl(form);

Expand Down Expand Up @@ -228,16 +234,29 @@ function FormController(element, attrs, $scope, $animate) {
* saving or resetting it.
*/
form.$setPristine = function () {
$animate.removeClass(element, DIRTY_CLASS);
$animate.addClass(element, PRISTINE_CLASS);
$animate.setClass(element, PRISTINE_CLASS, DIRTY_CLASS + ' ' + SUBMITTED_CLASS);
form.$dirty = false;
form.$pristine = true;
form.$submitted = false;
forEach(controls, function(control) {
control.$setPristine();
});
};
}

/**
* @ngdoc function
* @name ng.directive:form.FormController#$setSubmitted
* @methodOf ng.directive:form.FormController
*
* @description
* Sets the form to its submitted state.
*/
form.$setSubmitted = function () {
$animate.addClass(element, SUBMITTED_CLASS);
form.$submitted = true;
parentForm.$setSubmitted();

Choose a reason for hiding this comment

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

RFI: What's the rationale behind it? Why submitting a nested form needs marking all its parent as submitted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, I kept the logic of how the other states propagates upwards.
Do you have an use case that goes against this?

Choose a reason for hiding this comment

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

Yep, I have a multi-step form (aka wizard). I use .ng-submitted to style invalid inputs and have a plugin that detects if forms is dirty.

On pre-exit a state:

nestedForm.$setSubmitted();
if (!nestedForm.$valid) return;

On entering a sub-state / nested form:

this.target.mainForm.$setPristine();

Style:

.ng-submitted [ng-model].ng-invalid {
    border: 1px solid red;
}

The border keeps being draw because there's a form styled with .ng-submitted further up. And I actually use $setPristine() because it clears $submitted, but then it breaks form dirty-checking plugin.

Some strategies that could it (if they existed):

  • form.$setSubmitted(state: boolean);
  • form.$setSubmitted(updateParents: boolean);

I'm not sure yet about the invariants that could break with that. WDYT?

Choose a reason for hiding this comment

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

A workaround while form.$setSubmitted(state: boolean) doesn't exist:

const wasDirty = mainForm.$dirty;
mainForm.$setPristine();
if (wasDirty) {
    mainForm.$setDirty();
}

};
}

/**
* @ngdoc directive
Expand Down Expand Up @@ -287,6 +306,7 @@ function FormController(element, attrs, $scope, $animate) {
* - `ng-invalid` is set if the form is invalid.
* - `ng-pristine` is set if the form is pristine.
* - `ng-dirty` is set if the form is dirty.
* - `ng-submitted` is set if the form was submitted.
*
* Keep in mind that ngAnimate can detect each of these classes when added and removed.
*
Expand Down Expand Up @@ -422,6 +442,7 @@ var formDirectiveFactory = function(isNgForm) {
var handleFormSubmission = function(event) {
scope.$apply(function() {
controller.$commitViewValue();
controller.$setSubmitted();
});

event.preventDefault
Expand Down
38 changes: 34 additions & 4 deletions test/ng/directive/formSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,9 @@ describe('form', function() {

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

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


Expand Down Expand Up @@ -681,14 +684,42 @@ describe('form', function() {
expect(nestedInputCtrl.$dirty).toBe(false);
});
});

describe('$setSubmitted', function() {
beforeEach(function() {
doc = $compile(
'<form name="form" ng-submit="submitted = true">' +
'<input type="text" ng-model="name" required />' +
'<input type="submit" />' +
'</form>')(scope);

scope.$digest();
});

it('should not init in submitted state', function() {
expect(scope.form.$submitted).toBe(false);
});

it('should be in submitted state when submitted', function() {
browserTrigger(doc, 'submit');
expect(scope.form.$submitted).toBe(true);
});

it('should revert submitted back to false when $setPristine is called on the form', function() {

Choose a reason for hiding this comment

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

what about $animate.removeClass(element, SUBMITTED_CLASS)? we should also have a test for this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lucassus care to explain a little bit what you meant? Sorry I didn't get it.

scope.form.$submitted = true;
scope.form.$setPristine();
expect(scope.form.$submitted).toBe(false);
});
});
});

describe('form animations', function() {
beforeEach(module('ngAnimateMock'));

function assertValidAnimation(animation, event, className) {
function assertValidAnimation(animation, event, classNameAdded, classNameRemoved) {
expect(animation.event).toBe(event);
expect(animation.args[1]).toBe(className);
expect(animation.args[1]).toBe(classNameAdded);
expect(animation.args[2]).toBe(classNameRemoved);
}

var doc, scope, form;
Expand Down Expand Up @@ -741,8 +772,7 @@ describe('form animations', function() {

form.$setPristine();

assertValidAnimation($animate.queue[0], 'removeClass', 'ng-dirty');
assertValidAnimation($animate.queue[1], 'addClass', 'ng-pristine');
assertValidAnimation($animate.queue[0], 'setClass', 'ng-pristine', 'ng-dirty ng-submitted');
}));

it('should trigger custom errors as addClass/removeClass when invalid/valid', inject(function($animate) {
Expand Down