New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix region clean up, when view destroy #3262
Conversation
@@ -72,6 +72,10 @@ const Region = MarionetteObject.extend({ | |||
|
|||
this._attachView(view, options); | |||
|
|||
if (this._isReplaced) { | |||
view.on('before:destroy', this._empty, this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm... should https://github.com/marionettejs/backbone.marionette/pull/3262/files#diff-b8da5292cb07ec757ac64e78ff5af5c1R67
be updated to 'before:destroy' instead of doing this?
describe('and restore, when view was self destroyed', function() { | ||
beforeEach(function() { | ||
this.view.trigger('before:destroy', this.view); | ||
this.view._removeElement(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this.view.destroy();
should do the event emitting instead in order to match a real scenario.
👏 Good find. @rafde I don't think However I think the easiest cheapest solution to this is to add https://jsfiddle.net/bbapmrsw/1/ I don't think clean up of this event will be necessary either as the events will be cleaned up. |
@ftdebugger any interest on revising this a bit? |
Region with "replaceElement" setting don't restore region element, when view was destroyed by self. Because view element remove from DOM before
destroy
event fired. Listenbefore:destroy
fix this problem.