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

docs(form): Fixing broken example in form docs, added documentation for $setDirty(). #1709

Closed
wants to merge 1 commit into from

Conversation

jonbcard
Copy link
Contributor

No description provided.

@jonbcard
Copy link
Contributor Author

In case it seemed weird: the reason I added docs for $setDirty() but not some of the other methods on FormController was just based on what I've actually had to call in the past from applications..

@pkozlowski-opensource
Copy link
Member

Thanks for your contribution! In order for us to be able to accept it, we ask you to sign our CLA (contributor's license agreement).

CLA is important for us to be able to avoid legal troubles down the road.

For individuals (a simple click-through form):
http://code.google.com/legal/individual-cla-v1.0.html

For corporations (print, sign and scan+email, fax or mail):
http://code.google.com/legal/corporate-cla-v1.0.html

@jonbcard If you could amend the commit message as described in http://docs.angularjs.org/misc/contribute it would speed up things.

@jonbcard
Copy link
Contributor Author

Oops! Reading fail on the commit message format. I've fixed the commit message and CLA is signed. Does the empty "merge branch" commit screw up the pull request? Apologies on the hassle over a very minor PR -- this is kind of a practice run for me :).

@pkozlowski-opensource
Copy link
Member

@jonbcard I will manage this time with the commits but having clean commits saves as considerable amount of time and allow process more PRs / issues. I would recommend an excellent, freely available book on git: http://git-scm.com/book

GitHub is great but sometimes it is necessary to get hands dirty with a git command line to get the desired results. If you are motivated give it a try - ideally we would like to end up with one commit in a PR. Otherwise I will just pick the 3ec3566.

@jonbcard
Copy link
Contributor Author

I'll try to clean up my mess here :).

Form documentation fixes:
- Fix broken form example in docs
- Add method documentation for $setDirty()
- A few small other corrections in form docs.
@jonbcard
Copy link
Contributor Author

All clean and ready to merge.

@pkozlowski-opensource
Copy link
Member

👍

@pkozlowski-opensource
Copy link
Member

@jonbcard out of curiosity - what was your use-case when you had to call the $setDirty manually? Most of the time this flag would be set by calling $setViewValue (https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L1015)

Just want to make sure that we are not exposing more details than necessary.

@jonbcard
Copy link
Contributor Author

I hear ya: I mentioned that in the docs to try to convince developers they wouldn't normally need to call this.

The main case I've used it for is when there are controls on the form that are not a good fit for an ngModel-based directive. One maybe plausible example: a third-party editable grid component that's handling a big chunk of model data and not just a single string value. In these cases you can either 1) add a second gridDirty flag to your scope and combine any $dirty checks with that one, or 2) just hook right into the same $dirty flag as the rest of the form. I've opted for the latter since I still consider the component to be conceptually part of the form. It also provides some symmetry for your $setPristine docs ;). If you can buy this: I guess it might raise the question why I didn't also document $setValidity() too, come to think of it :).

All that being said, I'd be happy to yank those docs out of the PR if its still contentious. My main purpose was just to fix the broken example.

@pkozlowski-opensource
Copy link
Member

@jonbcard Finally I've removed the docs for $setDirty, changed the commit message and landed it as 3608993.

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants