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

Remove Component.lookup and Component.lookupTemplate overrides #22

Open
cmather opened this issue Jun 15, 2014 · 4 comments
Open

Remove Component.lookup and Component.lookupTemplate overrides #22

cmather opened this issue Jun 15, 2014 · 4 comments
Assignees

Comments

@cmather
Copy link
Owner

cmather commented Jun 15, 2014

cc @avital, @estark37, @tmeasday

In order to make "yield" and "contentFor" work with blaze in the last release of blaze-layout/iron-router, we overrode Component.lookup and Component.lookupTemplate. This is now breaking with additions that have been made to these functions in Core (related to UI._templateInstance()) in release/0.8.1-rc2. See:
iron-meteor/iron-router#691 (comment).

We should remove these overrides provided we can meet or change our original use cases. Those use cases are:

  1. When {{> yield}} is called, find the parent Layout component and call the yield method defined on that component.
  2. When {{> contentFor}} is called, find the parent Layout component and call the contentFor method defined on that component.
  3. When {{yield}} is called provide a helpful error message like: "Sorry, would you mind using {{> yield}} instead of {{yield}}?"

It's not immediately clear how we can tell Spacebars to lookup a rendered hierarchy of components for a method defined further up in the chain. But I recall some designs for this in Meteor's Blaze v2 hackpad. I need to verify that something exists. Alternatively, we can add the UI._templateInstance implementation logic to our override. But I'm hoping we can remove this hack.

@cmather cmather self-assigned this Jun 15, 2014
@tmeasday
Copy link
Contributor

@cmather - I think we are still waiting on Enable dynamically scoped helpers on components, with restrictions -- but in the meantime perhaps we can do something like this:

UI.registerHelper('yield', function() {
  var template = UI._templateInstance();
  var component = template.__component__;

  // find parent Layout component
  while (component && component.kind !== 'Layout')
    component = component.parent;

  if (! component) throw "NOT IN A LAYOUT";

  return component.yield.call(this, arguments);
});

Haven't tried that, just speculating...

@cmather
Copy link
Owner Author

cmather commented Jun 15, 2014

Yeah that looks promising.

On Jun 15, 2014, at 4:55 PM, Tom Coleman [email protected] wrote:

@cmather - I think we are still waiting on Enable dynamically scoped helpers on components, with restrictions -- but in the meantime perhaps we can do something like this:

UI.registerHelper('yield', function() {
var template = UI._templateInstance();
var component = template.component;

// find parent Layout component
while (component && component.kind !== 'Layout')
component = component.parent;

if (! component) throw "NOT IN A LAYOUT";

return component.yield.call(this, arguments);
});
Haven't tried that, just speculating...


Reply to this email directly or view it on GitHub.

@davidworkman9
Copy link

I was able to remove the override of UI.Component.lookup and confirm that UI._parentData(n) and UI._templateInstance() both work now. The rest of my app seems to work fine too, I'm not quite sure I fully grok what the override is trying to do, but my app doesn't seem to need it.

@cmather
Copy link
Owner Author

cmather commented Jul 1, 2014

This is all fixed up with a new layout package:
https://github.com/eventedmind/iron-layout

IronRouter integration is on a feature branch: feature/iron-layout. After some app testing we'll do a patch release.

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

No branches or pull requests

3 participants