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

Don't wrap views if using templates #546

Closed
meleyal opened this issue Aug 8, 2011 · 28 comments
Closed

Don't wrap views if using templates #546

meleyal opened this issue Aug 8, 2011 · 28 comments

Comments

@meleyal
Copy link

meleyal commented Aug 8, 2011

Problem

Most of my render methods end with something like:

var template = _.template( $("#asset-template").html(), data);
$(this.el).html(template);

With a template like:

<script id="asset-template" type="text/template">
  <li class="asset {{ classes }}">
    <img src="{{ img_url }}" data-url="{{ url }}" data-description="{{ description }}">
    <span class="caption">{{ description }}</span>
  </li>
</script>

So the template gets inserted into this.el (<div> by default).

But I tend to write my templates 'fully-formed' (as above), not requiring any extra wrapper elements. This presents 2 problems:

  • Your <li> is wrapped in a div (producing invalid markup) or another unneeded element
  • Your events are more verbose: 'click .asset' : 'select' instead of just 'click' : 'select'

One solution would be to exclude the <li> from the template, and have Backbone construct it with the el, className, etc. properties. But a big benefit of using templates is that they are not mixed with the view logic, and that you can write them just like normal HTML.

Solution(s)

My suggestion would be a new template property:

App.Views.Asset = Backbone.View.extend({

  events: {
    'click' : 'select'
  },

  template: _.template($("asset-template").html()),

    ...

}

If template is present, it's presumed the template is fully-formed and no wrapper elements are created.

Another option would be the ability to assign the template directly to this.el, while keeping the event delegation working:

// this currently works, but you lose event delegation
this.el = _.template( $("#asset-template").html(), data);

Could be I'm missing something and it's possible to achieve 'unwrapped templates' now, but after using Backbone for a couple of projects I kept stumbling on this.

@bruth
Copy link

bruth commented Aug 8, 2011

I support this feature in theory, but the issue comes down to operations that depend on the this.el. The usage of the template and the context/data that is passed to the template can be quite diverse across implementations. My current solution is to override this.el in the render method:

render: function() {
    // unbind all handlers since delegateEvents was called in the constructor
    $(this.el).unbind();
    this.el = this.template(this.model.toJSON());
    this.delegateEvents();
}

One possible solution to facilitate use of templates (and prevent redundant operations), is to check if this.template is defined during initialization. It is is, this.el would not be created and any operations that immediately depends on it (delegateEvents) would not occur.

@moll
Copy link

moll commented Aug 11, 2011

I myself handle non-wrapped this.els with a helper method on Backbone.View, that uses a template function I define on every view, renders the view with it and replaces the existing this.el with the newly rendered element.

Backbone.View.prototype.renderTemplate = function() {
  var args = Array.prototype.slice.apply(arguments); args.unshift({});
  this.el = this.template(_.extend.apply(_, args)).replaceAll(this.el)[0];
  this.delegateEvents();
  return this;
};

The above uses the view's template fn which in my app is jQuery Templates and therefore returns a jQuery object.

You'd use it in your render method:

render: function() {
  this.renderTemplate(this.model.attributes, {any_other: "values"});
}

@geddski
Copy link

geddski commented Aug 23, 2011

I'd really like this feature as well. Perhaps a simpler syntax though:

Backbone.View.extend({
el: null
})

or take advantage of document fragments somehow.

@piotr-cz
Copy link

Hi. +1 for the feature.
I'm too having two Views for an unordered list: List and Items.

  1. First issue is attaching events to the View element -
    I ended up attaching events to list items on the list Views.
  2. Another is adding attributes that are not supported (besides: tagName, className, id).
    I set the data-attributes that contain specific model attributes on the item View in the render method. Obviously I would prefer doing so using template, at least because it's more convenient.
  3. Then the template itself.
    I'd like to have an option to render same template files on the server side but I'll have to implement mechanism for creating <ul> and <li> there as well to mimic Backbone View

@kasperp
Copy link

kasperp commented Jan 5, 2012

+1

@kristoferjoseph
Copy link

+1
I like the el:null fix I find myself using tagName to get around this.

@jdkealy
Copy link

jdkealy commented Jan 20, 2012

+1

@jashkenas
Copy link
Owner

A large part of the advantage of using Backbone Views is the fact that they have their element available at all times -- regardless of whether a template has been rendered (many views have multiple templates) -- regardless of wether the view is present in the DOM or not.

This allows you to create and add views to the DOM, rendering later, and be sure that all of your events are bound correctly (because they're delegated from view.el).

If you can come up with a good strategy that allows a view to have "complete" templates while preserving the above characteristics ... then let's talk about it -- but it must allow the root element to exist without having to render the contents of the template (which may depend on data that might not arrive until later), and it must allow a view to easily have multiple templates.

@fguillen
Copy link

Can we use the setElement() to workaround this? http://stackoverflow.com/questions/11594961/backbone-not-this-el-wrapping

@geddski
Copy link

geddski commented Jul 21, 2012

Regarding @jashkenas requirements listed above: a view only ever needs one template if your templating engine supports nested templates, like Handlebars' partials. Most do. A template can be rendered before its data is returned from the server, in fact this is a great way to make your app feel responsive. Render instantly on click, render again when data comes back.

The only real trick about this feature is events, since they're bound to the root element (which is a good thing) so the template has to render to create that root elem. But subsequent renders mean you have to rewire your events. Might be worth testing out to see if that hurts performance at all.

In any case, backbone could support this feature for the majority whose templating engine meets certain requirements. It would certainly be an improvement to the current workflow of putting part of your HTML template into the JS view.

@moll
Copy link

moll commented Jul 21, 2012

@fguillen Yeah. I've been doing this since Backbone started — render the template the moment render is called (replacing this.el), add to DOM, then rerender the changed parts when this.model is finally fetched. After every full render, delegateEvents was called, but that is now even done for you by calling setElement.

The goal of @jashkenas can IMO be achieved much better by the convention that no view is DOMmable before render is called. That reduces the coupling between the view class and the view template, of which the former would otherwise have to have tagNames and friends. Postponing DOM stuff also allows you to unit test without useless DOM creations.

@moll
Copy link

moll commented Jul 22, 2012

@GEDDesign What requirements do you mean?

A simple solution would be just to not touch this.el in initialize and let people do whatever they want in whichever method they choose. Conventionally in render. And in the end call setElement.

If people wish to have this.el available right after new MyView (as opposed to (new MyView).render()), then there's initialize. I personally don't, and neither do I wish <div>s in <ul>s because I don't like tagName coupling between JS and HTML.

But regardless of the future, today you can already do that — just ignore the "ensured" element before calling render.

@srcspider
Copy link

@jashkenas

This allows you to create and add views to the DOM, rendering later, and be sure that all of your events are bound correctly (because they're delegated from view.el).

Given the following document.

<!doctype html>
<meta charset="UTF-8">
<title>Test</title>

<div id="demo">
</div>

<script src="http://cdnjs.cloudflare.com/ajax/libs/jquery/1.9.1/jquery.js"></script>
<script src="test.js"></script>

Where test.js is the following:

var $demo = $('#demo');
var $el = $('<div></div>');
var $template = $('<div id="template"><button>Click Me</button></ul>');

$demo.append($el);
$el.on('click', 'button', function () { alert('works'); });

function cloneCopyEvent(src, dest) {

    if ( dest.nodeType !== 1 || !jQuery.hasData( src ) ) {
        return;
    }

    var type, i, l,
        oldData = jQuery._data( src ),
        curData = jQuery._data( dest, oldData ),
        events = oldData.events;

    if ( events ) {
        delete curData.handle;
        curData.events = {};

        for ( type in events ) {
            for ( i = 0, l = events[ type ].length; i < l; i++ ) {
                jQuery.event.add( dest, type, events[ type ][ i ] );
            }
        }
    }

    // make the cloned public data object a copy from the original
    if ( curData.data ) {
        curData.data = jQuery.extend( {}, curData.data );
    }
}

cloneCopyEvent($el.get(0), $template.get(0));
$el.replaceWith($template);

The following will happen:

  • a empty div is created
  • the empty div is attached to the dom
  • an event is delegated to the empty div
  • a template div with a button replaces the empty div
  • if you click the button the event initially attached to the empty div works

The cloneCopyEvent is directly from jQuery source.

If you can come up with a good strategy that allows a view to have "complete" templates while preserving the above characteristics ... then let's talk about it -- but it must allow the root element to exist without having to render the contents of the template (which may depend on data that might not arrive until later), and it must allow a view to easily have multiple templates.

I'm not fully familiar with backbone to get the last multiple templates part, but the solution above covers everything else. But anyway...

From my POV the current behavior of being able to attach events to what is effectively nothing is a niche "hold your hand" feature with very annoying practical implications: stupid divs everywhere, coupling of template html and application logic, coupling of template root elements into application logic, etc

If people are binding events directly it's easy for them to ensure the template is loaded before they bind the events, if they don't bind events directly and do it though backbones "click .something" feature there's even less of an issue. So what problem is this whole div business solving?

Also, assuming the problems its solving existed, obviously the problem of "can't have clean templates" is also there. If you can't have both at the same time, then the solution should be to have both, not be biased towards one of them. Frankly I'd rather not have event handling integrated into the views at all then have these annoying to work with views.

@tbranyen
Copy link
Collaborator

There are lots of issues with this and I don't think everyone has thought them through. I help maintain Backbone Layout Manager that has the ability to set el: false to "unwrap" the <div>. This has been problematic for us, because el is a Node element. If you have multiple "top level" elements, what does el become? A NodeList? That's probably not what you would expect.

Event binding becomes more complicated too with top level elements as well. Do you bind events to all the top level elements separately (losing the benefit of delegation), or do you bind to the parent and then have to worry about cleaning up other Elements?

Another weird thing is with re-rendering, you end up having to do do a replaceChild on the first element in the list.

Anyways there are a lot of issues with this technique and it's hard to generalize and simplify the problem without having lots of documentation.

@braddunbar
Copy link
Collaborator

Just for posterity, you can now supply View#el as a function and not lose event delegation.

var View = Backbone.View.extend({
  el: _.template('<li>...</li>')
});

@tbranyen I've not seen any code with this.$el.length > 1, but my initial reaction would be to stay away from it. It breaks from convention and will likely lead to strange behavior since most views are written expecting this.$el.length === 1. Are there legitimate use cases for this or are you just looking for a way to more elegantly require 1 top level element?

@srcspider
Copy link

@tbranyen what's the use case for needing to have templates with more then one root element inside it that can't just have a single wrapping element such as an empty div like you're forced to at the moment?

The only case I can think of is if you want a template that is essentially body replacement, since every other element would have a pretty normal single parent element regardless of what depth you go into and personally I see concatenation of sets of elements as being something outside the scope of what a template should do (a template should be somewhat equivalent to a custom tag). The body replacement is extremely niche use case since it means you don't have things like script tags and other things like a wrapping element for your page and instead have all your stuff in the head which IMO is very bad practice.

@tgriesser
Copy link
Collaborator

Also maybe worth noting, events and el can be passed in the options... so you could potentially do this for quick one-off views:

var view = new Backbone.View({
  events: {
    'click li': function () { ... }
  },
  el: _.template('<ul><% this.collection.each(function (m) { %><li>...</li><% }); %></ul>'),
  collection: someCollection
});

@RStankov
Copy link
Contributor

I like having the wrap element, it is useful. Almost all of my views have templates but I just don't include the wrapper element there. I don't think that the complexity of having the view to "guess" if it needs the a wrapper and where to get is worth it. But I could see a lot of "bugs" coming with such a feature. Since "Backbone" doesn't have any knowledge about a tempting system. If we are going to have this feature or similar, in my mind is better to have some template opinions build in, before this feature.

@srcspider
Copy link

@tgriesser

Doesn't the template function there produce a function that needs to be called with data?

@RStankov (and multi-root templates by @braddunbar)

The following is the current implementation of _ensureElement called on view initialization:

_ensureElement: function() {
  if (!this.el) {
    var attrs = _.extend({}, _.result(this, 'attributes'));
    if (this.id) attrs.id = _.result(this, 'id');
    if (this.className) attrs['class'] = _.result(this, 'className');
    var $el = Backbone.$('<' + _.result(this, 'tagName') + '>').attr(attrs);
    this.setElement($el, false);
  } else {
    this.setElement(_.result(this, 'el'), false);
  }
}

Judging by the code it seems at the moment there is no such support for multiple roots in one template, since there are a lot of assumptions being made there of only one element existing. Essentially the only reason you have multiple tags inside the root of the template is because you're forced to but the current wanky implementation where backbone must generate the root element. Remove that requirement and you remove the multiple roots argument.

I think the confusion stems from tagName and className which from what I can see are no more then hacks to get the current system to kind-of work in it's corner cases where it's completely unacceptable to have it do what it currently does.

Essentially, is there really a difference between the following:

Backbone.View.extend({
  tagName: 'li',
  className: 'something something-else'

  // ...
});

...

<script type="text/x-underscore-template" id="example-template">
  <p>...</p>
  <p>...</p>
  <p>...</p>
</script>

Compared to just having:

Backbone.View.extend({
  // ...
});

<script type="text/x-underscore-template" id="example-template">
  <li class="something something-else" any-other-attribute>
    <p>...</p>
    <p>...</p>
    <p>...</p>
  </div>
</script>

Other then the second is more flexible and does a much better job of splitting up how the markup looks and how backbone glues it all together. (tagName and className would be deprecated/removed properties in the second method).

@tgriesser
Copy link
Collaborator

@srcspider - If el is a function, it's called with the current view as the context.

@RStankov
Copy link
Contributor

@srcspider the problem is not just deprecating tagName, className, attributes, el and start using only $el since it can be a jquery collection, this is fairly easy. The problem is when this element will be rendered and active. When is the rendering happening, how it relates to the rendering system you are using.

@srcspider
Copy link

@RStankov

The problem is when this element will be rendered and active. When is the rendering happening, how it relates to the rendering system you are using.

You'll have to be more specific, not really following what the problem is here and how the current system is solving anything and replacing the original div isn't.

@RStankov
Copy link
Contributor

@srcspider currently you are always sure that el is set, since _ensureElement is called really early. And knowing that you can do interesting stuff with it.

For example, you can insert a view into the DOM, before it has any content, and when the content arrives, then render the view content (I know that insert, render is slower than render, insert, but in some cases it can be helped). If we are fetching the view element(s) from the template we need to evaluate the template before my data is there add conditions and this will complicate this workflow or have a nasty dom element replacement and event binding.

Also the good thing about current system is that I call render N times, the content of my view changes, but the element is the same.

@srcspider
Copy link

@RStankov

For example, you can insert a view into the DOM, before it has any content, and when the content arrives, then render the view content

You would still have a empty default div until it's replaced by some proper template (if it's replaced). Anything you can do now with a empty div you can do if we replace the div later, I don't see the difference that's a disadvantage.

If we are fetching the view element(s) from the template we need to evaluate the template before my data is there add conditions and this will complicate this workflow or have a nasty dom element replacement and event binding.

What do you mean by "evaluate the template before your data is there"? If you are working with a empty div I don't see what data-anything you're actually doing with it. If you're inserting stuff into it then you're not using templates for the element, you're just using templates to populate it; there should be no difference to the current way of doing things.

Inserting said div into the dom is the same since the step of processing the template doesn't really change between the two approaches other then that currently you place the contents of the template inside the div and hence the container has to be hardcoded into your application, while in the second approach you replace the div with the template itself and copy the events, which doesn't require the markup to be hardcoded into the application. Again if you're only using templates to just throw content into your container and not using templates to define the actual container nothing changes; you just use the default div as you do now.

With regard to "add conditions and this will complicate this workflow or have a nasty dom element replacement and event binding," what conditions? Yes you wouldn't call this.$el.html when you want to replace the template, but I don't see the problem. As I said I feel the current approach is too heavily based on holding your hand over practical needs such as having views be nice and clean and independent of markup. You should be able to make an application and just by changing the template alter the way the application is structured, ie. changing from using div/div to using clean ul/li to using some html5 pattern, or add classes and any other attributes or markup with out going though programming hoops or needing to duplicate the code to support multiple approaches in markup.

Thinking about it the difference can probably be implemented as simply an extra method that you would use when you want the template to be replaced and not inserted in.

@srcspider
Copy link

Basic prototype implementation for discussion purposes,

app.View = Backbone.View.extend({

    // copy events from one element to another
    _copyEvents: function (src, dest) {
        if (dest.nodeType !== 1 || ! Backbone.$.hasData(src)) {
            return;
        }

        var type, i, l,
        oldData = Backbone.$._data(src),
        curData = Backbone.$._data(dest, oldData),
        events = oldData.events;

        if (events) {
            delete curData.handle;
            curData.events = {};

            for (type in events) {
                for (i = 0, l = events[type].length; i < l; i++) {
                    Backbone.$.event.add(dest, type, events[type][i]);
                }
            }
        }

        // make the cloned public data object a copy from the original
        if (curData.data) {
            curData.data = Backbone.$.extend({}, curData.data);
        }
    },

    // replace current root element with given el, and copies events from 
    // current root element to new element
    markup: function (new_el) {
        if (new_el instanceof Backbone.$) {
            new_el = new_el.get(0);
        }
        else if (typeof new_el === 'string') {
            new_el = $(new_el.trim()).get(0);
        }

        this._copyEvents(this.el, new_el);
        this.$el.replaceWith(new_el);
        this.el = new_el;
        this.$el = Backbone.$(this.el);
    }

});

Where app.View would be what you use as the base view.

eg.

(function ($) {

    var app = window.ExampleNamespace;

    app.Index = app.View.extend({

        template: app.template('Index'),

        events: {
            'click button': 'test'
        },

        render: function () {
            this.markup(this.template(this));
            return this;
        },

        test: function (event) {
            alert('works');
        }

    });

}(window.jQuery));

@jvitela
Copy link

jvitela commented Aug 20, 2015

Instead of trying to replace the original View's element, why not define tagName as a function and make it return the tag of the root element of the template?
For example, if the template is something like:

<section class="panel" id="panel123">
<!-- contents -->
</section>
this.tagName() // return 'section'

Then the render function has to insert only the template's contents:

render: function() {
   // Assuming $template is a jquery object with the rendered template
    this.$el.empty().append( $template.children());
}

Here is a working example

@ylacaute
Copy link

@jvitela : Your solution works well but sometimes it adds a little delay which I dont have at all when I use partial template (where root tags are generated from the view). Moreover, it is still a workaround.

I have started to learn Backbone for one week so maybe I miss something but your solution is the only one I saw wich is fully functionnal. This is surprising, I think it is an essential feature.

  • Is there a native way to define the root tag template IN the template and not in the View ? If yes, can you please give us an example ?
  • If it's not possible, what is the associated pull request ? this one Use template as el ?
    Thanks.

@jvitela
Copy link

jvitela commented Sep 29, 2015

Hi @ylacaute,

There is NO native way to define the root tag IN the template. This is because Backbone is template agnostic, you are free to choose whatever you want as template system and is imposible to provide a generic way to achieve this.

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

No branches or pull requests