fix($compile): support templates with thead and tfoot root elements #6290
Conversation
|
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
| directive('replaceWithThead', valueFn({ | ||
| replace: true, | ||
| template: '<thead><tr><td>TD</td></tr></thead>' | ||
| })); |
There was a problem hiding this comment.
There's another section which runs essentially the same tests with asynchronous (templateUrl) directives, just for completeness you could add those too
|
@caitp Thanks for pointing this out. Updated the PR with these new tests |
| leaf = /(td|th)/.test(type) && table.find('tr'); | ||
| if (tbody.length && type !== 'tbody') { | ||
| leaf = /(td|th)$/.test(type) && table.find('tr'); | ||
| if (tbody.length && !/(thead|tbody|tfoot)/.test(type)) { |
There was a problem hiding this comment.
So, I don't totally understand the logic the logic here. If we have a tbody, and we aren't a tbody element, then we set the wrapping element to tbody. But tfoot and thead should always be outside of the tbody, no? So if it's a thead or tfoot, it seems like we'd be missing it.
There was a problem hiding this comment.
@caitp I think you are right, will post a simplified patch
If the first element in a template is a <thead> or a <tfoot>, then use the existing logic to handle table elements compilation. Closes angular#6289
|
@caitp this seems good to me. Can we merge this? |
|
@btford yeah, do the triage crew want to do that? I think we want to get this in for 1.2.14 --- otherwise I'll do it |
|
Go for it @ashleygwilliams |
|
I'll do it since @ashleygwilliams is p busy for a while. |
|
Landed as 53ec5e1. Thanks! |
If the first element in a template is a
<thead>or a<tfoot>, then use the existing logic to handle table elements compilation.Closes #6289