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

Should setElement replace the element in the dom with the new element? #1639

Closed
wshaver opened this issue Sep 10, 2012 · 2 comments
Closed
Labels

Comments

@wshaver
Copy link

wshaver commented Sep 10, 2012

There seems to be a lot of confusion about DOM rendering related to templates as backbone is expecting an element on creation. For example this question on SO:
http://stackoverflow.com/questions/11594961/backbone-not-this-el-wrapping

And these two previous issues:
#546
#1180

Currently calling setElement doesn't change the DOM at all, only the $el associated with the backbone view.

setElement: function(element, delegate) {
      if (this.$el) this.undelegateEvents();
      this.$el = element instanceof Backbone.$ ? element : Backbone.$(element);
      this.el = this.$el[0];
      if (delegate !== false) this.delegateEvents();
      return this;
}

Instead it could call jquery's replaceWith on the element which would also cause the DOM to update correctly:

setElement: function(element, delegate) {
      if (this.$el) {        
        this.undelegateEvents();
        this.$el.replaceWith(element);  // <----- added this line ----->
      }
      this.$el = element instanceof Backbone.$ ? element : Backbone.$(element);
      this.el = this.$el[0];
      if (delegate !== false) this.delegateEvents();
      return this;
}

This works very well if you want meta-data on your top level template items such as:

<li data-id="{id}" class="well" style="list-style:none">
    Campus: {id} {name}
</li>

With this change one can call: setElement(renderedTemplate);
Before this change it is necessary to call:

var old = this.$el;
this.setElement(renderedTemplate);
old.replaceWith(this.$el);

Not a big deal, but it does highly confuse some new users. I'd be happy to issue a pull request with the one-line addition to setElement, but wasn't sure if this automatic functionality would be desired by everyone. I can't think of a downside but perhaps there are ones other than performance.

Thoughts?

@braddunbar
Copy link
Collaborator

Mornin' @wshaver, thanks for opening an issue!

setElement is meant to be flexible with regard to what you'll do with the element after replacing it. I think we should stick with that because your proposal would fail, or at least add no benefit, in the following situations:

  1. The previous element is document.body or some other equally non-replaceable element. Replacing the element in this case would not be productive.
  2. The previous element is not inserted into the DOM. Replacing it would have no effect or possibly throw.

@wshaver
Copy link
Author

wshaver commented Sep 11, 2012

Thanks for the quick and thoughtful reply.

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

No branches or pull requests

2 participants