feat(AbstractControlDirective): implement hasError() and getError() #7255

Closed
PascalPrecht opened this Issue Feb 24, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@PascalPrecht
Contributor

PascalPrecht commented Feb 24, 2016

Due to #7238 I realised, that when building template-driven forms, we don't have APIs such as hasError() and getError() on control instances.

E.g. having a tiny form like this:

<form>
  <input ngControl="name" #name="ngForm" minlength="3">
  <p *ngIf="name.errors.minlength">Too short!</p>
</form>

We always have to walk down the properties of ngControl instance to access the error type.

However, when this same form is built model-driven, all of a sudden we have additional APIs:

<form [ngFormModel]="someFormModel">
  <input ngControl="name" #name="ngForm" minlength="3">
  <p *ngIf="name.hasError('minlength')">Too short!</p>
</form>

What makes the difference, even though we seem to work with the same instance?

Well, when building model-driven forms, the form model is built in the component and then associated to a form in the DOM using [ngFormModel]. The model consists of Control and ControlGroup and it happens that Control extends AbstractControl which implements hasError() and getError().

We don't have this with template-driven forms, as we're only using the NgControlName directive, which extends NgControl which extends AbstractControlDirective, and that one doesn't implement hasError() or getError().

I'd expect the same APIs in the template no matter if I'm building template-driven forms or model-driven forms. So I propose extending AbstractControlDirective accordingly, as it has access to its Control.

Does that make sense?

@PascalPrecht

This comment has been minimized.

Show comment
Hide comment
@PascalPrecht

PascalPrecht Feb 26, 2016

Contributor

UPDATE

Turns out, ngControl does have a handle to its actual Control instance via

class NgControlName {
...
get control(): Control { return this.formDirective.getControl(this); }
}

However, that one doesn't help either, as the control itself is not registered yet at the point we're accessing it from the template:

<input ngControl="name" #name="ngForm">
<p *ngIf="name.control.hasError('minLength')">...</p>

The control gets only registered the very first time the value of this control changes.

Contributor

PascalPrecht commented Feb 26, 2016

UPDATE

Turns out, ngControl does have a handle to its actual Control instance via

class NgControlName {
...
get control(): Control { return this.formDirective.getControl(this); }
}

However, that one doesn't help either, as the control itself is not registered yet at the point we're accessing it from the template:

<input ngControl="name" #name="ngForm">
<p *ngIf="name.control.hasError('minLength')">...</p>

The control gets only registered the very first time the value of this control changes.

@PascalPrecht

This comment has been minimized.

Show comment
Hide comment
@PascalPrecht

PascalPrecht Feb 26, 2016

Contributor

UPDATE

However, that one doesn't help either, as the control itself is not registered yet at the point we're accessing it from the template. The control gets only registered the very first time the value of this control changes.

Realised I can use the elvis operator to make this work (as this is what we use for expressions that evaluated asynchronously):

<input ngControl="name" #name="ngForm">
<p *ngIf="name.control?.hasError('minLength')">...</p>

However, I still think, that both strategies, template-driven and model-driven, should provide the same API in the template.

Contributor

PascalPrecht commented Feb 26, 2016

UPDATE

However, that one doesn't help either, as the control itself is not registered yet at the point we're accessing it from the template. The control gets only registered the very first time the value of this control changes.

Realised I can use the elvis operator to make this work (as this is what we use for expressions that evaluated asynchronously):

<input ngControl="name" #name="ngForm">
<p *ngIf="name.control?.hasError('minLength')">...</p>

However, I still think, that both strategies, template-driven and model-driven, should provide the same API in the template.

@PascalPrecht

This comment has been minimized.

Show comment
Hide comment
@PascalPrecht

PascalPrecht Feb 26, 2016

Contributor

Further more, it'd be nice to have an API that tells if there are errors at all, so we can do sth. like:

<div *ngIf="controlInstance.hasErrors()">
  <!-- only now check if specific error messages are needed -->
</div>

This could also be done by extending hasError() to work without any argument (currently it requires a string for the error type).

hasError(errorCode: string = null, ...) {
  if (!errorCode) { return isEmpty(this.errors); }
  return isPresent(this.getError(errorCode, path));
}
Contributor

PascalPrecht commented Feb 26, 2016

Further more, it'd be nice to have an API that tells if there are errors at all, so we can do sth. like:

<div *ngIf="controlInstance.hasErrors()">
  <!-- only now check if specific error messages are needed -->
</div>

This could also be done by extending hasError() to work without any argument (currently it requires a string for the error type).

hasError(errorCode: string = null, ...) {
  if (!errorCode) { return isEmpty(this.errors); }
  return isPresent(this.getError(errorCode, path));
}
@wardbell

This comment has been minimized.

Show comment
Hide comment
@wardbell

wardbell Mar 11, 2016

Contributor

it'd be nice to have an API that tells if there are errors at all

I'm confused. We have valid. Isn't that the same thing?

From the plunker in the Forms chapter

<input type="text" class="form-control" required
   [(ngModel)]="model.name"
   ngControl="name"  #name="ngForm" >
<div [hidden]="name.valid || name.pristine" class="alert alert-danger">
  Name is required
</div>

That could have been

 <div *ngIf="!name.valid && !name.pristine" class="alert alert-danger">
   Name is required
 </div>

The valid property isn't precisely the same but I rather doubt you'll find a time when you need the difference.

Contributor

wardbell commented Mar 11, 2016

it'd be nice to have an API that tells if there are errors at all

I'm confused. We have valid. Isn't that the same thing?

From the plunker in the Forms chapter

<input type="text" class="form-control" required
   [(ngModel)]="model.name"
   ngControl="name"  #name="ngForm" >
<div [hidden]="name.valid || name.pristine" class="alert alert-danger">
  Name is required
</div>

That could have been

 <div *ngIf="!name.valid && !name.pristine" class="alert alert-danger">
   Name is required
 </div>

The valid property isn't precisely the same but I rather doubt you'll find a time when you need the difference.

@wardbell

This comment has been minimized.

Show comment
Hide comment
@wardbell

wardbell Mar 11, 2016

Contributor

I do like the idea of sugaring AbstractControlDirective with getError and hasError methods.

I think it's as simple as adding:

getError(errorCode: string): any {
  return isPresent(this.control) ? this.control.getError(errorCode, this.path) : null;
}

hasError(errorCode: string): boolean {
  return isPresent(this.getError(errorCode, this.path));
}

That's consistent with its errors property

get errors(): {[key: string]: any} {
  return isPresent(this.control) ? this.control.errors : null;
}
Contributor

wardbell commented Mar 11, 2016

I do like the idea of sugaring AbstractControlDirective with getError and hasError methods.

I think it's as simple as adding:

getError(errorCode: string): any {
  return isPresent(this.control) ? this.control.getError(errorCode, this.path) : null;
}

hasError(errorCode: string): boolean {
  return isPresent(this.getError(errorCode, this.path));
}

That's consistent with its errors property

get errors(): {[key: string]: any} {
  return isPresent(this.control) ? this.control.errors : null;
}

cexbrayat added a commit to cexbrayat/angular that referenced this issue Sep 29, 2016

feat(forms): add hasError and getError to AbstractControlDirective
Allows cleaner expressions in template-driven forms.

Before:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.control.hasError('required')">Username is required</div>

After:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.hasError('required')">Username is required</div>

Fixes #7255

cexbrayat added a commit to cexbrayat/angular that referenced this issue Sep 29, 2016

feat(forms): add hasError and getError to AbstractControlDirective
Allows cleaner expressions in template-driven forms.

Before:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.control.hasError('required')">Username is required</div>

After:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.hasError('required')">Username is required</div>

Fixes #7255

cexbrayat added a commit to cexbrayat/angular that referenced this issue Sep 29, 2016

feat(forms): add hasError and getError to AbstractControlDirective
Allows cleaner expressions in template-driven forms.

Before:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.control.hasError('required')">Username is required</div>

After:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.hasError('required')">Username is required</div>

Fixes #7255

cexbrayat added a commit to cexbrayat/angular that referenced this issue Oct 10, 2016

feat(forms): add hasError and getError to AbstractControlDirective
Allows cleaner expressions in template-driven forms.

Before:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.control.hasError('required')">Username is required</div>

After:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.hasError('required')">Username is required</div>

Fixes #7255

cexbrayat added a commit to cexbrayat/angular that referenced this issue Oct 10, 2016

feat(forms): add hasError and getError to AbstractControlDirective
Allows cleaner expressions in template-driven forms.

Before:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.control.hasError('required')">Username is required</div>

After:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.hasError('required')">Username is required</div>

Fixes #7255

cexbrayat added a commit to cexbrayat/angular that referenced this issue Oct 13, 2016

feat(forms): add hasError and getError to AbstractControlDirective
Allows cleaner expressions in template-driven forms.

Before:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.control.hasError('required')">Username is required</div>

After:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.hasError('required')">Username is required</div>

Fixes #7255

@alxhub alxhub closed this in #11985 Oct 19, 2016

alxhub added a commit that referenced this issue Oct 19, 2016

feat(forms): add hasError and getError to AbstractControlDirective (#…
…11985)

Allows cleaner expressions in template-driven forms.

Before:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.control.hasError('required')">Username is required</div>

After:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.hasError('required')">Username is required</div>

Fixes #7255

btrigueiro added a commit to btrigueiro/angular that referenced this issue Oct 21, 2016

feat(forms): add hasError and getError to AbstractControlDirective (#…
…11985)

Allows cleaner expressions in template-driven forms.

Before:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.control.hasError('required')">Username is required</div>

After:

    <label>Username</label><input name="username" ngModel required #username="ngModel">
    <div *ngIf="username.dirty && username.hasError('required')">Username is required</div>

Fixes #7255
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment