Skip to content

[form-field] add md-error-container directive to content projection #6625

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

Closed
willshowell opened this issue Aug 24, 2017 · 18 comments
Closed
Assignees
Labels
feature This issue represents a new feature or feature request rather than a bug or bug fix P4 A relatively minor issue that is not relevant to core functions

Comments

@willshowell
Copy link
Contributor

Bug, feature request, or proposal:

Proposal

Currently

If a user wishes to surround MdErrors with a structural directive, the errors will no longer be projected inside the mat-input-subscript-wrapper.

<!-- This will not work -->
<md-form-field>
  <input mdInput>
  <ng-container *ngFor="let error of allErrors">
    <md-error *ngIf="showError(error)">{{error.message}}</md-error>
  </ng-container>
</md-form-field>

This is (sort of) expected behavior if you know that content projection works only on top level elements. Still, if you want to conditionally render your errors with ngFor and ngIf you have to use one of the workarounds (listed under Motivation).

Proposed

<!-- This could work -->
<md-form-field>
  <input mdInput>
  <md-error-container *ngFor="let error of allErrors">
    <md-error *ngIf="showError(error)">{{error.message}}</md-error>
  </md-error-container>
</md-form-field>
<!-- form-field.html -->
...

<div *ngSwitchCase="'error'" [@transitionMessages]="_subscriptAnimationState">
  <ng-content select="md-error, md-error-container"></ng-content>
</div>

Reproduction

This is an example showing how it could work

http://plnkr.co/edit/6c6Zi6m1dNAnF10smWPK?p=preview

Motivation

Couple of issues bringing this up

#5263
#5292

Workarounds:

#5263 (comment)
#5263 (comment)

@rafaelss95
Copy link
Contributor

rafaelss95 commented Aug 24, 2017

Good idea, however it still fails if you want to abstract some complex logic in a separated component, like this:

<md-form-field>
  <input mdInput>
  <app-form-validation [control]="myCtrl"></app-form-validation>
</md-form-field>

form-validation.component.ts:

<md-error-container *ngIf="(control.errors | keys | first) as firstError">
  <md-error [translate]="'errors.messages.' + firstError"
            [translateParams]="getFormattedError(firstError)"></md-error>
</md-error-container>

@willshowell
Copy link
Contributor Author

willshowell commented Aug 24, 2017

Good point. I suppose it could also work as an attribute?

<md-form-field>
  <input mdInput>
  <app-form-validation md-error-container [control]="myCtrl"></app-form-validation>
</md-form-field>
<md-form-field>
  <input mdInput>
  <ng-container md-error-container *ngFor="let error of allErrors">
    <md-error *ngIf="showError(error)">{{error.message}}</md-error>
  </ng-container>
</md-form-field>

@julianobrasil
Copy link
Contributor

julianobrasil commented Aug 27, 2017

@willshowell, I do not remember where I found it, but you can use, as an acceptable workaround, the ngProjectAs (sorry if maybe I'm not getting right the big picture here):

<md-form-field>
  <input mdInput>
  <ng-container *ngFor="let error of allErrors" ngProjectAs="md-error">
    <md-error *ngIf="showError(error)">{{error.message}}</md-error>
  </ng-container>
</md-form-field>

I will look for the reference where I got this.

Edited: just found it => https://medium.com/claritydesignsystem/ng-content-the-hidden-docs-96a29d70d11b

Plunk: http://plnkr.co/edit/pmJbQMFMtNqFEbLRZAdP?p=preview

Anyway, it doesn't look like this so not-documented ngProjectAs is something that should be expected to be used by the developers in day-by-day coding as it demands some knowledge on how the target component works on the inside (in this case, the md-form-field selector).

@willshowell
Copy link
Contributor Author

Whoa thank you @julianobrasil! I've skimmed that article a half dozen times but somehow missed it each time. Personally, I think utilizing ngProjectAs is better than adding more fluff to the api, though I do agree that it requires the consumer to know ng-content is being used internally.

@rafaelss95 what do you think? That should also solve your logic encapsulation use case, right?

@rafaelss95
Copy link
Contributor

rafaelss95 commented Aug 29, 2017

@julianobrasil @willshowell It's an awesome solution for simple cases, but for the case that I described (unless I'm missing something here) it still fails.

Reproduction

@julianobrasil
Copy link
Contributor

@rafaelss95, I think you are right. The using of ngProjectAs won't work because <md-error> would be two levels bellow <md-form-field>. Unless someone finds out that there is another "undocumented" way to nest ngProjectAs or something like that, @willshowell's idea (the directive) seems to be the way to follow IMO.

@rafaelss95
Copy link
Contributor

rafaelss95 commented Aug 29, 2017

@julianobrasil At the first, I thought the same, however the surprise (at least for me) came when I realized that it worked even wrapped by two (or more) "tags".

Note that it's doesn't matter if you use ng-container, div or whatever.. it works, but not when you wrap it by a component.

Check this plunker

@willshowell
Copy link
Contributor Author

I think there may be a bug in core. This shows all 4 errors (3 from app-form-validation):

  <md-form-field>
    <input mdInput [formControl]="myControl" required>
    <app-form-validation ngProjectAs="md-error"></app-form-validation>
    <ng-container ngProjectAs="md-error">
      <md-error>This field is required</md-error>
    </ng-container>
  </md-form-field>

http://plnkr.co/edit/2VqnymNAkTzRCOyK0dT1?p=preview

...while (as demoed), this doesn't show any:

  <md-form-field>
    <input mdInput [formControl]="myControl" required>
    <app-form-validation ngProjectAs="md-error"></app-form-validation>
  </md-form-field>

@rafaelss95
Copy link
Contributor

rafaelss95 commented Aug 29, 2017

@willshowell I agree. It really seems like a bug in core. Do you mind to fill an issue there?

@julianobrasil
Copy link
Contributor

julianobrasil commented Aug 29, 2017

Oh, these are great news! It was kind of frustrating to find out that ngProjectAs had "almost" solved the problem. Hope this is just a bug.

@willshowell
Copy link
Contributor Author

I tried to reproduce to file an issue, but have had no luck. @rafaelss95, @julianobrasil could either of you look over this to make sure I'm not missing something?

http://plnkr.co/edit/pF09Kc7DwZS7Wb1Mj6L8?p=preview

It may actually be an issue specific to MdFormField. In that case, this issue could be repurposed to make sure ngProjectAs can be properly used for error messages.

@julianobrasil
Copy link
Contributor

@willshowell, I saw no problem with your code. In fact I tried something similar with md-list, and md-select. But the problem is that these two work even without ngProjectAs: https://plnkr.co/edit/v16XIXmeH1jgID7X7HJH?p=preview.

We're running out of options (except for blaming md-form-field).

@julianobrasil
Copy link
Contributor

This mess a little bit with ngProjectAs too (the icon goes to the end of the line):

 <md-list-item *ngFor="let note of notes">
    <ng-container ngProjectAs="md-icon">
      <md-icon md-list-icon>folder</md-icon>
    </ng-container>
    <h4 md-line>{{note.name}}</h4>
    <p md-line> {{note.updated | date}} </p>
  </md-list-item>

Plunk: https://plnkr.co/edit/WcfB6eZySPXMPlTXZDG6?p=preview

@josephperrott josephperrott added the feature This issue represents a new feature or feature request rather than a bug or bug fix label Sep 27, 2017
@josephperrott josephperrott added the P4 A relatively minor issue that is not relevant to core functions label Sep 27, 2017
@willshowell
Copy link
Contributor Author

Haven't tested by running it locally, but it looks to be due to the @ContentChildren query for MatErrors

https://github.com/angular/material2/blob/7fe1b81a47b6de036ae85c183328400e45c40b15/src/lib/form-field/form-field.ts#L157

I suspect if it were to use {descendants: true}, then you could abstract the logic to a child component. However, I don't know if there are any downsides to this (cc @mmalerba).

Here's an updated StackBlitz of the issue.

@mmalerba
Copy link
Contributor

I think it should be fine to use {descendants: true}, anyone want to send a PR?

@rafaelss95
Copy link
Contributor

@willshowell do you mind to send a PR?

@willshowell
Copy link
Contributor Author

I gave it a shot, and I could be missing something, but it looks like descendants: true is for deeper levels of content projection, not for looking inside other components. This was not the easy-out that I hoped it might be.

I then figured you should be able to provide your custom error component as MatError

providers: [
  { provide: MatError, useClass: MyFancierError } 
]

but I discovered QueryChildren matches on Type instead of InjectionToken, so I think that's a dead end too..?

Since ngProjectAs handily covers my initial purpose in opening this issue, I'm going to close it.

@rafaelss95 if you ever discover a way to achieve what you need or end up filing something in core, please report back!

@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 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature This issue represents a new feature or feature request rather than a bug or bug fix P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
Development

No branches or pull requests

5 participants