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

Instantiate a ViewState's view if it's not already an instance #413

Merged
merged 1 commit into from
Jan 23, 2012
Merged

Instantiate a ViewState's view if it's not already an instance #413

merged 1 commit into from
Jan 23, 2012

Conversation

devinus
Copy link
Member

@devinus devinus commented Jan 17, 2012

E.g.:

  var viewState = Ember.ViewState.extend({
    view: Ember.View.extend({
      elementId: 'test-view'
    })
  });

@ghost ghost assigned wycats Jan 17, 2012
@tchak
Copy link
Member

tchak commented Jan 20, 2012

why calling destroy on the view? What happens then I return to this ViewState?

@wagenet
Copy link
Member

wagenet commented Jan 20, 2012

@devinus, @tchak seems right, I have some doubts.

@devinus
Copy link
Member Author

devinus commented Jan 20, 2012

@tchak the view is created again. Destroying the view will ensure that bindings aren't being run on the view when it's no longer being used and the view can GC.

@tchak
Copy link
Member

tchak commented Jan 20, 2012

Yes if it is a class, but if I set view to an instance?

@devinus
Copy link
Member Author

devinus commented Jan 23, 2012

@tchak I've reverted my change destroying the view on exit for now and rebased. Hopefully by whittling down the changes in this pull request it'll be easier to evaluate it's merits.

@wycats
Copy link
Member

wycats commented Jan 23, 2012

I am ok with this. Can you rebase against master?

@devinus
Copy link
Member Author

devinus commented Jan 23, 2012

@wycats Done.

wycats added a commit that referenced this pull request Jan 23, 2012
Instantiate a ViewState's view if it's not already an instance
@wycats wycats merged commit 042490d into emberjs:master Jan 23, 2012
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