Skip to content

Commit

Permalink
updated examples and updated layoutmanager to allow render to be assi…
Browse files Browse the repository at this point in the history
…gned in initialize
  • Loading branch information
tbranyen committed Jun 17, 2012
1 parent f899bad commit b2c2f5d
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 69 deletions.
72 changes: 52 additions & 20 deletions backbone.layoutmanager.js
Expand Up @@ -7,11 +7,16 @@

"use strict";

// Alias the libraries from the global object
// Alias the libraries from the global object.
var Backbone = window.Backbone;
var _ = window._;
var $ = window.$;

// Store a reference to the original View _configure function.
var _configure = Backbone.View.prototype._configure;
// Store a reference to the original View render function.
var render = Backbone.View.prototype.render;

var LayoutManager = Backbone.View.extend({
// This is a named function to improve logging and debugging within browser
// dev tools. Typically you do not use "anonymous" named functions since IE
Expand Down Expand Up @@ -104,21 +109,19 @@ var LayoutManager = Backbone.View.extend({
options = view._options();

// Ensure render is set correctly.
if (options.render !== LayoutManager.prototype.options.render) {
view.render = options.render;
options.render = LayoutManager.prototype.options.render;
}
//if (options.render !== LayoutManager.prototype.options.render) {
// view.render = options.render;
// options.render = LayoutManager.prototype.options.render;
//}

// Set up the View.
LayoutManager.setupView(view, options);

// Add in all missing LayoutManager properties and methods.
view._render = view.render;

// If no render override was specified assign the default
if (view.render === Backbone.View.prototype.render) {
view._render = function(layout) {
return layout(this).render();
// If no render override was specified assign the default; if the render
// is the fake function inserted, ensure that is updated as well.
if (view.render.__fake__) {
view._render = function(manage) {
return manage(this).render();
};
}

Expand Down Expand Up @@ -474,6 +477,7 @@ var LayoutManager = Backbone.View.extend({
// Configure a View to work with the LayoutManager plugin.
setupView: function(view, options) {
var proto = Backbone.LayoutManager.prototype;
var keys = _.keys(LayoutManager.prototype.options);

// Extend the options with the prototype and passed options.
options = view.options = _.defaults(options || {}, proto.options);
Expand All @@ -488,16 +492,16 @@ var LayoutManager = Backbone.View.extend({
__manager__: {}
});

// If the user provided their own render override, use that instead of the
// default.
if (this.render !== proto.render && !this._render) {
this._render = this.render;
this.render = proto.render;
}
// Pick out the specific properties that can be dynamically added at
// runtime and ensure they are available on the view object.
_.extend(options, _.pick(this, keys));

// By default the original Remove function is the Backbone.View one.
view._remove = Backbone.View.prototype.remove;

// Reset the render function.
view.options.render = LayoutManager.prototype.options.render;

This comment has been minimized.

Copy link
@sebdeckers

sebdeckers Jul 9, 2012

This is making it impossible to override the render method for a LayoutManager instance. What is the purpose of this reset? I'm trying to do the following in my code:

var layout = new Backbone.LayoutManager({
  render: function (template, context) {
    return Mustache.to_html(template, context);
  }
});

This comment has been minimized.

Copy link
@tbranyen

tbranyen Jul 9, 2012

Author Owner

That is a bug, will fix asap.


// If the user provided their own remove override, use that instead of the
// default.
if (view.remove !== proto.remove) {
Expand All @@ -519,7 +523,7 @@ var LayoutManager = Backbone.View.extend({
}
},

// Completely remove all subViews
// Completely remove all subViews.
removeView: function(root, append) {
// Can be used static or as a method.
if (!_.isObject(root)) {
Expand Down Expand Up @@ -555,16 +559,44 @@ _.each(["get", "set", "insert"], function(method) {

// Attach the singular form.
backboneProto[method + "View"] = layoutProto[method + "View"];

// Attach the plural form.
backboneProto[method + "Views"] = layoutProto[method + "Views"];
});

var test = Backbone.View.prototype.initialize;

_.extend(Backbone.View.prototype, {
// Add the ability to remove all Views.
removeView: LayoutManager.removeView,

// Add options into the prototype.
_options: LayoutManager.prototype._options
_options: LayoutManager.prototype._options,

// Override initialize to provide extra functionality that is necessary
// before any custom initialize functions are provided.
_configure: function() {
var retVal = _configure.apply(this, arguments);

// Only update the render method for non-Layouts, which need them.
if (!this.__manager__) {
// Ensure the proper setup is made.
this._render = this.options.render || this.render;

// Ensure render functions work as expected.
this.render = function() {
this.render();

This comment has been minimized.

Copy link
@atomictag

atomictag Jun 21, 2012

Contributor

Doesn't this cause an infinite recursion? I have a 'root' Backbone.View that renders a Backbone.Layout inside of itself which keeps spinning.
Removing this makes it work again.

This comment has been minimized.

Copy link
@tbranyen

tbranyen Jun 21, 2012

Author Owner

You don't nest Layout Managers inside of Views.
https://github.com/tbranyen/backbone.layoutmanager#create-and-render-a-layout

"Note: Nesting LayoutManagers is not supported. If you want sub layouts, simply read on about nesting Views."

This comment has been minimized.

Copy link
@atomictag

atomictag Jun 21, 2012

Contributor

I understand that one cannot nest a LayoutManager inside a LayoutManager - and that's ok given nested views.
However, it seems very reasonable to attach a LayoutManager inside of a regular view (it's a "chrome" view in my case), in the same way as in "adding a layout to the DOM" https://github.com/tbranyen/backbone.layoutmanager/tree/#adding-a-layout-to-the-dom That was working fine up to 0.5.1 - and it still does if the recursive this.render = function() { this.render(); } in lines 587-589 is removed. Perhaps I completely missed the purpose of this assignment?

This comment has been minimized.

Copy link
@tbranyen

tbranyen Jun 21, 2012

Author Owner

Its used to support the this.something.on("something", this.render, this); syntax. Instead of having to do:

this.something.on("something", function() {
  this.render();
}, this);

can you make a failing unit test so I can patch this?

This comment has been minimized.

Copy link
@atomictag

atomictag Jun 21, 2012

Contributor

Yup - I understand. On the other hand it is very convenient to nest LayoutManagers inside regular Backbone.View objects (e.g. one root view that defines the chrome with one or more LayoutManagers to manage parts of it). Strangely enough by removing the culprit lines I got my setup to work fine - a root view that attaches a LayoutManager in its vanilla render() - as well as the this.something.on('something', this render, this) bit (but I confess I haven't digged much into the inner workings to claim I have really understood what's happening). A failing test is a good idea indeed

This comment has been minimized.

Copy link
@atomictag

atomictag Jun 21, 2012

Contributor

added to issue #98, which seems to have reported the same problem with regular views outside of layouts

This comment has been minimized.

Copy link
@tbranyen

tbranyen Jun 22, 2012

Author Owner

Its impossible for that binding to work as you described without this code. it would be binding the function reference to the wrong render. unless of course you're binding outside of the initialize function. this code is specifically for initialize

This comment has been minimized.

Copy link
@atomictag

atomictag Jun 22, 2012

Contributor

I see what you mean. Indeed I don't bind inside initialize - but kinda lazily, after the view has rendered.

This comment has been minimized.

Copy link
@atomictag

atomictag Jun 22, 2012

Contributor

the problem is that no Backbone.View will work outside of a Layout (I mistakenly thought the problem was limited to Layouts embedded in Views - but actually just about any view that is not part of a layout would go into an infinite loop as soon as render() is called...)

This comment has been minimized.

Copy link
@tbranyen

tbranyen Jun 22, 2012

Author Owner

Correct, that is by design. How would this work outside of lm?

Backbone.View.extend({
  render: function(manage) {
    return manage(this).render();
  }
})

This comment has been minimized.

Copy link
@atomictag

atomictag Jun 22, 2012

Contributor

The problem is that no other "regular" view not embedded in a layout - with the simplest render - will work anymore...

var SomeView = Backbone.View.extend({
  el : $('.whatever'),
  render: function() {        
    this.$el.html("<p>here I am</p>");
    return this;
  }
});

will go into an infinite recursion with this change...
The design assumption seems to be that there must be only one LayoutManager at the root of everything with nested views inside of it (and none outside of it) - while a very common case is probably to use the LayoutManager only on parts of the UI alongside other "regular" Backbone.View's (something that was perfectly possible up until 0.5.1)

Personally I like the fact that layouts and regular views can be mixed and matched - with the only rule 'no nested layouts'.
The fact Layout views require a very different render() implementation can be puzzling at first - but actually helps getting to a point where very few views really ever require render to be customized... which I think is nice. On the other hand it's sometimes weird to deal with views that require completely different render() implementations depending on whether they live inside a layout or not. I guess things would be a bit simpler if there was a specialized LayoutView type extension instead of monkey-patching the Backbone.View prototype

};

// Mark this function as fake for later checking and overriding in the
// setView function.
if (this._render === render) {
this.render.__fake__ = true;
}
}

return retVal;
}
});

// Convenience assignment to make creating Layout's slightly shorter.
Expand Down
2 changes: 1 addition & 1 deletion dist/backbone.layoutmanager.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions examples/append.html
Expand Up @@ -40,8 +40,8 @@ <h1>Hello</h1>
// This will place the contents of the Content View into the main
// Layout's <p></p>.
views: {
// THIS IS WHY YOU NEED TO CALL IT EXPLICITY
"p": [new Content()]
// Appending a new content view using the array syntax
p: [ new Content() ]
}
});

Expand Down
7 changes: 2 additions & 5 deletions examples/events.html
Expand Up @@ -54,20 +54,17 @@ <h1>My super important list...</h1>

// Custom render function is necessary to inject at runtime.
render: function(manage) {
// Allow LayoutManager to take over this View.
var view = manage(this);

// Iterate the collection, inserting a new Item View for each entry.
this.collection.each(function(model) {
// Using the insert without a selector will automatically insert
// the View into the root element... in this case its an <ol></ol>.
view.insertView(new Item({
this.insertView(new Item({
model: model
}));
}, this);

// This is required for LayoutManager to handle View rendering.
return view.render();
return manage(this).render();
}
});

Expand Down
9 changes: 3 additions & 6 deletions examples/incremental.html
Expand Up @@ -46,25 +46,22 @@ <h1>My super important list...</h1>

// Custom render function is necessary to inject at runtime.
render: function(manage) {
// Allow LayoutManager to take over this View.
var view = manage(this);

// Iterate the collection, inserting a new Item View for each entry.
this.collection.each(function(model) {
// Using the insert without a selector will automatically insert
// the View into the root element... in this case its an <ol></ol>.
view.insertView(new Item({
this.insertView(new Item({
model: model
}));
}, this);

// This is required for LayoutManager to handle View rendering.
return view.render();
return manage(this).render();
},

initialize: function() {
this.collection.on("add", function(model) {
this.setView(new Item({ model: model }), true).render();
this.insertView(new Item({ model: model })).render();
}, this);
}
});
Expand Down
7 changes: 2 additions & 5 deletions examples/list.html
Expand Up @@ -46,20 +46,17 @@ <h1>My super important list...</h1>

// Custom render function is necessary to inject at runtime.
render: function(manage) {
// Allow LayoutManager to take over this View.
var view = manage(this);

// Iterate the collection, inserting a new Item View for each entry.
this.collection.each(function(model) {
// Using the insert without a selector will automatically insert
// the View into the root element... in this case its an <ol></ol>.
view.insertView(new Item({
this.insertView(new Item({
model: model
}));
}, this);

// This is required for LayoutManager to handle View rendering.
return view.render();
return manage(this).render();
}
});

Expand Down
7 changes: 2 additions & 5 deletions examples/remove.html
Expand Up @@ -46,20 +46,17 @@ <h1>My super important list...</h1>

// Custom render function is necessary to inject at runtime.
render: function(manage) {
// Allow LayoutManager to take over this View.
var view = manage(this);

// Iterate the collection, inserting a new Item View for each entry.
this.collection.each(function(model) {
// Using the insert without a selector will automatically insert
// the View into the root element... in this case its an <ol></ol>.
view.insertView(new Item({
this.insertView(new Item({
model: model
}));
}, this);

// This is required for LayoutManager to handle View rendering.
return view.render();
return manage(this).render();
},

initialize: function() {
Expand Down

0 comments on commit b2c2f5d

Please sign in to comment.