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

Proposal: Host Projection #7297

Closed
hansl opened this issue Feb 26, 2016 · 9 comments
Closed

Proposal: Host Projection #7297

hansl opened this issue Feb 26, 2016 · 9 comments
Assignees
Labels
feature Issue that requests a new feature hotlist: components team Related to Angular CDK or Angular Material

Comments

@hansl
Copy link
Contributor

hansl commented Feb 26, 2016

Design: https://docs.google.com/document/d/1E_C9Dz9IyVRgeWmPu2RwTohoLREup9dkPDvL31ruZr0

Just copying the API here for ease of discussion:

app.ts:

@Component({
  selector: 'my-app',
  template: `
    <input md-input class="my-class" (click)="window.alert(value)"
           [(ngModel)]="value" [placeholder]="'First Name'">
  `,
  providers: [MdInput]
})
export class MyApp {
  value: string = "Default Name";
}

component.ts:

@Component({
  selector: '[mdInput]',
  renderAs: 'md-container',  // See issue #6710
  template: `
    <div>{{placeholder}}</div>
    <ng-host></ng-host>
  `
})
export class MdInput {
  // Using an @Input prevents the binding from being transfered to the
  // <ng-host />
  @Input('class') class_: string = "";
  @Input() placeholder: string = "";
}

Resulting DOM:

<my-app>
  <md-container md-input class="my-class">
    <div>First Name</div>
    <input value="Default Value">
  </div>
</my-app>
@jelbourn jelbourn added feature Issue that requests a new feature type: RFC / discussion / question labels Feb 26, 2016
@jelbourn
Copy link
Member

cc @tbosch @mhevery

@lacolaco
Copy link
Contributor

Cannot this use case be solved by TemplateRef?

@hansl
Copy link
Contributor Author

hansl commented Feb 26, 2016

Although it would be possible to do using a template, a native core solution would be better and perform better.

@lacolaco
Copy link
Contributor

I see. But I think we need something marker like *ngIf if a directive can modify the DOM tree.

e.g.

 <input *mdInput class="my-class" (click)="window.alert(value)" [(ngModel)]="value" [placeholder]="'First Name'">

@mhevery
Copy link
Contributor

mhevery commented Jun 14, 2016

A fundemental problem of this proposal is where should binding and styling go? For example:

 <input mdInput class="my-class" (click)="window.alert(value)" 
      [placeholder]="'First Name'">

It is not cleare whether the class and placeholder should be applied to the projected host or the generated host. This is the case for classes properties, attributes, bindings, and styles. So let's take a step back.

The goal of this discussion is to allow wrapping an underlying element in a decorating component. A better way to think about it is:

<md-input class="standard-spacing">
  <input class="my-class" (click)="window.alert(value)" [placeholder]="'First Name'">
</md-input>

With the above wrap around approach it is clear what goes where and which binding should be applied. The downside is that it is wordy. Our approach should be to take the correct behavior and simplify the wordiness. I suggest merging the two and using the + symbol as a prefix.

Therefore:

 <input +md-input +class="standard-spacing" +(mouseover)="..." 
             class="my-class" (click)="window.alert(value)" 
             [placeholder]="'First Name'">

is short for:

<md-input class="standard-spacing">
  <input class="my-class" (click)="window.alert(value)" [placeholder]="'First Name'">
</md-input>

And no new concepts have to be created.

Advantagas are:

  • It is clear to the author of the template that +md-input is a wraping the <input>.
  • It is clear which bindings/classes/styles are applied to the wrapped element and which are applied to the wrapper element.
  • No new concepts only a new syntax.

There is a requirement that the md-input will probably need to set up listeners on the projected content. To acchieve that I propose that we allow decoration of the ng-content such as:

@Component({
  selector: '[mdInput]',
  template: `
    <div>{{placeholder}}</div>
    <ng-content class="additional-class" (change)="doSomething()"></ng-content>
  `
})
export class MdInput {
}

Because the <ng-content> is a static projection, all of the augmentations to the <ng-content> such as (change) and class can be computed statically without any runtime penalty.

@pkozlowski-opensource
Copy link
Member

So I had a look at the initial and latest proposals and here are some thoughts.

To start with I feel like we are trying to drive change in the framework based on just one use-case. If you ask me I can't think of a use-case other than inputs for the "host projection" future. And we all know that designing API on just one use case is doomed to fail :-) So if anyone has different use-cases that would benefit from the mechanism discussed here it would be great to discuss those use-cases together.

Then, I completely agree with @mhevery that the initial proposal doesn't make it clear which attributes / bindings / event handlers go where. If I write <md-input foo="bar" (focus)="doSth()"> it is not clear at all which element will trigger (focus) event handler and what is going to happen with the foo attribute. In this sense the evolved proposal (let's put wordiness of it aside for now):

<md-input class="standard-spacing">
  <input class="my-class" (click)="window.alert(value)" [placeholder]="'First Name'">
</md-input>

is much better since it makes it clear what belongs where. At the same time it introduces new questions:

  • where should I put ngModel now? On the <md-input> or <input> element?
  • where should I put validation constraints (required etc.)? Disabled?
  • what should happen with <ng-content class="additional-class" (change)="doSomething()"> when <ng-content> matches more than one element? Should the (change) handler be added to all the matching elements? The first one?

So the new proposal makes certain things clearer and other things more confusing now. Based on this I would do more "soul searching" before implementing anything.

The last remark is about the proposed +md-input syntax: my first gut-feeling reaction is that no of characters to type "saved" doesn't justify new syntax.

At this point I'm not sure what the final solution for the "inputs case" should be but I think that it requires more thought before we can implement things.

@pkozlowski-opensource
Copy link
Member

Based on the discussion in this issue and chat I had with Jeremy / Misko we are not going to implement host projection as described here for now. The reasoning being:

  • issue seems to be specific to one directive type and we are not sure if it would benefit other use-cases
  • implementation would have confusing semantic

Subscribing to events / binding to properties on <ng-content> sounds like a valuable addition and we might explore this in a separate issue(s).

@thelgevold
Copy link
Contributor

thelgevold commented Jun 20, 2016

@pkozlowski-opensource @mhevery @hansl
I understand the challenges, but my primary concern about the current md-input approach is that it makes it very hard to add your own attributes to the input element, unless md-input adds an API around it to ferry inn custom attributes.

One specific example that comes to mind is forms integration.

The new forms directives are not platform directives, so it seems like integration with forms would require a dependency on forms from md-input to get access toREACTIVE_FORM_DIRECTIVES . This seems undesirable in cases where you don't need forms.

Sorry if I am missing missing something here, but with the current approach, what is the suggested integration strategy with forms from md-input?

Forms seems like a very common use case for md-input.

@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
feature Issue that requests a new feature hotlist: components team Related to Angular CDK or Angular Material
Projects
None yet
Development

No branches or pull requests

7 participants