Change .append() to .appendChild() #953
Conversation
| @@ -222,11 +222,12 @@ export function plot(options = {}) { | |||
| if (caption != null || legends.length > 0) { | |||
| figure = document.createElement("figure"); | |||
| figure.style.maxWidth = "initial"; | |||
| figure.append(...legends, svg); | |||
| if (legends.length > 0) figure.appendChild(...legends); | |||
There was a problem hiding this comment.
appendChild unlike append only takes a single argument, so this will need to be a for loop now.
There was a problem hiding this comment.
Got it. Easy change. I think I was assuming that there would only ever be one legend in the array. I'll make this a for of loop.
| if (caption != null) { | ||
| const figcaption = document.createElement("figcaption"); | ||
| figcaption.append(caption); | ||
| figure.append(figcaption); | ||
| typeof caption === "object" ? figcaption.appendChild(caption) : (figcaption.innerHTML = caption); |
There was a problem hiding this comment.
We’ll need a more precise test for an element or text node here. I think we can crib from hypertext literal for the exact logic. And then in the non-element case, we’ll need to create a text node. (We shouldn’t set innerHTML since that would allow the caption to contain HTML and potentially cause encoding issues.)
There was a problem hiding this comment.
Gotcha about the innerHTML. I'm looking at the code now from the hypertext literal. I might need assistance here but let me see what I come up with.
Thanks for the feedback.
|
Merging into a branch so I that I can finish the remaining steps. |
* Change .append() to .appendChild() (#953) * Update plot.js to use .appendChild() rather than .append() * Refactor to use ternary and typeof check * appendChild fixes Co-authored-by: Josh Kenzer <[email protected]>
Reason for change
While using Plot with external frameworks that have their own Dom implementations, it's possible the framework's Dom implementation doesn't support the newer Dom.append() method. This pull requests addresses the instance where plot uses Dom.append() to add a legend or a caption by using the Dom.appendChild() method.
Testing with Salesforce's Lightning Web Component (LWC) framework
I found this issue while trying to use Plot inside of Salesforce.com to create customized visualizations that Salesforce isn't capable of out of the box. I was able to successfully use Plot version 0.5.0 with no issues other than when I tried to add either a legend or a caption.
To test this PR, I built a dist after running tests and uploaded dist/plot.umd.js to Salesforce and added both a legend and caption to my visualizations. Here is a sample screenshot
This contribution addresses the topic raised in this community post.