christophergandrud / networkD3

D3 JavaScript Network Graphs from R
http://christophergandrud.github.io/networkD3
652 stars 269 forks source link

Sankey: explicit <xhtml:pre> instead of <xhtml:body> for bootstrap-datepicker compatibility #215

Open octaviancorlade opened 7 years ago

octaviancorlade commented 7 years ago

Removes the necessity of declaring a new namespace for each using the <xhtml:body> tag, since there is only one <xhtml:pre> for each title in any case.</p> <p>Working on Chrome 60.0.3112.113, FF 55.0.3, Safari 10.1.2, IE11.</p> <p>Fixes #214 </p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/octaviancorlade"><img src="https://avatars.githubusercontent.com/u/15089539?v=4" />octaviancorlade</a> commented <strong> 7 years ago</strong> </div> <div class="markdown-body"> <p>Just to keep track of what was found with some more testing (see comments on Issue #214):</p> <p>No reason to use <code>xhtml:pre</code> instead of <code>pre</code>, since the namespace is removed in <code>d3.selection.append()</code> and therefore doesn't affect the result.</p> <p>No reason to remove the <code>foreignObject</code> tag, which would be semantically more correct.</p> <p>Just removing the <code>.append("xhtml:body")</code> should be enough.</p> <p>Consistency in appending <code>.html()</code> or <code>.text()</code> would be a good idea, probably using <code>.text()</code> with <code>\n</code> for new lines is easier inside <code>pre</code>.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cjyetman"><img src="https://avatars.githubusercontent.com/u/1708872?v=4" />cjyetman</a> commented <strong> 7 years ago</strong> </div> <div class="markdown-body"> <p>I haven't actually tested this, but based on the discussion and what I know/remember, this gets my seal of approval. Using the <code><foreignObject></code> tag I think is necessary for older browser compatibility, but use of the <code><body></code> tag, namespaced or not, seems to cause conflicts with other JavaScript packages when they select elements in the DOM.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cjyetman"><img src="https://avatars.githubusercontent.com/u/1708872?v=4" />cjyetman</a> commented <strong> 5 years ago</strong> </div> <div class="markdown-body"> <p>this would probably fix #250 as well</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cjyetman"><img src="https://avatars.githubusercontent.com/u/1708872?v=4" />cjyetman</a> commented <strong> 5 years ago</strong> </div> <div class="markdown-body"> <p>The problem that this was intended to workaround has been fixed upstream in <code>shiny</code>, so I'm not sure this needs to be merged anymore. Will leave open for now though as this may be a better strategy than the current one.</p> </div> </div> <div class="page-bar-simple"> </div> <div class="footer"> <ul class="body"> <li>© <script> document.write(new Date().getFullYear()) </script> Githubissues.</li> <li>Githubissues is a development platform for aggregating issues.</li> </ul> </div> <script src="https://cdn.jsdelivr.net/npm/jquery@3.5.1/dist/jquery.min.js"></script> <script src="/githubissues/assets/js.js"></script> <script src="/githubissues/assets/markdown.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/highlight.min.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/languages/go.min.js"></script> <script> hljs.highlightAll(); </script> </body> </html>