Skip to content

Conversation

@mklement0
Copy link

Note that this implicitly fixes the nested-div-wrappers bug with array input data (issue #101), but only if you turn option 'noDivWrapper' on.

@mklement0
Copy link
Author

I'm not 100% confident in this change, but it seems to work well for my purposes. I hope it at least points you in the right direction.

@ghost
Copy link

ghost commented May 9, 2016

Thanks for your nice job @mklement0
It works good!

@codepb
Copy link
Owner

codepb commented May 10, 2016

I'll check when I get the chance and merge. I'm also considering whether this is just an underlying bug that should be fixed.

@mklement0
Copy link
Author

@codepb Thanks. If you're not concerned about backward compatibility, I'd say it's probably worth treating it as a bug (if you're using semantic versioning, you could just bump the major version number).

@gmazza
Copy link

gmazza commented May 11, 2016

Thanks for the PR @mklement0, I just started experimenting with jquery-template yesterday, and immediately ran into this problem. I was hoping to use this tool to generate multiple table rows, which partially works but the unwanted wrapping the end up causing the table-level styling (column widths, in my case) to be ignored -- no matter where I place the width percentages -- at col, th, or with the tr's being created.

@gmazza
Copy link

gmazza commented May 11, 2016

I switched to JsRender, which is working fine for me.

@mklement0
Copy link
Author

Note: @itimryan mentions a potential problem with this PR in a comment on issue #101:

I found your code brought about a new bug, when I use your option
noDivWrapper: true

Then the prepend option will be no longer has any effect
prepend: true

All elements are just appended to the container.
And after I removed
noDivWrapper: true
The prepend option comes back again.

@codepb
Copy link
Owner

codepb commented Jul 6, 2016

This has now been resolved so there is never a div wrapper. Closing as will not merge this as a result.

@codepb codepb closed this Jul 6, 2016
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