Skip to content
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(component-tester): call detached and unbind when disposing of tested component #27

Merged
merged 2 commits into from
Jul 27, 2016

Conversation

tkhyn
Copy link
Contributor

@tkhyn tkhyn commented Jul 23, 2016

ComponentTester.dispose() should also detach and unbind the view.

As calling unbind and detached twice would have no effect, there is no need to monkey-patch them in _prepareLifecycle as it's done for bind and attached.

@EisenbergEffect
Copy link
Contributor

@martingust Can you review this real quick?

@martingust
Copy link
Contributor

@tkhyn @EisenbergEffect I think this change makes sense. Ultimately the dispose function should probably be moved out of the create method and put on class level. @tkhyn Any chance you can get that in as well, if not we can merge this and update later. Thanks!

@tkhyn
Copy link
Contributor Author

tkhyn commented Jul 27, 2016

@martingust sure I'll do that tonight.

I simply assumed that you wanted the dispose to be set there so that for some reason it's available only once create has been called, and wanted to push only minimal changes. I agree it will be cleaner with the class method.

@EisenbergEffect EisenbergEffect merged commit f1585cc into aurelia:master Jul 27, 2016
@martingust
Copy link
Contributor

Thanks @tkhyn!

this.host = document.createElement('div');
this.host.innerHTML = this._html;
document.body.appendChild(this.host);
aurelia.enhance(this._bindingContext, this.host);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enhance returns a promise now. is this still working?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants