Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Copy Button to Code Blocks #138

Closed
wants to merge 18 commits into from
Closed

Add Copy Button to Code Blocks #138

wants to merge 18 commits into from

Conversation

trebor
Copy link
Contributor

@trebor trebor commented Nov 9, 2023

this uses the markdown-it plugin markdown-it-copy to add a copy-button to preview nodes. i've made a few dubious expedient css choices.

  • i've copied default.css from the module into ../public/markdown-it-copy.css and made small modes in there.
    • maybe this could be imported directly from the module, but i couldn't get that to work - happy dig deeper if important now.
  • in this file i have also hidden the language label provided, i assuem, by markdown-it and instead use the label from markdown-it-copy so text aligns better. i am sure there is a better way than a display: none;, but i can't find an option anywhere to disable this.

here is a js code block:

image

a dot code block:

image

a dot code block with the success notification which lasts 2 seconds:

image

i doubt this will be the final visual treatment - for example, a nice copy icon button, but i don't think we have a set of icons in the project yet.

@trebor trebor requested a review from mbostock November 9, 2023 19:55
@trebor
Copy link
Contributor Author

trebor commented Nov 9, 2023

working on failing test now...

@trebor
Copy link
Contributor Author

trebor commented Nov 9, 2023

the library inserts a UUID into the generated html, this causes any snap-shot used by our tests to fail.

it may make sense to use the library as a starting point for our own custom code.

@trebor trebor requested a review from cinxmo November 14, 2023 18:39
@trebor trebor added the design Designs needed for a polished UI label Nov 14, 2023
@trebor
Copy link
Contributor Author

trebor commented Nov 14, 2023

fixes: #17

@Fil Fil self-requested a review November 14, 2023 21:37
Fil added a commit that referenced this pull request Nov 15, 2023
- client.js only
- visual feedback with css
- uses navigator.clipboard.writeText
- adds the copy button on all PRE > CODE (or even PRE > SPAN) elements
- pass data-copy=off to disable

supersedes #138
@Fil Fil mentioned this pull request Nov 15, 2023
Copy link
Contributor

@Fil Fil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using markdown-it was a good way to prototype, but the plugin implementation does not appear to be a good fit for the CLI platform:

  • duplicates the contents in each block, making pages bigger
  • uses a bunch of (obfuscated) code just to call document.execCommand("copy").
  • execCommand is deprecated, we must use navigator.clipboard
  • runs server-side, which means it can't process reactive code blocks.

In terms of UI I want to show "copy" only on hover, and replace it with "success" or "error" (how can it error, I don't know?) after the click.

I'd also want to make this optional (maybe a front-matter option, project option, or even block option).

Let me suggest an alternative approach, that builds on this work, but with a different implementation: #177

@@ -0,0 +1,65 @@
/* hide the langauge label provided by MarkdownIt */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/* hide the langauge label provided by MarkdownIt */
/* hide the language label provided by MarkdownIt */

@@ -21,7 +21,7 @@ describe("parseMarkdown(input)", () => {
const snapshot = parseMarkdown(await readFile(path, "utf8"), "test/input", name);
let allequal = true;
for (const ext of ["html", "json"]) {
const actual = ext === "json" ? jsonMeta(snapshot) : snapshot[ext];
const actual = applySpecialCaseFilters(ext === "json" ? jsonMeta(snapshot) : snapshot[ext]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want the build to be always reproducible, not only in tests; otherwise it's going to be painful for people who track changes of the artifacts, or when you who use a deploy script based on file differences.

</code><div class="m-mdic-copy-wrapper"><span class="u-mdic-copy-code_lang">js</span><div class="u-mdic-copy-notify" id="j-notify">success</div><button class="u-mdic-copy-btn j-mdic-copy-btn" data-mdic-content="function add(a, b) {
return a + b;
}
" data-mdic-attach-content="" data-mdic-notify-id="j-notify" data-mdic-notify-delay="2000" data-mdic-copy-fail-text="fail" onclick="!function(t){const e={copy:(t='',e='')=>new Promise((c,o)=>{const n=document.createElement('textarea'),d=e?`\n\n${e}`:e;n.value=`${t}${d}`,document.body.appendChild(n),n.select();try{const t=document.execCommand('copy');document.body.removeChild(n),t?c():o()}catch(t){document.body.removeChild(n),o()}}),btnClick(t){const c=t&&t.dataset?t.dataset:{},o=c.mdicNotifyId,n=document.getElementById(o),d=c.mdicNotifyDelay,i=c.mdicCopyFailText;e.copy(c.mdicContent,c.mdicAttachContent).then(()=>{n.style.display='block',setTimeout(()=>{n.style.display='none'},d)}).catch(()=>{alert(i)})}};e.btnClick(t)}(this);">copy</button></div></pre>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should roll our own, without this obfuscated code.

@trebor
Copy link
Contributor Author

trebor commented Nov 15, 2023

this is superseded by #177

@trebor trebor closed this Nov 15, 2023
Fil added a commit that referenced this pull request Nov 15, 2023
- client.js only
- visual feedback with css
- uses navigator.clipboard.writeText
- adds the copy button on all PRE > CODE (or even PRE > SPAN) elements
- pass data-copy=off to disable

supersedes #138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Designs needed for a polished UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants