Skip to content

Commit

Permalink
fixed bug that prevented default regionType from being used. fixes #674
Browse files Browse the repository at this point in the history
  • Loading branch information
Derick Bailey committed Aug 14, 2013
1 parent d1bbf70 commit 888be67
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 8 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Layout
* Will properly attach regions if the layout's `close` method was called prior to `render`
* Calling `.addRegions` will correctly modify the layout instance' region list instead of the prototype's
* Fixed bug that prevented default `regionType` from being used

* CompositeView
* The `itemViewContainer` can be supplied in the constructor function options
Expand Down
15 changes: 8 additions & 7 deletions spec/javascripts/layout.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,25 +83,26 @@ describe("layout", function(){
layoutManager = new LayoutCustomRegion
});

it("should instantiate the default regionManager if specified", function() {
expect(layoutManager).toHaveOwnProperty("regionThree");
expect(layoutManager.regionThree).toBeInstanceOf(Marionette.Region);
expect(layoutManager).toHaveOwnProperty("regionThree");
expect(layoutManager.regionThree).toBeInstanceOf(Marionette.Region);
});

it("should instantiate specific regions with custom regions if speficied", function() {
expect(layoutManager).toHaveOwnProperty("regionOne");
expect(layoutManager.regionOne).toBeInstanceOf(CustomRegion1);
expect(layoutManager).toHaveOwnProperty("regionTwo");
expect(layoutManager.regionTwo).toBeInstanceOf(CustomRegion2);
});

it("should instantiate the default regionManager if specified", function() {
expect(layoutManager).toHaveOwnProperty("regionThree");
expect(layoutManager.regionThree).toBeInstanceOf(CustomRegion1);
expect(layoutManager).toHaveOwnProperty("regionFour");
expect(layoutManager.regionThree).toBeInstanceOf(CustomRegion1);
});

it("should instantiate marionette regions is no regionType is specified", function() {
var layoutManagerNoDefault = new LayoutNoDefaultRegion();
expect(layoutManagerNoDefault).toHaveOwnProperty("regionTwo");
expect(layoutManagerNoDefault.regionTwo).toBeInstanceOf(Backbone.Marionette.Region);
});

});

describe("when regions are defined as a function", function(){
Expand Down
3 changes: 2 additions & 1 deletion src/marionette.layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ Marionette.Layout = Marionette.ItemView.extend({
addRegion: function(name, definition){
var regions = {};
regions[name] = definition;
return this.addRegions(regions)[name];
return this._buildRegions(regions)[name];
},

// Add multiple regions as a {name: definition, name2: def2} object literal
Expand All @@ -80,6 +80,7 @@ Marionette.Layout = Marionette.ItemView.extend({
var that = this;

var defaults = {
regionType: Marionette.getOption(this, "regionType"),
parentEl: function(){ return that.$el; }
};

Expand Down
1 change: 1 addition & 0 deletions src/marionette.region.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ _.extend(Marionette.Region, {
// ```
//
buildRegion: function(regionConfig, defaultRegionType){

var regionIsString = (typeof regionConfig === "string");
var regionSelectorIsString = (typeof regionConfig.selector === "string");
var regionTypeIsUndefined = (typeof regionConfig.regionType === "undefined");
Expand Down

0 comments on commit 888be67

Please sign in to comment.