codeBlockCopyButton and codeBlockWrapButtons Plugins Side Effects

[gerteck]

Please confirm that you have searched existing issues in the repo

Yes, I have searched the existing issues

Any related issues?

Previous PRs on these Plugins for reference: #1043 #1668

Tell us about your environment

Win 11

MarkBind version

5.5.3

Describe the bug and the steps to reproduce it

The codeBlockCopyButtons.ts and codeBlockWrapButtons.ts plugins introduce unwanted side effects.

See:

markbind/packages/core/src/plugins/codeBlockCopyButtons.ts

Lines 67 to 76 in 2cadb78

	export = {
	getScripts: () => [copyCodeBlockScript],
	processNode: (_: PluginContext, node: MbNode) => {
	if (node.name === 'pre' && !doesFunctionBtnContainerExistInNode(node)) {
	cheerio(node).append(CONTAINER_HTML);
	} else if (isFunctionBtnContainer(node)) {
	cheerio(node).append(getButtonHTML());
	}
	},
	};

markbind/packages/core/src/plugins/codeBlockWrapButtons.ts

Lines 51 to 60 in 2cadb78

	export = {
	getScripts: () => [wrapCodeBlockScript],
	processNode: (_: PluginContext, node: MbNode) => {
	if (node.name === 'pre' && !doesFunctionBtnContainerExistInNode(node)) {
	cheerio(node).append(CONTAINER_HTML);
	} else if (isFunctionBtnContainer(node)) {
	cheerio(node).append(getButtonHTML());
	}
	},
	};

Both plugins process all <pre> nodes and insert copy and wrap buttons to <pre> elements if enabled, no matter if it contains <code> or not.

For some context, note that markdown-it follows the CommonMark spec, and processes triple backticks into (''') into <pre/> and <code>. For example, in the first code block below, this code snippet that we might write in markdown file, gets converted to the following in html on the web page (second code block):

'''js
console.log("Hello, world!");
'''

⬇

<pre><code class="language-js">console.log("Hello, world!");</code></pre>

We should minimize the side effect by only appending/inserting these code-copy or code-wrap buttons if we find <code> in the <pre> element.

Why is this not just a trivial issue? The <pre> (preformatted text) element is used to display text exactly as is written in HTML code, preserving spaces, line breaks and indentation. If any user wants to use this element, as per HTML specifications, they will incur this side effect if they have the plugin enabled. (- MarkBind should support HTML)

This also may have other unexpected effects on other components that use <pre> to format their components. This includes - mermaid example, where the mermaid diagrams are formatted in <pre/> elements for post-processing by mermaid js. Having the plugin enabled causes the buttons to be inserted in the element which should only contain the diagram code, causing the mermaid diagrams to fail to render because of the added syntax inside the block.

Steps to Reproduce

Add to your site.json

  "plugins": [
    "codeBlockWrapButtons",
    "codeBlockCopyButtons"
  ]

We can test a negative impact of this side effect, try adding some ASCII Art as so:

<pre>
   /\_/\
  ( o.o )
   > ^ <
</pre>

Both copy and wrap buttons will show up beside the cat. (They should not show up since the cat is not code, the buttons should only show up for code blocks)

Others

markbind/packages/core-web/src/styles/markbind.css

Lines 288 to 307 in 2cadb78

	pre {
	position: relative;
	display: flex;
	}
	button.function-btn {
	border: 1px solid #d7dadc;
	border-radius: 5px;
	color: darkgray;
	cursor: pointer;
	display: none;
	height: 30px;
	margin: 0.17rem;
	padding: 0.35rem;
	position: absolute;
	right: 0;
	width: 30px;
	text-align: center;
	white-space: nowrap;
	}

PRs #1043 and #1668 also add direct changes to markbind.css, which also have similar negative side effects (on mermaid, and possibly for future implementation). I propose to 1. shift the css changes into the plugins instead of adding it globally, and 2. better scope the css rules to minimize side effects:

This was also mentioned in the PR, I think it was a good catch which would have prevented this portion of the issue:

#1043 (review)

Just a follow-up thought: It feels kinda odd that we add plugin styles to the main markbind.css file. Perhaps we should come up with a better way for plugin writers to keep assets contained within the plugin folder itself.

We can do this by using cheerio to add the style changes through head, or link it as an additional css file, instead of directly adding to the markbind css. This would be better for maintenance in the future as the css would be located together with the plugin.

Some overall refactoring would be good.

Expected behavior

Anything else?

• HTML <pre> Tag
• CommonMark spec for ``` (backticks fenced code blocks))

[Tim-Siu]

This is a great catch! Thank you for your detailed documentation as well! I’ll review your PR shortly.