-
Notifications
You must be signed in to change notification settings - Fork 20
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add tests for all the special table nodes (col and thead look missing). Otherwise looks ok. Thanks!
@sorvell Done! |
@TimvdLippe This is awesome! Thanks! |
I'm wondering if the similar fix can be applied to the |
@web-padawan Hm yes that should be possible. Let me update the PR tomorrow |
template.js
Outdated
['tr']: ['tbody', 'table'], | ||
['th']: ['tr', 'tbody', 'table'], | ||
['td']: ['tr', 'tbody', 'table'] | ||
'option': ['select'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW maybe also optgroup
tag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is working correctly in IE11, is it not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, I was just guessing. Here is the glitch to check: https://morning-fuel.glitch.me
@sorvell Is there anything more, that stops this PR from merging? |
But it will be failed in this case: <table>
<tbody><template>
<tr><td>Something...</td></tr>
</template></tbody>
</table> HTML parser of IE will drop |
@TechQuery Yes and there is nothing we can do about it 😢 The |
@TimvdLippe So I can only allow users to replace |
Unfortunately that is the result of parsing of IE11. Don't think you have any other option. I would like it to be different, but haven't found a workable solution 😢 |
I got a workable solution for you if you have to use a table layout. I created a custom element for each layers of my table:
With this solution, I'm able to do |
Using the fix from jQuery when setting the
innerHTML
text. See the links in the source code for the original implementation. This solution was previously submitted in a different form in #16 and #19 by @kapouer, but these were never merged for other unrelated reasons.Fixes #3