-
Notifications
You must be signed in to change notification settings - Fork 26.3k
V2 tutorial #62014
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
V2 tutorial #62014
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey I didn't know where to properly provide some feedback/questions, so I decided to send it as a PR review.
Thanks for all the effort with the signal forms, I really like it so far ❤️
|
||
We'll leave adding `canDeactivate` as an exercise to the reader. | ||
|
||
> There is no pristine state; let us know if it might be useful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a pristine state will definitely be useful (assuming the same as current touched
is what is being referred to here), because usually you want to show form validation errors:
- if the user tried to submit a form (form.submitted)
- when the user touches a control (control.touched)
- if the user changes a value (control.dirty)
on the matter of submitted
state, will there be any changes to NgForm directive, a new directive, or a submitted property on the signal forms api itself?
Also, not sure resetting a form group is available yet, but it would be good to still be able to reset to an arbitrary value group.reset(value)
}); | ||
}, | ||
// This allows us to get the data outside the schema. | ||
{asKey: VERSIONS_KEY}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this the way to define metadata for specific controls? This will be useful for declaratively defining behavior for a form using metadata, for example defining a component class that should be used for rendering a control. I'm really excited for that possibility 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, interesting case, thanks
yes, currently you can set metadata for controls
disabled: | ||
|
||
```typescript | ||
disabled(path.feedback, ({valueOf}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- On a disabled state, is the value of a control impacted?
Current behavior: on a group's value, if a control is disabled, the control's value will be undefined. On a control's value, the value is returned even if control is disabled. - Will undefined be the new default value when instantiating a control and not providing an initial value?
Current behavior: form builders (both Nullable and NonNullable) initialize a control withnull
(yes, even non nullable - is this a bug?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just realized, does disabled support a list of controls, like validatorTree? it would be nice if it did, as it would be simpler to define a single place (one logic) for multiple controls to be enabled/disabled.
It would solve a very common scenario that is to enable/disable one or more controls based on other one or more controls values, or some other information that is not part of the form (and possibly even async).
so bringing the disabled, readonly apis closer to what validatorTree does would be a dream come true.
|
||
## validateTree | ||
|
||
`validateTree` allows running validation on a group field and attaching the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is amazing! I really appreciate how the new API is flexible.
): TreeValidator<{ password: string; confirmationPassword: string }> { | ||
return ({valueOf, fieldOf}) => { | ||
return valueOf(path.confirmationPassword) === valueOf(path.password) | ||
? [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how we are not returning null
anymore (hoping for this).
} | ||
``` | ||
|
||
> There is currently no `readonly reason`; let us know if there's a use case for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be useful, people might benefit from generating a custom message in case the control is readonly, and it appears that since reason is a signal, you could dinamically generate the message, even based on other fields values. (same use case as disabled)
@@ -115,7 +115,7 @@ A simple form just takes a signal with the data and produces matching field stru | |||
// feedback.ts | |||
import { | |||
form, | |||
} from 'google3/experimental/angularsignalforms'; | |||
} from '@angular/forms/experimental'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, I'm excited to test signal forms API. Will it be released as experimental on 20.1? How can I try it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have specific release in mind that we want to commit to yet, there are still a lof of unknowns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the way to test it for now would be to compile Angular from this branch and reference the local angular in a local project? I noticed previously it was using google3 to import and now it uses @angular
, which makes me believe this would be possible to use locally outside of google now.
No description provided.