arronhunt / highlightjs-copy

📋❇️ A simple, accessible highlightjs plugin to add a copy button to your code blocks.
https://highlightjs-copy.netlify.app
The Unlicense
98 stars 23 forks source link

What if the <pre> or <code> element does not have a parent? #30

Open khumam opened 6 days ago

khumam commented 6 days ago

I have the article form with a text editor, and I noticed that the code is inlined with other paragraphs, like this:

<p>Hello world</p>
<pre>// code</pre>

When I used highlightjs-copy on my article's read page, it returned an error because the plugin attempted to do the following:

el.parentElement.classList.add("hljs-copy-wrapper");
el.parentElement.appendChild(button);

// The el.parentElement is null

Sometimes, the button appears on top of the article's parent, not inside the <pre> code block. However, the issue was resolved by manually adding a wrapper before calling the plugin:

hljs.addPlugin({
    "after:highlightElement": ({ el, text }) => {
        const wrapperDiv = document.createElement("div");
        el.before(wrapperDiv);
        wrapperDiv.appendChild(el);
    },
});

hljs.addPlugin(new CopyButtonPlugin());\

I think it would be better if we add a handler or condition to check if el.parentElement is null and then add the parent manually to prevent this error. Maybe something like this:

// code

let button = Object.assign(document.createElement("button"), {
    innerHTML: locales[lang]?.[0] || "Copy",
    className: "hljs-copy-button",
});
button.dataset.copied = false;

// Check if the pre code block does not have a parent element
// If not, add a parent
if (el.parentElement === null) {
    const wrapperDiv = document.createElement("div");
    el.before(wrapperDiv);
    wrapperDiv.appendChild(el);
}

el.parentElement.classList.add("hljs-copy-wrapper");
el.parentElement.appendChild(button);

// Add a custom property to the code block so that the copy button can reference and match its background-color value.
el.parentElement.style.setProperty(
    "--hljs-theme-background",
    window.getComputedStyle(el).backgroundColor
);

// code
arronhunt commented 4 days ago

Hey @khumam. Is there are reason you aren't following highlightjs' recommendation to use <pre><code> ? I'm also confused how the parentElement could be null. Worst case, parentElement would be the page body. Are you creating the codeblock at runtime using something like document.createElement?

I'm hesitant to always wrap the codeblock in a div because this could break some implementations. Ideally, this add-on wouldn't add any extra DOM elements. The parent check seems like a working solution, but I want to understand how you got into this edge case first :D