leonidas / transparency

Transparency is a semantic template engine for the browser. It maps JSON objects to DOM elements by id, class and data-bind attributes.
http://leonidas.github.com/transparency/
MIT License
969 stars 112 forks source link

Issues when clearing and/or updating the values #23

Closed juhamust closed 12 years ago

juhamust commented 12 years ago

While inserting new values seems to work ok, removing the entries does not reflect on the outcome as expected. Also, after value removal, the elements can no longer be updated.

Issue shown as an example:

<script type="text/javascript">
$('tbody.users').render([{username:'user1'}]); // OK
$('tbody.users').render([{username:'user1'},{username:'user2'}]); // OK
$('tbody.users').render([{username:'user1'}]); // NOK: user2 still shown
$('tbody.users').render([{username:'user1'},{username:'user3'}]); // NOK: user2 still shown
$('tbody.users').render([{username:'user4'},{username:'user3'}]); // NOK: user4 updated, user2 still shown
</script>
<table>
<tbody class="users">
    <tr>
        <td class="username"></td>
    </tr>
</tbody>
</table>

Tested with browser Google Chrome 19.0.1049.3 dev

juhamust commented 12 years ago

Reproduced issue with Firefox 10.0.2 as well.

pyykkis commented 12 years ago

I'll investigate and fix immediately.

pyykkis commented 12 years ago

Most probably the error is on lines 58-60:

  # Remove leftover templates from DOM and save them to the cache for later use.
  while models.length < context.transparency.instances.length
    context.transparency.templateCache.push(
      (context.removeChild n) for n in context.transparency.instances.pop())

Out of my head, I'd say fix would be

  # Remove leftover templates from DOM and save them to the cache for later use.
  while models.length < context.transparency.instances.length
    context.transparency.templateCache.push(
      (n.parentNode.removeChild n) for n in context.transparency.instances.pop())

I'll write a test case exposing the bug and implement the fix.

pyykkis commented 12 years ago

Fix available in master and npm. Apologies for the inconvience and thanks for reporting the issue.

juhamust commented 12 years ago

Thanks for the speedy fix! Confirmed to now work as expected.