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

template rendering bug ? #61

Closed
DanRoche opened this issue Apr 8, 2018 · 4 comments
Closed

template rendering bug ? #61

DanRoche opened this issue Apr 8, 2018 · 4 comments
Labels
bug Something isn't working released Published

Comments

@DanRoche
Copy link

DanRoche commented Apr 8, 2018

Hello,

i just began to "play" with dna.js, and i find it is great
but i think i may have found a bug, or at least something i do not understand.

in the attached example, ( test.txt - to be renamed as test.html )
the second template is rendered AFTER the last HR despite it is positioned before.
and more, if you remove or comment the last HR the template is not rendered at all !

tested on firefox and chromium.
reproduce problem with dna.js 1.3.4 and 1.3.7

test.txt

@dpilafian dpilafian added the bug Something isn't working label Apr 8, 2018
@dpilafian
Copy link
Member

This bug is because the location of the template within the container is determined by simply looking at the index of the template relative to its siblings. This breaks if there are multiple templates at the same level within the same container.

The workaround is to wrap each template.

For example, replace:

<div id=book class=dna-template>
   <p>Title:  <span>~~title~~</span></p>
</div>

with:

<div class=books>
   <div id=book class=dna-template>
      <p>Title:  <span>~~title~~</span></p>
   </div>
</div>

Wrapping templates has as two advantages:
1) Performance, because dna.js does not have to calculate position within the container
2) Semantics, because multiple clones (plural) are closely related and should be in a group (singular)

Nonetheless, this bug needs to be fixed. The fix has been tested and will be released shortly. The fix involves making the location calculation of unwrapped templates smarter to keep track of templates further up in the DOM and adjust automatically as they are cloned.

dpilafian added a commit that referenced this issue Apr 14, 2018
@dpilafian
Copy link
Member

The original test.txt running with the old dna.js v1.3.7:
https://jsfiddle.net/bwrbch25/7

...and with the new dna.js v1.3.8:
https://jsfiddle.net/bwrbch25/8

@DanRoche
Copy link
Author

DanRoche commented Apr 15, 2018 via email

@dpilafian dpilafian added the released Published label Apr 28, 2018
@dpilafian
Copy link
Member

Release v1.3.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released Published
Projects
None yet
Development

No branches or pull requests

2 participants