Skip to content

Commit

Permalink
Implement BaseView#preDestroy hook
Browse files Browse the repository at this point in the history
Use a test-driven development approach to define acceptance criteria for
the method *before* writing the implementation. This aspect of our
process is not especially useful for other developers, so the individual
commits have been "squashed" together.
  • Loading branch information
jugglinmike committed Sep 8, 2014
1 parent f2f593f commit 15c3594
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 14 deletions.
10 changes: 3 additions & 7 deletions src/modules/components/photo/webcam.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,14 @@ define(function(require) {
this.collection = options.collection;
},

// Override the default implementation of `destroy` to avoid a known bug in
// Google Chrome that keeps the camera running even after destroying the
// video element. Call the original implementation to preserve expected
// behavior.
destroy: function() {
// Avoid a known bug in Google Chrome that keeps the camera running even
// after destroying the video element.
preDestroy: function() {
var video = this.$('video')[0];

if (video) {
video.src = undefined;
}

return BaseView.prototype.destroy.apply(this, arguments);
},

filterAndSave: function() {
Expand Down
6 changes: 6 additions & 0 deletions src/modules/core/base-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ define(function(require) {

var BaseView = Backbone.View.extend({

preDestroy: function() {},

destroy: function() {
// In case an instance has specialized logic for cleaning itself up,
// invoke the `preDestroy` hook before any actual destruction occurs.
this.preDestroy();

this.stopListening();
this.$el.empty();

Expand Down
10 changes: 3 additions & 7 deletions src/modules/layouts/one-row.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,10 @@ define(function(require) {
return this;
},

// Override the default implementation of `destroy` to remove the element's
// class. Call the original implementation to preserve expected behavior.
destroy: function() {
// remove the container class name before we empty
// the element.
// remove the container class name before we empty
// the element.
preDestroy: function() {
this.$el.removeClass('single');

return BaseLayout.prototype.destroy.call(this, arguments);
}
});

Expand Down
15 changes: 15 additions & 0 deletions test/unit/tests/base-layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,21 @@ define(function(require) {

sinon.assert.callCount(this.subView.destroy, 1);
});

test(
'Invokes the `preDestroy` method prior to removing the layout element',
function() {
var layout = this.layout;
layout.render();
sinon.stub(layout, 'preDestroy', function() {
assert.equal(layout.$('.b').length, 1);
});

layout.destroy();

sinon.assert.callCount(layout.preDestroy, 1);
}
);
});
});
});
18 changes: 18 additions & 0 deletions test/unit/tests/base-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,24 @@ define(['modules/core/base-view'], function(BaseView) {

assert.equal(this.view.$el.contents().length, 0);
});

test(
'invokes custom `preDestroy` hook prior to emptying the view',
function() {
var view = this.view;
view.template = sinon.stub().returns('<p>');

sinon.stub(this.view, 'preDestroy', function() {
assert.equal(view.$('p').length, 1);
});

view.render();
this.view.destroy();

sinon.assert.callCount(this.view.preDestroy, 1);
assert.equal(view.$('p').length, 0);
}
);
});
});
});

0 comments on commit 15c3594

Please sign in to comment.