Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: fix reactive forms language and example #22094

Closed

Conversation

kapunahelewong
Copy link
Contributor

@kapunahelewong kapunahelewong commented Feb 8, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #2209 and #21968

What is the new behavior?

change to not call ngOnChanges directly, changes button from type reset, and reworks text to reflect these changes. Also clarifies error map info.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@buildsize
Copy link

buildsize bot commented Feb 8, 2018

This change will decrease the build size from 10.83 KB to 8.04 KB, a decrease of 2.79 KB (35%)

File name Previous Size New Size Change
bundle.min.js 8.06 KB 8.04 KB -23 bytes (0%)
bundle.min.js.brotli 2.77 KB [deleted]

@kapunahelewong kapunahelewong added aio: preview target: patch This PR is targeted for the next patch release labels Feb 21, 2018
@mary-poppins
Copy link

You can preview 2b04361 at https://pr22094-2b04361.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 285ec66 at https://pr22094-285ec66.ngbuilds.io/.

@mary-poppins
Copy link

You can preview d4d2a09 at https://pr22094-d4d2a09.ngbuilds.io/.

@kapunahelewong kapunahelewong changed the title [WIP]docs: fix reactive forms language and example docs: fix reactive forms language and example Feb 27, 2018
@mary-poppins
Copy link

You can preview c777faf at https://pr22094-c777faf.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 01bba32 at https://pr22094-01bba32.ngbuilds.io/.

@mary-poppins
Copy link

You can preview c16fa3d at https://pr22094-c16fa3d.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 6310262 at https://pr22094-6310262.ngbuilds.io/.

@mary-poppins
Copy link

You can preview a17ae91 at https://pr22094-a17ae91.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 61e0505 at https://pr22094-61e0505.ngbuilds.io/.

Copy link
Contributor

@kara kara left a comment

Choose a reason for hiding this comment

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

Small nits

@@ -106,15 +106,15 @@ You must wrap your test block in `async()` or `fakeAsync()` to
avoid looking for values in the form that aren't there yet.
With reactive forms, everything is available when you expect it to be.

### Which is better, reactive or template-driven?
### Choosing reactive or template-driven forms
Copy link
Contributor

Choose a reason for hiding this comment

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

Like this language better, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!


## Essential form classes
This guide uses four fundamental classes to build a reactive forms:
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: a reactive forms -> a reactive form

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch. Fixed!

`FormControl`, `FormGroup`, and `FormArray`.
It provides their common behaviors and properties, some of which are _observable_.
It provides their common behaviors and properties,
some of which are observable.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand some of which are observable. All of them are observable, no? Maybe it's supposed to be some of them are observables? I think it should be removed bc it's confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Removed that phrase.

It corresponds to an HTML form control such as an input box or selector.
[`FormControl`](api/forms/FormControl "API Reference: FormControl")
tracks the value and validity status of an individual form control.
It corresponds to an HTML form control such as an `<input>` or `<selector>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

You mean <select> here, I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha! Yes, thank you for catching that. Fixed. :)

this.rebuildForm();
}

rebuildForm() {
this.heroForm.reset({
name: this.hero.name
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't comment directly on it, but looks like there is still an ngOnChanges reference in onSubmit below (line 76)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhhh I see. Yes, changed to rebuildForm().

This was referenced Mar 15, 2018
leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018
@kapunahelewong kapunahelewong deleted the reactive-forms-fix branch June 7, 2019 14:20
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker aio: preview cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants