Skip to content

Conversation

heme
Copy link

@heme heme commented Mar 28, 2013

I frequently find the need to render HTML strings that have not yet been inserted into the DOM. I currently do this by...
var frag = $('<div/>').html(htmlTemplateString);
frag.render(data, directives);
$('#notifications').prepend(frag);

(this is mostly for lazy loaded templates that don't need to be inserted until after they are rendered)

This commit allows the context parameter on the render method to be an HTML string. The render method then returns a fully rendered DOM node ready to appended/prepended. So, The above code can be made DRYer by...
var frag = Transparency.render(htmlTemplateString, data, directives);
$('#notifications').prepend(frag);
Considerations:
  1. The HTML string always gets wrapped in a DIV element. I do not know if this would have adverse affects on rendering; the developer/designer would have to be aware of this as she/he setups their templates. Could the wrapper element be passed in with options param? e.g {container: 'span'}
  2. I notice you detach the context element to avoid reflows/paints. This element should not be attached to the DOM at all. I couldn't tell if some code need to be skipped in this case.
  3. This was my first experience with CoffeeScript. I did compile and it ran correctly for me.

Nice library. Thank You for sharing all your hard work!

IF context parameter is a string, THEN
create a DIV
AND insert context into that DIV
AND reassign context = DIV
@pyykkis
Copy link
Contributor

pyykkis commented Mar 28, 2013

Wow, that's a great idea! Wrapper element as an option sounds good, I'll take a closer look at morning. Regarding points 2. and 3, everything seems to be fine 👍

Thanks for the well-thought pull request.

@bleadof
Copy link

bleadof commented Mar 28, 2013

I see this as a fairly common usecase! 👍 It would be awesome if you included a test too! :-)

@pyykkis
Copy link
Contributor

pyykkis commented Mar 29, 2013

I got a chance to think about this for a while. Regarding {container: span} option. How would you guys see, could the wrapper element be just a part of the template?

tmpl = '<ul><li class="todo"></li></ul>'

Transparency.render(tmpl, [{todo: 'Foo'}, {todo: 'Bar'}]);
/*
<ul>
  <li class="todo">Foo</li>
  <li class="todo">Bar</li>
</ul>
*/

That would be consistent with the existing API, as I'm not sure how container option should work, if one uses it with a piece of DOM instead of HTML string.

There's also an existing workaround in case you use jQuery in the project, as it can be used for parsing HTML strings:

// Template is defined with the wrapper element
tmpl = '<div> ... </div>'

$('#notifications').prepend($(tmpl).render(data));

That said, the feature is still valid and valuable for those who don't use jQuery/Zepto.

@heme
Copy link
Author

heme commented Mar 29, 2013

Container

I think that the container being in the template/HTML String is ideal. This is inline with how Transparency renders today, i.e. it gets passed a single DOM element. The container element in the HTML string would be equivalent to that DOM element. The question becomes how does Transparency handle invalid HTML or the following HTML?

<div id="my-id"></div>
<div class="my-class"></div>
<input name="my-name" />
<div data-bind="my-data"></div>

(No container element passed to Transparency)

Is this something that jQuery/Zepto already handles well? Do you just want to tell the developer this is his/her responsibility to do before passing to Transparency?

jQuery Shortcut

Thanks for the shortcut. That is what I should be using with my jQuery example. That will help me remove some boilerplate, and makes the code much more manageable/acceptable. Thanks! I think you should think about making this part of the docs if you decide to forgo any String/HTML/Template passing.

I'm good with whatever you decide. Thanks again for sharing. This is great stuff! I'm not so sure about the coffeeScript though! ;)

@heme
Copy link
Author

heme commented Mar 29, 2013

Testing

The container issue causes another problem related to testing. I'm not sure how to write the test, because the resulting template from Transparency has an extra DIV container element around it. This no longer matches the expected template. I guess the answer is to keep the template html the same, but wrap the expected in another DIV as this is what is expected.

  it "should accept context as a string", ->
    template = '<div class="hello"></div>'
    data     = hello: 'Hello'
    expected = $ '<div><div class="hello">Hello</div></div>'

    template = $ window.Transparency.render template, data
    expect(template).toBeEqual expected

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.

3 participants