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

Conversation

nicoabie
Copy link
Contributor

@nicoabie nicoabie commented Jul 3, 2014

Added new state to form:

  • it sets to true when form is submitted
  • it sets back to false when $setPristine is called on the form

Added new state to form:
- it sets to true when form is submitted
- it sets back to false when $setPristine is called on the form
@mary-poppins
Copy link

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 (#8056)

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!

@nicoabie nicoabie mentioned this pull request Jul 3, 2014
@nicoabie nicoabie added cla: yes and removed cla: no labels Jul 3, 2014
@Narretz Narretz added this to the 1.3.0 milestone Jul 10, 2014
@rodyhaddad
Copy link
Contributor

Some jshint errors, but otherwise LGTM

Related #5574

cc @matsko

@matsko
Copy link
Contributor

matsko commented Jul 11, 2014

Please make those two fixes and fix the jshint error that shows up when you run grunt test. Otherwise looks good.

@matsko
Copy link
Contributor

matsko commented Jul 14, 2014

@nicoabie any updates?

@nicoabie
Copy link
Contributor Author

@matsko will push it tonight, sorry for the delay.

@matsko
Copy link
Contributor

matsko commented Jul 14, 2014

No rush. Thank you for the quick reply. :)

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.

use $animate.setClass in form.$setPristine to just use one animation
use $animate.addClass in form.$setSubmitted

Closes angular#8056
@rodyhaddad
Copy link
Contributor

Updates? jshint is failing, though not the same error

@nicoabie
Copy link
Contributor Author

@rodyhaddad I pushed what @matsko suggested and fixed the jshint error. Locally it is all green. Don't get it why it fails in here. I followed this guideline. https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md

@PatrickJS
Copy link
Member

+1

@rodyhaddad rodyhaddad closed this in 108a69b Aug 1, 2014
rodyhaddad pushed a commit to rodyhaddad/angular.js that referenced this pull request Aug 22, 2014
The $submitted state changes
- to true when the form is submitted
- to false when $setPristine is called on the form

A .ng-submitted class is added to the form when $submitted=true

Closes angular#8056
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();
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants