#304 ✓ checked-in
David Humphrey

Create a templating plugin [mustache]

Reported by David Humphrey | January 29th, 2011 @ 08:15 PM | in 0.4

The recent PBS demo we did made me think that we'd do well to have the ability to use templates for situations where we need to consume data (json) and produce html. Lots of modern templating engines are implemented in JavaScript now (http://mustache.github.com/ is one, https://json-template.googlecode.com/svn/trunk/doc/Introducing-JSON... another, and there are dozens more). What I'm imagining is a plugin that knows how to take a feed or data source, a template, and combine them to produce html that gets put in a div.

Comments and changes to this ticket

  • David Humphrey

    David Humphrey February 12th, 2011 @ 04:06 PM

    • State changed from “new” to “assigned”
    • Milestone set to 0.4
    • Milestone order changed from “23” to “0”

    I've started this, and chosen to use Mustache. Here's my work thus far, still have to do tests and sort some things out, but working well.

    Branch: https://github.com/humphd/popcorn-js/tree/304
    Commit: https://github.com/humphd/popcorn-js/commit/737ba5849755717baae6f8f...

  • David Humphrey

    David Humphrey February 12th, 2011 @ 09:12 PM

    • State changed from “assigned” to “peer-review-requested”
    • Title changed from “Create a templating plugin” to “Create a templating plugin [mustache]”
    • Assigned user set to “David Humphrey”

    OK, this is ready for review.

    Branch: https://github.com/humphd/popcorn-js/tree/304

    The idea behind Mustache is that you can write a hybrid markup, with {{name}} blocks that refer to names within JSON or an Object. Mustache takes care of merging a template with JSON/Object data, and producing HTML (or any text).

    See http://mustache.github.com/#demo for a demo of what it does. I've copied the demo in popcorn.mustache.html if you want to try it.

  • David Humphrey
  • annasob

    annasob February 14th, 2011 @ 10:44 AM

    This is going to land soon. It will fix the remote script problem.

  • annasob

    annasob February 14th, 2011 @ 10:55 AM

    • State changed from “peer-review-requested” to “review-needs-work”

    Code looks great but a couple of things:
    -options.static might be a problem as static is a reserved word. -i am assuming the demo should have 3 different templates (even though the data is the same). I altered the data: items: name: of each template for testing purposes by adding a number to the end of the name (2 for the second template and 3 for the third) like so:

    items: [
      { name: "red2", first: true, url: "#Red" },
      { name: "green2", link: true, url: "#Green" },
      { name: "blue2", link: true, url: "#Blue"}
    ],
    
    When I play the demo with the altered data I get the third template appearing all three times. I am not sure what the cause of this is but I am assuming it is not a desired outcome. Perhaps we should leave the test data unique per template to make it easier to test. :)
  • David Humphrey

    David Humphrey February 14th, 2011 @ 06:29 PM

    • State changed from “review-needs-work” to “peer-review-requested”

    Thanks, good catches. Fixes on my 304 branch, r?

    https://github.com/humphd/popcorn-js/commit/ca47490350a4729d8b874fc...

  • Rick

    Rick February 14th, 2011 @ 07:06 PM

    • State changed from “peer-review-requested” to “under-review”
  • Rick

    Rick February 14th, 2011 @ 07:48 PM

    • State changed from “under-review” to “review-needs-work”

    One thing to note is that once Popcorn.getScript() lands, you'll be able to swap out the remote script request code.

    Aside from that, can you take a look at the style guide: https://webmademovies.lighthouseapp.com/projects/63272/styleguide

    Things to look closely at:

    1. Whitespace conformance
    2. Variable declaration conformance
    3. Single/Double Quote consistency
    4. End of line comments

    With regard to optimization, I think you could benefit from storing a reference to those typeof calls...

    This...

    
    var shouldReload = !!options.dynamic;
    
    if (typeof options.template === 'function') {
      if (!shouldReload) {
        set('template', options, options.template(options));
      } else {
        set('getTemplate', options, options.template);
      }
    } else if (typeof options.template === 'string') {
      set('template', options, options.template);
    } else {
      throw "Mustache Plugin Error: options.template must be a String or a Function.";
    }
    
    if (typeof options.data === 'function') {
      if (!shouldReload) {
        set('data', options, options.data(options));
      } else {
        set('getData', options, options.data);
      }
    } else if (typeof options.data === 'string') {
      set('data', options, JSON.parse(options.data));
    } else if (typeof options.data === 'object') {
      set('data', options, options.data);
    } else {
      throw "Mustache Plugin Error: options.data must be a String, Object, or Function.";
    }
    

    Could be optimized, perhaps like this:

    
    var shouldReload = !!options.dynamic,
        typeTemplate = typeof options.template, 
        typeData = typeof options.data;
    
    //  Process potential errors first, exit early.
    if ( ["function", "string"].indexOf( typeTemplate ) === -1 ) {
      throw "Mustache Plugin Error: options.template must be a String or a Function.";
    }
    if ( ["function", "string", "object"].indexOf( typeData ) === -1 ) {
      throw "Mustache Plugin Error: options.data must be a String, Object, or Function.";
    }
    
    if ( typeTemplate === 'function' ) {
      if ( !shouldReload ) {
        set( 'template', options, options.template(options) );
      } else {
        set( 'getTemplate', options, options.template );
      }
    } else {
      set( 'template', options, options.template );
    } 
    
    if ( typeData === 'function' ) {
      if ( !shouldReload ) {
        set( 'data', options, options.data(options) );
      } else {
        set( 'getData', options, options.data );
      }
    } else {
    
      if ( typeData === 'string') {
        set('data', options, JSON.parse(options.data));
      } else {
        set('data', options, options.data);
      } 
    }
    

    That's just a recommendation, I think you see where I'm going with the optimizations (also trying to limit the else/ifs)

  • David Humphrey

    David Humphrey February 14th, 2011 @ 09:03 PM

    • State changed from “review-needs-work” to “peer-review-requested”

    OK, fixed: https://github.com/humphd/popcorn-js/commit/34fad181582535d5fede84d...

    You'll have to help me understand vertical white space rules. The style guide is really foreign to me, so I need to try and get my head around when to do things vs. blindly following. Also, do you guys have a pretty-printer for jquery-style code?

  • Rick

    Rick February 15th, 2011 @ 09:42 AM

    Humph, the style guide is actually based on several documents:

    http://autonome.wordpress.com/2006/03/24/

    • Specifically:

      • Adhere to the style of the code you’re fixing (or fix it, if it’s incorrect!)

      • Use inline comments liberally

    http://javascript.crockford.com/style1.html

    • General JavaScript Style

    http://javascript.crockford.com/code.html

    • Statement formation

    • despite this publication, mulitple var per scope is discouraged.

    http://docs.jquery.com/JQuery_Core_Style_Guidelines

    • Specifically with regard to whitespace that promotes code readability

    http://na.isobar.com/standards/

    As for vertical whitespace, I generally think it's a good idea to have empty lines separating blocks - again for readability.

  • Rick

    Rick February 15th, 2011 @ 11:00 AM

    • State changed from “peer-review-requested” to “review-needs-work”

    A few more review notes...

    Can you update the quotes throughout the source to be consistent? Either single or double, but please not both.

    Everything else is cool - thanks for those updates.

  • David Humphrey

    David Humphrey February 15th, 2011 @ 11:32 AM

    • State changed from “review-needs-work” to “super-review-requested”
  • annasob

    annasob February 15th, 2011 @ 05:36 PM

    • State changed from “super-review-requested” to “review-needs-work”

    Ok the code looks good. One error on line 26 of popcorn.mustache.html. However when i fix this and play the demo the templates dont appear to work when you seek back to the position. So I let it play and I see Test 1- Test 3. When I get close to the end I seek back to 5,or 15 nothing happens. Dave can you look into this?

  • David Humphrey

    David Humphrey February 15th, 2011 @ 10:53 PM

    • State changed from “review-needs-work” to “super-review-requested”
    • Assigned user changed from “David Humphrey” to “annasob”

    Thanks for the great reviews, both of you. Fixed the unclosed string literal.

    Branch: https://github.com/humphd/popcorn-js/tree/304
    Commit: https://github.com/humphd/popcorn-js/commit/294a8f3282e547755bf74c6...

    Anna, I looked at the other issue you mentioned, and it's not confined to my plugin. I filed a spin-off bug to deal with that issue, see #345:

    https://webmademovies.lighthouseapp.com/projects/63272-popcorn-js/t...

  • annasob

    annasob February 16th, 2011 @ 11:52 AM

    • State changed from “super-review-requested” to “staged”

    Great!

    Staged in annasob/popcorn-js commit

  • annasob

    annasob March 21st, 2011 @ 02:47 PM

    • State changed from “staged” to “checked-in”
    • Milestone order changed from “24” to “0”
  • annasob

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Popcorn.js is an HTML5 video framework that lets you bring elements of the web into your videos.

Popcorn.js is a project of Web Made Movies, Mozilla's Open Video Lab.

Shared Ticket Bins

Tags

Referenced by

Pages