diff --git a/spec/javascripts/region.spec.js b/spec/javascripts/region.spec.js index a3e6ef6df1..90f34557a5 100644 --- a/spec/javascripts/region.spec.js +++ b/spec/javascripts/region.spec.js @@ -99,6 +99,8 @@ describe('region', function() { this.regionSwapSpy = this.sinon.spy(); this.viewBeforeShowSpy = this.sinon.spy(); this.viewShowSpy = this.sinon.spy(); + this.regionEmptySpy = this.sinon.spy(); + this.regionBeforeEmptySpy = this.sinon.spy(); this.view = new this.MyView(); this.sinon.spy(this.view, 'render'); @@ -112,7 +114,8 @@ describe('region', function() { this.myRegion.on('before:show', this.regionBeforeShowSpy); this.myRegion.on('before:swap', this.regionBeforeSwapSpy); this.myRegion.on('swap', this.regionSwapSpy); - + this.myRegion.on('empty', this.regionEmptySpy); + this.myRegion.on('before:empty', this.regionBeforeEmptySpy); this.view.on('before:show', this.viewBeforeShowSpy); this.view.on('show', this.viewShowSpy); @@ -193,6 +196,11 @@ describe('region', function() { expect(this.regionBeforeSwapSpy).to.have.been.called; }); + it('should trigger empty once', function() { + expect(this.regionEmptySpy).to.have.been.calledOnce; + expect(this.regionBeforeEmptySpy).to.have.been.calledOnce; + }); + it('should trigger a swap event for the region', function() { expect(this.regionSwapSpy).to.have.been.called; }); @@ -739,4 +747,34 @@ describe('region', function() { expect(this.region.empty).to.have.been.called; }); }); + + describe('when destroying a view in a region', function() { + beforeEach(function() { + this.setFixtures('
'); + this.beforeEmptySpy = new sinon.spy(); + this.emptySpy = new sinon.spy(); + + this.region = new Backbone.Marionette.Region({ + el: '#region' + }); + + this.region.on('before:empty', this.beforeEmptySpy); + this.region.on('empty', this.emptySpy); + + this.View = Backbone.Marionette.View.extend({ + template: _.template('') + }); + + this.view = new this.View(); + + this.region.show(this.view); + this.region.currentView.destroy(); + }); + + it('should remove the view from the region after being destroyed', function() { + expect(this.beforeEmptySpy).to.have.been.calledOnce.and.calledWith(this.view); + expect(this.emptySpy).to.have.been.calledOnce.calledWith(this.view); + expect(this.region.currentView).to.be.undefined; + }); + }); }); diff --git a/src/marionette.region.js b/src/marionette.region.js index 8196b18c17..77c244dd4c 100644 --- a/src/marionette.region.js +++ b/src/marionette.region.js @@ -1,4 +1,4 @@ -/* jshint maxcomplexity: 10, maxstatements: 28 */ +/* jshint maxcomplexity: 10, maxstatements: 29 */ // Region // ------ @@ -154,6 +154,13 @@ _.extend(Marionette.Region.prototype, Backbone.Events, { var _shouldShowView = isDifferentView || forceShow; if (_shouldShowView) { + + // We need to listen for if a view is destroyed + // in a way other than through the region. + // If this happens we need to remove the reference + // to the currentView since once a view has been destroyed + // we can not reuse it. + view.once('destroy', _.bind(this.empty, this)); view.render(); if (isChangingView) { @@ -218,19 +225,31 @@ _.extend(Marionette.Region.prototype, Backbone.Events, { // current view, it does nothing and returns immediately. empty: function() { var view = this.currentView; - if (!view || view.isDestroyed) { return; } - - this.triggerMethod('before:empty', view); - // call 'destroy' or 'remove', depending on which is found - if (view.destroy) { view.destroy(); } - else if (view.remove) { view.remove(); } + // If there is no view in the region + // we should not remove anything + if (!view) { return; } + this.triggerMethod('before:empty', view); + this._destroyView(); this.triggerMethod('empty', view); + // Remove region pointer to the currentView delete this.currentView; }, + // call 'destroy' or 'remove', depending on which is found + // on the view (if showing a raw Backbone view or a Marionette View) + _destroyView: function() { + var view = this.currentView; + + if (view.destroy && !view.isDestroyed) { + view.destroy(); + } else if (view.remove) { + view.remove(); + } + }, + // Attach an existing view to the region. This // will not call `render` or `onShow` for the new view, // and will not replace the current HTML for the `el`