Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Change connectOutlet API to prefer Strings

Instead of:

  connectOutlet(App.PostView)

Do:

  connectOutlet('post')

The usage of a view before was hiding the way that
the controller was looked up. Reverse engineering
a string and getting a controller from the String
was confusing.

Now, you pass a string and we determine both the
view class and the controller to use.

You can override the details via:

  connectOutlet({
    viewClass: App.PostView,
    controller: postController,
    context: post
  })

You can specify an alternate outlet name:

  connectOutlet({
    outletName: 'detail',
    name: 'post'    
    context: post
  })

You can specify either a name or a view class,
but not both.
  • Loading branch information...
commit be69395f5eec4187b1df052d7386bcda45f79475 1 parent ee692b2
@wycats wycats authored
View
7 packages/ember-application/lib/system/application.js
@@ -224,7 +224,10 @@ Ember.Application.registerInjection({
router.set(name, controller);
- controller.set('target', router);
- controller.set('controllers', router);
+ controller.setProperties({
+ target: router,
+ controllers: router,
+ namespace: app
+ });
}
});
View
2  packages/ember-application/tests/system/application_test.js
@@ -114,6 +114,8 @@ test("initialize controllers into a state manager", function() {
equal(getPath(stateManager, 'fooController.target'), stateManager, "the state manager is assigned");
equal(getPath(stateManager, 'barController.target'), stateManager, "the state manager is assigned");
+ equal(getPath(stateManager, 'fooController.namespace'), app, "the namespace is assigned");
+ equal(getPath(stateManager, 'fooController.namespace'), app, "the namespace is assigned");
});
test('initialized application go to initial route', function() {
View
85 packages/ember-views/lib/system/controller.js
@@ -4,6 +4,7 @@ Ember.ControllerMixin.reopen({
target: null,
controllers: null,
+ namespace: null,
view: null,
/**
@@ -68,64 +69,60 @@ Ember.ControllerMixin.reopen({
controller's `content` property, if a controller can be
found.
*/
- connectOutlet: function(outletName, viewClass, context) {
+ connectOutlet: function(name, context) {
// Normalize arguments. Supported arguments:
//
- // viewClass
- // outletName, viewClass
- // viewClass, context
- // outletName, viewClass, context
+ // name
+ // name, context
+ // options
//
- // Alternatively, an options hash may be passed:
+ // The options hash has the following keys:
//
- // {
- // name: 'outletName',
- // view: App.ViewClass,
- // context: {}
- // }
-
- var controller;
-
- if (arguments.length === 2) {
- if (Ember.Object.detect(outletName)) {
- context = viewClass;
- viewClass = outletName;
- outletName = 'view';
- }
- } else if (arguments.length === 1) {
- if (Ember.typeOf(outletName) === 'object') {
- var options = outletName;
- outletName = options.name;
- viewClass = options.view;
- context = options.context;
+ // name: the name of the controller and view
+ // to use. If this is passed, the name
+ // determines the view and controller.
+ // outletName: the name of the outlet to
+ // fill in. default: 'view'
+ // viewClass: the class of the view to instantiate
+ // controller: the controller instance to pass
+ // to the view
+ // context: an object that should become the
+ // controller's `content` and thus the
+ // template's context.
+
+ var outletName, viewClass, view, controller, options;
+
+ if (arguments.length === 1) {
+ if (Ember.typeOf(name) === 'object') {
+ options = name;
+ outletName = options.outletName;
+ name = options.name;
+ viewClass = options.viewClass;
controller = options.controller;
- } else {
- viewClass = outletName;
- outletName = 'view';
+ context = options.context;
}
+ } else {
+ options = {};
}
- var parts = viewClass.toString().split("."),
- last = parts[parts.length - 1],
- match = last.match(/^(.*)View$/);
+ outletName = outletName || 'view';
- Ember.assert("The parameter you pass to connectOutlet must be a class ending with View", !!match);
+ Ember.assert("You must supply a name or a view class to connectOutlets, but not both", (!!name && !viewClass && !controller) || (!name && !!viewClass));
- var bareName = match[1], controllerName;
- bareName = bareName.charAt(0).toLowerCase() + bareName.substr(1);
+ if (name) {
+ var namespace = get(this, 'namespace'),
+ controllers = get(this, 'controllers');
- if (controller === undefined) {
- controllerName = bareName + "Controller";
- controller = get(get(this, 'controllers'), controllerName);
+ var viewClassName = name.charAt(0).toUpperCase() + name.substr(1) + "View";
+ viewClass = get(namespace, viewClassName);
+ controller = get(controllers, name + 'Controller');
- Ember.assert("You specified a context, but no " + controllerName + " was found", !context || !!controller);
- } else if (controller === null) {
- Ember.assert("You specified a context, but the controller passed to connectOutlet was null", !context || !!controller);
+ Ember.assert("The name you supplied " + name + " did not resolve to a view " + viewClassName, !!viewClass);
+ Ember.assert("The name you supplied " + name + " did not resolve to a controller " + name + 'Controller', !!controller);
}
- if (context) { controller.set('content', context); }
-
- var view = viewClass.create();
+ if (controller && context) { controller.set('content', context); }
+ view = viewClass.create();
if (controller) { set(view, 'controller', controller); }
set(this, outletName, view);
View
29 packages/ember-views/tests/system/controller_test.js
@@ -25,9 +25,10 @@ test("connectOutlet instantiates a view, controller, and connects them", functio
var postController = Ember.Controller.create();
var appController = TestApp.ApplicationController.create({
- controllers: { postController: postController }
+ controllers: { postController: postController },
+ namespace: { PostView: TestApp.PostView }
});
- var view = appController.connectOutlet(TestApp.PostView);
+ var view = appController.connectOutlet('post');
ok(view instanceof TestApp.PostView, "the view is an instance of PostView");
equal(view.get('controller'), postController, "the controller is looked up on the parent's controllers hash");
@@ -38,9 +39,10 @@ test("connectOutlet takes an optional outlet name", function() {
var postController = Ember.Controller.create();
var appController = TestApp.ApplicationController.create({
- controllers: { postController: postController }
+ controllers: { postController: postController },
+ namespace: { PostView: TestApp.PostView }
});
- var view = appController.connectOutlet('mainView', TestApp.PostView);
+ var view = appController.connectOutlet({ name: 'post', outletName: 'mainView' });
ok(view instanceof TestApp.PostView, "the view is an instance of PostView");
equal(view.get('controller'), postController, "the controller is looked up on the parent's controllers hash");
@@ -52,9 +54,10 @@ test("connectOutlet takes an optional controller context", function() {
context = {};
var appController = TestApp.ApplicationController.create({
- controllers: { postController: postController }
+ controllers: { postController: postController },
+ namespace: { PostView: TestApp.PostView }
});
- var view = appController.connectOutlet(TestApp.PostView, context);
+ var view = appController.connectOutlet('post', context);
ok(view instanceof TestApp.PostView, "the view is an instance of PostView");
equal(view.get('controller'), postController, "the controller is looked up on the parent's controllers hash");
@@ -67,9 +70,10 @@ test("connectOutlet works if all three parameters are provided", function() {
context = {};
var appController = TestApp.ApplicationController.create({
- controllers: { postController: postController }
+ controllers: { postController: postController },
+ namespace: { PostView: TestApp.PostView }
});
- var view = appController.connectOutlet('mainView', TestApp.PostView, context);
+ var view = appController.connectOutlet({ name: 'post', outletName: 'mainView', context: context });
ok(view instanceof TestApp.PostView, "the view is an instance of PostView");
equal(view.get('controller'), postController, "the controller is looked up on the parent's controllers hash");
@@ -86,8 +90,9 @@ test("connectOutlet works if a hash of options is passed", function() {
});
var view = appController.connectOutlet({
- name: 'mainView',
- view: TestApp.PostView,
+ outletName: 'mainView',
+ viewClass: TestApp.PostView,
+ controller: postController,
context: context
});
@@ -106,8 +111,8 @@ test("if the controller is explicitly set to null while connecting an outlet, th
});
var view = appController.connectOutlet({
- name: 'mainView',
- view: TestApp.PostView,
+ outletName: 'mainView',
+ viewClass: TestApp.PostView,
controller: null
});

7 comments on commit be69395

@josepjaume

So now it's cleaner, what about allowing connectOutlets itself to be a string so then we could:

Ember.Route.create({
  route: '/posts',
  connectOutlets: 'posts'
});

Or even just simply:

Ember.Route.create({route: 'posts'});
@tchak

+1

Ember.Route.create({
  route: '/posts',
  connectOutlets: 'posts'
});
@wycats
Owner

This is tricky for two reasons:

  1. The context of an outlet is often not possible to determine automatically. Any time you're setting the context as an Array, you basically have to decide manually. For example, if you were in a post and wanted to show trackbacks, you would need to do router.get('postController').connectOutlet('trackbacks', post.get('trackbacks'). The context doesn't come from anywhere else in particular.
  2. It won't work for multiple outlets, which happens in a master-detail.

In short, this solution has a similar problem as the old ViewState solution: it demos well and feels good, but falls apart in the face of common, real-world scenarios.

I'd rather not have two visually very different ways to do things based on small, common differences in the specifics of what you're trying to achieve.

@josepjaume

You're totally right.

Maybe it's the router.get('someController').connectOutlet('someOtherThing') idiom that could be improved somehow?

@trek
Owner

Multiple outlets doesn't appear to be working after this commit: http://jsfiddle.net/aMcgW/1/

@trek
Owner

Oh, nope. It works, just not with instances of ObjectController, which try to proxy the outlet name property to content.

@krisselden
Owner

@trek working fiddle with ObjectController http://jsfiddle.net/CmxAa/

Thinking of adding to the ControllerMixin an inject(property, value) method that will check if the property is defined and if not, define it, then set it, so that it will work with ObjectController without you having to define it first.

something like:

inject: function (key, value) {
  if (!key in this) {
    Ember.defineProperty(this, key);
  }
  this.set(key, value);
}

And using then when injecting properties into controllers. @wycats thoughts?

Please sign in to comment.
Something went wrong with that request. Please try again.