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 for using {{render}} with same template multiple times #2312

Merged
merged 1 commit into from
Mar 28, 2013

Conversation

sebastianseilund
Copy link
Contributor

Yehuda added support for using {{render}} with same template multiple times with different models in 47dfe5e. But when using it, Ember threw an assertion error, "This view is already rendered", for everything but the first {{render}} call.

This should be fixed now, and I added a test for it and adjusted the assertion error message.

@darthdeus
Copy link
Member

This is a duplicate of #2274

@sebastianseilund
Copy link
Contributor Author

Thanks! Must have missed #2274. I think that a combination of those two would be optimal.

The test should make a template with two different models, like in mine. And both the change you made and the one I made in Ember.Handlebars.registerHelper('render'... should be there. I'll update this PR accordingly.

@sebastianseilund
Copy link
Contributor Author

I also added another test where the render helper is used with the same name but both with and without a model.

@wycats
Copy link
Member

wycats commented Mar 21, 2013

Can you rebase this against master?

@sebastianseilund
Copy link
Contributor Author

@wycats Done :)

@@ -44,7 +44,9 @@ Ember.onLoad('Ember.Handlebars', function(Handlebars) {
container = options.data.keywords.controller.container;
router = container.lookup('router:main');

Ember.assert("This view is already rendered", !router || !router._lookupActiveView(name));
if (!context) {
Copy link
Member

Choose a reason for hiding this comment

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

In general, we want all conditionals to be part of the assert so that they can get stripped out for prod/min builds. This means changing the assert condition to context || !router || !router._lookupActiveView(name).

@wagenet
Copy link
Member

wagenet commented Mar 27, 2013

I think once the one small fix I suggested is made then we're good to go.

@wagenet wagenet closed this Mar 27, 2013
@wagenet wagenet reopened this Mar 27, 2013
@sebastianseilund
Copy link
Contributor Author

@wagenet Makes sense :). I have fixed the assertion now. Thanks.

tomdale added a commit that referenced this pull request Mar 28, 2013
Fix for using {{render}} with same template multiple times
@tomdale tomdale merged commit 2cf2548 into emberjs:master Mar 28, 2013
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.

5 participants