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

Routable components: step one #11939

Merged
merged 1 commit into from
Aug 2, 2015
Merged

Routable components: step one #11939

merged 1 commit into from
Aug 2, 2015

Conversation

ebryn
Copy link
Member

@ebryn ebryn commented Jul 31, 2015

@ef4 @rwjblue @mmun

I consider this a MVP of the Routable Components RFC.

Things of note:

  • Our current strategy for avoiding naming collisions with existing components is to require routable components to be GlimmerComponents (isGlimmerComponent flag checks). Upon completion of the glimmer-component branch, the tests should be updated to use the new base class.
  • This punts on the creation of the Route#attrs hook for now. These routable components just get created with an attrs hash containing model.

@ebryn ebryn changed the title Routable components Routable components: step one Jul 31, 2015
@ebryn ebryn force-pushed the routable-components branch from 252866b to d69b08c Compare July 31, 2015 23:35
let componentName = options && options.component || namePassed && name || route.componentName || name;
let componentLookup = route.container.lookup('component-lookup:main');
Component = componentLookup.lookupFactory(componentName);
let isGlimmerComponent = Component && Component.proto().isGlimmerComponent;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to do Component.isGlimmerComponentFactory (and do Ember.GlimmerComponent.reopenClass({ isGlimmerComponentFactory: true});) over Component.proto().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I was just aligning with the existing glimmer-component branch

@rwjblue
Copy link
Member

rwjblue commented Aug 1, 2015

I fixed a few issues with Travis (and optional feature tests), can you rebase?

@ebryn ebryn force-pushed the routable-components branch from d69b08c to b45b466 Compare August 1, 2015 20:46
@ebryn
Copy link
Member Author

ebryn commented Aug 1, 2015

Rebased


* `ember-routing-routable-components`

TODO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you link to the RFC here (similar to ember-debug-handlers section).

@rwjblue
Copy link
Member

rwjblue commented Aug 2, 2015

Looks good, we can merge these incrementally. Can you squash commits and prefix with [FEATURE ember-routing-routable-components]?

@ebryn ebryn force-pushed the routable-components branch from b45b466 to 7c8d59a Compare August 2, 2015 18:45
@ebryn
Copy link
Member Author

ebryn commented Aug 2, 2015

Done and done

rwjblue added a commit that referenced this pull request Aug 2, 2015
@rwjblue rwjblue merged commit 68cef73 into master Aug 2, 2015
@rwjblue rwjblue deleted the routable-components branch August 2, 2015 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants