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

DI/Discuss: Allow providers to be injected based on the final ng-content transclusion point #5126

Open
matanlurey opened this issue Nov 4, 2015 · 9 comments

Comments

@matanlurey
Copy link
Contributor

@matanlurey matanlurey commented Nov 4, 2015

Problem

Had a nice discussion with @tbosch and @yjbanov this AM.

Assume you have the following component, say Tabs:

@Component(
  selector: 'tabs',
  bindings: const [
    const Binding(TabManager, toAlias: TabsComponent)
  ]
)
@View(
  template: '<ng-content></ng-content>'
)
class TabsComponent implements TabManager {
  getActiveTab() => ...
}

Today, the following works:

@Component(selector: 'tab')
class TabComponent {
  TabComponent(TabManager tabManager) { ... }
}
<tabs>
  <tab></tab>
  <tab></tab>
</tabs>

However, what if you want to build a bigger, composite widget and use delegating?

@Component(
  selector: 'page'
)
@View(
  directives: const [TabsComponent],
  template: r'''
    <h1>Navigation</h1>
    <tabs>
      <ng-content select="tab"></ng-content>
    </tabs>
    <main>
     <ng-content select="main-content"></ng-content>
    </main>
  '''
)
<page>
  <tab>Home</tab>
  <tab>Contact</tab>
  <tab>About</tab>
  <main-content>
    Welcome to my home page!
  </main-content>
</page>

In this case, the code will fail! <tab> is transcluded finally into <tabs>, but does not have knowledge or a way to interact using the TabManager interface. This hurts making composable/re-usable components 😢

Suggestions

  1. Have yet a third type of binding/provider, that supports transcluded content
  2. Have a way to re-publishing providers upwards (?)
  3. Have an alternative way to allow interaction in this manner

@IgorMinar @vsavkin

@matanlurey
Copy link
Contributor Author

@matanlurey matanlurey commented Nov 4, 2015

@jelbourn who possibly has run into this problem? Maybe :)

@yjbanov yjbanov assigned vsavkin and unassigned yjbanov Nov 4, 2015
@yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Nov 4, 2015

Assigning @vsavkin to provide comments. I have yet to form an opinion about it. On one hand, ng-content is there simply because not all browsers support native shadow DOM, in which case conceptually the projected DOM does not physically move into ng-content, only visually. On the other hand, this seems like a totally legit use-case. Perhaps we could implement two orthogonal features, one for "visual reprojection" and one for "DI reprojection"?

@matanlurey
Copy link
Contributor Author

@matanlurey matanlurey commented Nov 5, 2015

Angular has thrown native shadow DOM out anyway, so I don't feel strongly about keeping browser contracts 😇

@PascalPrecht
Copy link
Contributor

@PascalPrecht PascalPrecht commented Nov 6, 2015

It's not thrown away but disabled by default.

On Thu, Nov 5, 2015, 9:53 AM Matan Lurey notifications@github.com wrote:

Angular has thrown native shadow DOM out anyway, so I don't feel strongly
about keeping browser contracts [image: 😇]


Reply to this email directly or view it on GitHub
#5126 (comment).

@vsavkin
Copy link
Contributor

@vsavkin vsavkin commented Dec 15, 2015

Since I am not sure how frequent this use case is, I am reluctant to mix the DI and ng-content concerns. It can make DI a lot more complicated.

What about this way of doing it?

@Component({
  selector: 'tabs',
  providers: [
    new Provider(TabManagerProvider, {useAlias: TabsComponents})
  ],
  template: '<ng-content></ng-content>'
)
class TabsComponent implements TabManager {
  get tabManager() { return this; }
}

@Component({selector: 'tab'})
class TabComponent {
  constructor(private provider: TabManagerProvider){}

  onInit() {
    this.provider.tabManager; // do interesting stuff with it
  }
}

@Component({
  selector: 'page'
  directives: [TabsComponent],
  template: ``
    <h1>Navigation</h1>
    <tabs>
      <ng-content select="tab"></ng-content>
    </tabs>
    <main>
     <ng-content select="main-content"></ng-content>
    </main>
  `,
  providers: [new Provider(TabManagerProvider, {useAlias: Page})]
})
class Page {
  @ViewChild(Tabs) tabManager;
}

This pattern requires an extra level of indirection, but keeps both DI and ng-content simple

@matanlurey
Copy link
Contributor Author

@matanlurey matanlurey commented Dec 16, 2015

What if the <tab> component is created before <tabs>?

I think this is a decent work around/pattern, but could fail if you have the following logic:

<!-- The <tabs> component won't be created right away, but <tab> will be. -->
<tabs *ng-if="isLoading">
  <ng-content select="tab"></ng-content>
</tabs>
@vsavkin vsavkin removed their assignment Mar 9, 2016
@IgorMinar IgorMinar removed the P3: important label Oct 4, 2016
@vicb vicb added the type: feature label Oct 7, 2016
@artaommahe
Copy link

@artaommahe artaommahe commented Jan 6, 2017

Same issue while upgarding ng1 app to ng2 with menu component. Any chances to get a solution soon?

plnkr with example http://plnkr.co/edit/p7Vd4FRpHPRtyEuL6pEZ

@brachi-wernick
Copy link

@brachi-wernick brachi-wernick commented Jul 5, 2017

This cause to this issue: formControlName could not be used with component transclusion
angular can't find the ControlContainer because it comes thru transclusion. exactly like @artaommahe example above

@ngbot ngbot bot added this to the Backlog milestone Jan 23, 2018
@splincode
Copy link
Contributor

@splincode splincode commented Jul 26, 2018

@matanlurey Is the issue relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.