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

The isolated container exposes Ember's controllers. #4

Merged
merged 1 commit into from
Nov 19, 2014

Conversation

cyril-sf
Copy link
Contributor

Those classes are automatically registered by Ember:

  • Ember.ObjectController ( controller:object )
  • Ember.ArrayController ( controller:array )
  • Ember.Controller ( controller:basic )
  • Ember.Route ( route:basic )
  • Ember.View ( view:default and view:toplevel )
  • Ember.Select ( view:select )

[Fixes rwjblue/ember-qunit#73]

@cyril-sf
Copy link
Contributor Author

I'm not really sure how to write tests for Ember.ObjectController and Ember.Controller, let me know if you want tests around that.

@rwjblue
Copy link
Member

rwjblue commented Nov 18, 2014

If we are going down this route, we also need route:basic, view:toplevel, view:default, and view:select.

@cyril-sf
Copy link
Contributor Author

I'll add them.

I'm not familiar with the use cases that would require those classes. So if you want tests for those, which I'm happy to add, I would need some guidance.

@rwjblue
Copy link
Member

rwjblue commented Nov 18, 2014

  • Add {{view 'select'}} to any unit test for the select issue.
  • Add {{#view}}Something{{/view}} in a test to confirm view:default.

I would like tests for the scenarios to prevent regressions and whatnot, but I think maybe just having knowledge that a given thing is registered by default by Ember means we should add it...

@dgeb - Thoughts?

@dgeb
Copy link
Member

dgeb commented Nov 18, 2014

I think maybe just having knowledge that a given thing is registered by default by Ember means we should add it...

@rwjblue I agree. It would be nice to have a hook in Ember itself we could call to extract (or perform) default registrations so that duplicate logic doesn't need to be maintained here.

However, just getting the registrations in here is a good first step, so thanks @cyril-sf. Please let me know if you plan to add any more tests to this PR.

@cyril-sf
Copy link
Contributor Author

view:default is defined based on DefaultView which is simply Ember._MetamorphView.

It doesn't feel great to use that directly, but I guess it's better for the user ATM.

@dgeb I'll add a few more tests

Those classes are automatically registered by Ember:

* Ember.ObjectController
* Ember.ArrayController
* Ember.Controller
* Ember.Route
* Ember.View
* Ember.Select

[Fixes rwjblue/ember-qunit#73]
@cyril-sf
Copy link
Contributor Author

I've added tests. I'm not super convinced by the views ones.

Let me know if you think of something better.

@dgeb
Copy link
Member

dgeb commented Nov 19, 2014

@cyril-sf the tests verify the presence of those items in the container, so they're fine by me. Thanks again!

dgeb added a commit that referenced this pull request Nov 19, 2014
The isolated container exposes Ember's controllers.
@dgeb dgeb merged commit b33bdce into emberjs:master Nov 19, 2014
@rwjblue
Copy link
Member

rwjblue commented Nov 19, 2014

Thanks @cyril-sf !

@cyril-sf cyril-sf deleted the cyril/component-controller branch February 9, 2017 23:04
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.

3 participants