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

feat(forms): easier updating of validators on existing controls #9516

Merged
merged 1 commit into from
Jun 23, 2016
Merged

feat(forms): easier updating of validators on existing controls #9516

merged 1 commit into from
Jun 23, 2016

Conversation

thelgevold
Copy link
Contributor

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

This will add support for setting and clearing validators on exiting controls. It's already possible to do this by manipulating the .validator property on the control, but I am proposing this as an enhancement of the existing API.

The use case is dynamic forms where user actions in the form might require validation rules on any given control to change dynamically. The goal is to make the validator updates easier.

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[x] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@thelgevold thelgevold changed the title feat(forms): easier updating of validators on exiting controls feat(forms): easier updating of validators on existing controls Jun 23, 2016
@vicb vicb added area: forms feature Issue that requests a new feature action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 23, 2016
@@ -100,6 +100,13 @@ export abstract class AbstractControl {

get pending(): boolean { return this._status == PENDING; }

setValidators(newValidator: ValidatorFn|ValidatorFn[]) {
let newValidators = !Array.isArray(newValidator) ? [newValidator] : <ValidatorFn[]>newValidator;
Copy link
Contributor

@kara kara Jun 23, 2016

Choose a reason for hiding this comment

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

Seems like you could use coerceToValidator here?

this.validator = coerceToValidator(newValidator);

@kara kara added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jun 23, 2016
@kara
Copy link
Contributor

kara commented Jun 23, 2016

If there is a way to set the validator, seems like there should be a way to set and clear async validators as well. Is there a reason that wasn't added?

@thelgevold
Copy link
Contributor Author

@kara That is a good point. I have added async counter parts and addressed your other comments as well,

Thanks!

@kara kara added pr_state: LGTM and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 23, 2016
@kara
Copy link
Contributor

kara commented Jun 23, 2016

Thanks for the PR! LGTM.

@vicb vicb merged commit 638fd74 into angular:master Jun 23, 2016
@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 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: forms cla: yes feature Issue that requests a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants