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
nicoabie wants to merge 2 commits into
angular:masterfrom
nicoabie:form-submitted
Closed

feat(form): $submitted state#8056
nicoabie wants to merge 2 commits into
angular:masterfrom
nicoabie:form-submitted

Conversation

@nicoabie
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
Contributor

Some jshint errors, but otherwise LGTM

Related #5574

cc @matsko

@matsko
Copy link
Copy Markdown
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
Copy Markdown
Contributor

matsko commented Jul 14, 2014

@nicoabie any updates?

@nicoabie
Copy link
Copy Markdown
Contributor Author

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

@matsko
Copy link
Copy Markdown
Contributor

matsko commented Jul 14, 2014

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

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor

Updates? jshint is failing, though not the same error

@nicoabie
Copy link
Copy Markdown
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
Copy Markdown
Contributor

+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
Comment thread src/ng/directive/form.js
Copy link
Copy Markdown

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
Copy Markdown
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?

Copy link
Copy Markdown

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?

Copy link
Copy Markdown

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