This repository has been archived by the owner. It is now read-only.

upgrading forms guide to new api #1696

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
8 participants
@thelgevold
Contributor

thelgevold commented Jun 19, 2016

This is work in progress. DO NOT MERGE.

The sample code has been converted for JS and TS, but the prose has not been started yet.
One open question: Have the export names been changed yet? Currently it doesn't seem to work if I try to reference ngModel from the template variable like #name="ngModel":

<input type="text" class="form-control" required
           [(ngModel)]="model.name"
           name="name"  #name="ngModel" >

The old forms api samples have been moved to a deprecated folder.

@googlebot googlebot added the CLA: yes label Jun 19, 2016

@thelgevold

This comment has been minimized.

Contributor

thelgevold commented Jun 19, 2016

Have to also make deprecated versions of the jade files for dart and js

@filipesilva

This comment has been minimized.

Member

filipesilva commented Jun 20, 2016

@thelgevold I asked @StephenFluin and he tells me that the change is in angular/angular master and should be out in next RC.

[NgControlName](../api/common/NgControlName-directive.html) directive.
There is also a `NgControl` *abstract* directive which is *not the same thing*.
We often ignore this technical distinction and refer to `NgControlName` more conveniently (albeit incorrectly) as the *NgControl* directive.

This comment has been minimized.

@thelgevold

thelgevold Jun 21, 2016

Contributor

I deleted this section since ngControl is removed

two-way binding syntax is now a property of the `NgControlName` directive.
The `NgModel` directive is no longer involved. We only need one directive to manage the DOM element
and there is no practical difference in the way either directive handles data binding.

This comment has been minimized.

@thelgevold

thelgevold Jun 21, 2016

Contributor

It's a bit unclear if we should replace this with alternative details. It's my understanding that the features of ngControl has been added to ngModel, so I assume this section is no long relevant, but we might want to confirm.

@thelgevold

This comment has been minimized.

Contributor

thelgevold commented Jun 21, 2016

We have to take a look at the exportAs section.

Why "ngForm"? A directive's exportAs property tells Angular how to link the reference variable to the directive. We set name to ngForm because the NgControlName directive's exportAs property happens to be "ngForm".......

One of the goals of the new forms API is to no longer export everything as ngForm, so we should look into how we want to handle this section. I Don't think the granular exports are included yet.

@filipesilva

This comment has been minimized.

Member

filipesilva commented Jun 21, 2016

@StephenFluin can you have a look at the comments that @thelgevold added?

@filipesilva

This comment has been minimized.

Member

filipesilva commented Jun 21, 2016

Well need a section regarding the bootstrap logic needed at the moment, like the dynamic forms cookbook has.

@StephenFluin

This comment has been minimized.

Member

StephenFluin commented Jun 21, 2016

I'm not up to speed on ngControl, so we'll have to get some input from Kara to get answers for your questions.

@filipesilva

This comment has been minimized.

Member

filipesilva commented Jun 21, 2016

@kara can you have a look?

Angular form family &mdash; including `NgForm`, `NgModel`, `NgControlName` and `NgControlGroup` &mdash; *exportAs* "ngForm"
and we only ever apply *one* of these directives to an element tag.
Consistency rules!
This mean we will be able to use the more intuitive `#name=ngModel` when accessing the underlying Angular control.

This comment has been minimized.

@thelgevold

thelgevold Jun 22, 2016

Contributor

I see that this change is already on master, but it doesn't seem to work in rc2. Explained briefly that this change is coming soon though... Removed some of original justification for the same name (consistency) since that goes against the new goal of separate names.

This comment has been minimized.

@thelgevold

thelgevold Jun 22, 2016

Contributor

@filipesilva It looks like this was included in rc3, so we have to update the template to export ngModel instead of ngForm for the input tags. We should also remove the part that says it will be here soon since it's already here :-)

@googlebot

This comment has been minimized.

googlebot commented Jun 22, 2016

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

1 similar comment
@googlebot

This comment has been minimized.

googlebot commented Jun 22, 2016

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@googlebot googlebot added CLA: no and removed CLA: yes labels Jun 22, 2016

@filipesilva

This comment has been minimized.

Member

filipesilva commented Jun 22, 2016

@googlebot me and @thelgevold are working together on this PR and he approves of me editing his commits.

@filipesilva

This comment has been minimized.

Member

filipesilva commented Jun 22, 2016

@thelgevold I updated code for RC3, added in the JS prose edits, removed unused dart forms-deprecated example code, and renamed docregions.

Please review the individual commits and squash them into yours.

@@ -283,6 +306,8 @@ figure.image-display
The diagnostic is evidence that we really are flowing values from the input box to the model and
back again. **That's two-way data binding!**
Notice that we also added a `name` attribute to our `<input>` tag. This is a requirement when using `[(ngModel)]` in combination with a form.

This comment has been minimized.

@kara

kara Jun 22, 2016

Contributor

Can you add a line to clarify why the name attribute is required? Specifically, it's to provide a name for the form control so it can be easily referred to in the aggregate form value and validity state.

This comment has been minimized.

@thelgevold

thelgevold Jun 22, 2016

Contributor

That's a good idea. We make that point later in the article, but it probably belongs here as well

This comment has been minimized.

@filipesilva
- The special CSS classes that `ngControl` adds to form controls and how we can use them to provide strong visual feedback
- The Special CSS classes follow the state of the controls and can be used to provide strong visual feedback

This comment has been minimized.

@shefalisinha

shefalisinha Jun 22, 2016

The special CSS classes that follow the state of ...

This comment has been minimized.

@filipesilva
@@ -492,7 +508,7 @@ figure.image-display
Angular added it surreptitiously, wrapping it around the `<form>` element
The `NgForm` directive supplements the `form` element with additional features.
It collects `Controls` (elements identified by an `ngControl` directive)
It holds the controls we created for the elements with `ngModel` and `name` attributes

This comment has been minimized.

@shefalisinha

shefalisinha Jun 22, 2016

...with 'ngModel' directive and 'name' attributes.

This comment has been minimized.

@filipesilva
- the special CSS classes that `ngControl` adds to form controls and how we can use them to provide strong visual feedback
- special CSS classes follow the state of the controls and can be used to provide strong visual feedback

This comment has been minimized.

@shefalisinha

shefalisinha Jun 22, 2016

special CSS classes that follow the state ....

This comment has been minimized.

@filipesilva
@@ -588,7 +596,7 @@ figure.image-display
Angular did. Angular creates and attaches an `NgForm` directive to the `<form>` tag automatically.
The `NgForm` directive supplements the `form` element with additional features.
It holds the controls we created for the elements with `ngControl` attributes
It holds the controls we created for the elements with `ngModel` and `name` attributes

This comment has been minimized.

@shefalisinha

shefalisinha Jun 22, 2016

... with 'ngModel' directive and 'name' attributes.

This comment has been minimized.

@filipesilva
docs(forms): upgrading forms guide to new api
This PR upgrades the existing forms to the new API,
while leaving a copy for existing users.

The current forms will be the default until RC4, at
which point we will switch the default to the new API
but still retain a link to the old forms API.

After RC5 the old API docs will be completely removed.
@googlebot

This comment has been minimized.

googlebot commented Jun 22, 2016

CLAs look good, thanks!

1 similar comment
@googlebot

This comment has been minimized.

googlebot commented Jun 22, 2016

CLAs look good, thanks!

@googlebot googlebot added CLA: yes and removed CLA: no labels Jun 22, 2016

@filipesilva

This comment has been minimized.

Member

filipesilva commented Jun 22, 2016

@naomiblack this PR is now fully complete and ready to be merged.

@naomiblack naomiblack changed the title from WIP(forms): upgrading forms guide to new api to upgrading forms guide to new api Jun 22, 2016

@naomiblack

This comment has been minimized.

Member

naomiblack commented Jun 22, 2016

merged and live!

@naomiblack naomiblack closed this Jun 22, 2016

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