Add copy-to-clipboard button to code blocks#313
Conversation
Closes joshbuchea#130 - Vanilla JS in default.html matches existing inline-script convention - Native Clipboard API, no dependency on clipboard.js - Styling reuses One Dark palette (#abb2bf) already in main.css - Button hidden until hover or keyboard focus
There was a problem hiding this comment.
Pull request overview
Adds copy-to-clipboard controls for code blocks in the Jekyll site layout, with inline JavaScript and CSS styling.
Changes:
- Adds a script that creates a copy button for each
<pre>block. - Uses
navigator.clipboard.writeText()to copy code text. - Adds styles to position and reveal the copy button.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
_layouts/default.html |
Adds copy-button creation and clipboard click handling. |
main.css |
Adds positioning, visibility, and visual styling for copy buttons. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| navigator.clipboard.writeText(text).then(function () { | ||
| button.textContent = 'Copied'; | ||
| setTimeout(function () { button.textContent = 'Copy'; }, 1500); | ||
| }).catch(function () { | ||
| button.textContent = 'Failed'; | ||
| setTimeout(function () { button.textContent = 'Copy'; }, 1500); | ||
| }); |
There was a problem hiding this comment.
Two ways to address this:
1. Copilot's version: More defensive, adds about 15 lines, with showCopied/showFailed helpers and try/catch around the call. On unsupported browsers, the button still appears and shows "Failed" when clicked.
2. Minimal version: One-line early return at button creation: if (!navigator.clipboard || typeof navigator.clipboard.writeText !== 'function') return;. On unsupported browsers, no button appears at all, so users get the unchanged baseline experience.
Happy to go either way. Which do you prefer?
There was a problem hiding this comment.
The minimal version sounds great, thanks!
| button.textContent = 'Copy'; | ||
| button.setAttribute('aria-label', 'Copy code to clipboard'); | ||
|
|
||
| button.addEventListener('click', function () { | ||
| var code = pre.querySelector('code'); | ||
| var text = code ? code.innerText : pre.innerText; | ||
| navigator.clipboard.writeText(text).then(function () { | ||
| button.textContent = 'Copied'; | ||
| setTimeout(function () { button.textContent = 'Copy'; }, 1500); | ||
| }).catch(function () { | ||
| button.textContent = 'Failed'; | ||
| setTimeout(function () { button.textContent = 'Copy'; }, 1500); |
There was a problem hiding this comment.
Three ways to address this:
1. Copilot's version. A setButtonState(visibleText, accessibleText) helper that updates both textContent and aria-label together.
2. Remove the aria-label so the button's textContent becomes its accessible name directly.
3. Add an aria-live="polite" status region. A separate element that explicitly announces "Copied" or "Copy failed" when the button is clicked. It's more robust since screen readers don't reliably re-announce a focused button's label change with options 1 or 2.
Option 3 is a little verbose but actually fixes the accessibility gap. I'd lean that way if you're up for it. Happy to do any of the three. Which do you prefer?
There was a problem hiding this comment.
Option 3 sounds great, thanks again.
There was a problem hiding this comment.
Done in the latest commit. Added a static aria-live="polite" region in the layout and a .visually-hidden utility class in main.css. The region updates on copy success/failure.
Heads up though: in my testing, VoiceOver on macOS wasn't announcing the aria-live update when I clicked the button. The button text change to "Copied" seemed to override it. I'm not an accessibility expert and it could be a quirk of my VoiceOver setup, so take that with a grain of salt. The DOM is structurally correct (status region exists, gets updated, has the right attributes), so screen readers that support dynamic aria-live updates well should still get the announcement.
Happy to dig deeper if you want, but figured it was worth shipping the implementation and flagging the caveat rather than chasing a perfect VoiceOver fix.
| opacity: 0; | ||
| text-align: center; | ||
| transition: opacity 0.15s ease-in-out; |
There was a problem hiding this comment.
Good catch. Fixed in the latest commit:
@media (hover: none) {
.copy-btn {
opacity: 1;
}
}Button was invisible on mobile and tablet. Adds @media (hover: none) rule so it shows where hover isn't available as a primary input.
7899b54 to
93c76d0
Compare
Adds an early return at button creation time when navigator.clipboard or its writeText method isn't available. Users on unsupported browsers get the unchanged baseline experience rather than a broken button.
Replaces the redundant aria-label with a shared aria-live='polite' region appended to body, visually hidden via a reusable .visually-hidden class in main.css. Updated on copy success/failure so assistive tech announces the result. The button's textContent serves as its accessible name.
|
All three review points addressed in the latest commits. |
|
|
||
| button.addEventListener('click', function () { | ||
| var code = pre.querySelector('code'); | ||
| var text = code ? code.innerText : pre.innerText; |
There was a problem hiding this comment.
This seems like a good suggestion on the surface.
There was a problem hiding this comment.
Good catch from Copilot. Switching to textContent in the next commit.
| position: absolute; | ||
| top: 8px; | ||
| right: 8px; |
There was a problem hiding this comment.
Good catch. I’d prefer to avoid sticky if possible, so let’s move the button outside the scrollable element instead. That’s how GitHub handles copy buttons in code blocks. It’s a slightly bigger change, but I think it’s worth it here.
There was a problem hiding this comment.
Makes sense to go with the wrapper approach instead of sticky. The JS wraps each <pre> in a .pre-wrapper div at runtime, then positions the button on the wrapper so it stays pinned when the code block scrolls horizontally. I tested it on the long meta tags block and the button holds its position now.
| var statusRegion = document.createElement('div'); | ||
| statusRegion.className = 'visually-hidden'; | ||
| statusRegion.setAttribute('aria-live', 'polite'); | ||
| statusRegion.setAttribute('role', 'status'); | ||
| document.body.appendChild(statusRegion); | ||
|
|
||
| var preBlocks = document.querySelectorAll('pre'); | ||
| var preBlocksLength = preBlocks.length; | ||
| var j; | ||
| for (j = 0; j < preBlocksLength; j++) { | ||
| (function (pre) { | ||
| if (!navigator.clipboard || typeof navigator.clipboard.writeText !== 'function') return; | ||
| var button = document.createElement('button'); | ||
| button.className = 'copy-btn'; | ||
| button.type = 'button'; | ||
| button.textContent = 'Copy'; | ||
|
|
||
| button.addEventListener('click', function () { | ||
| var code = pre.querySelector('code'); | ||
| var text = code ? code.innerText : pre.innerText; | ||
| navigator.clipboard.writeText(text).then(function () { | ||
| button.textContent = 'Copied'; | ||
| statusRegion.textContent = 'Copied to clipboard'; | ||
| setTimeout(function () { button.textContent = 'Copy'; }, 1500); | ||
| }).catch(function () { | ||
| button.textContent = 'Failed'; | ||
| statusRegion.textContent = 'Copy to clipboard failed'; | ||
| setTimeout(function () { button.textContent = 'Copy'; }, 1500); | ||
| }); | ||
| }); | ||
|
|
||
| pre.appendChild(button); | ||
| })(preBlocks[j]); |
There was a problem hiding this comment.
Good point. Moved the Clipboard API check to the top so the whole thing exits early when it's unavailable. No status region or buttons get created at all. Also switched the status region to static HTML in the layout instead of creating it in JS, since VoiceOver doesn't reliably pick up dynamically-injected aria-live regions.
|
@mattsafaii thanks for addressing the review points and providing options 💯 This is a bit of an experiment using Copilot to assist with PR reviews, so apologies for the back and forth. Most of the flagged items seem valid so far, and I appreciate your patience and willingness to make adjustments. |
- Use textContent instead of innerText to preserve code formatting - Wrap each pre in a .pre-wrapper div so the button stays pinned on horizontal scroll - Move Clipboard API check to the top so nothing is created when it's unavailable - Make the aria-live region static HTML for more reliable screen reader support
|
No worries, I've enjoyed working on this. Pushed the changes for all three just now. Also, having Copilot assist with reviews seems to be working well since it caught a few things I missed. Let me know if anything else needs adjusting. |
| .copy-btn { | ||
| position: absolute; | ||
| top: 8px; | ||
| right: 8px; | ||
| min-width: 60px; | ||
| padding: 4px 10px; | ||
| font-family: inherit; | ||
| font-size: 12px; | ||
| color: #abb2bf; | ||
| background: transparent; | ||
| border: 1px solid #abb2bf; | ||
| border-radius: 4px; | ||
| cursor: pointer; | ||
| opacity: 0; | ||
| text-align: center; | ||
| transition: opacity 0.15s ease-in-out; | ||
| } | ||
|
|
||
| .pre-wrapper:hover .copy-btn, | ||
| .copy-btn:focus { | ||
| opacity: 1; | ||
| } |
| navigator.clipboard.writeText(text).then(function () { | ||
| button.textContent = 'Copied'; | ||
| statusRegion.textContent = 'Copied to clipboard'; | ||
| setTimeout(function () { button.textContent = 'Copy'; }, 1500); | ||
| }).catch(function () { | ||
| button.textContent = 'Failed'; | ||
| statusRegion.textContent = 'Copy to clipboard failed'; | ||
| setTimeout(function () { button.textContent = 'Copy'; }, 1500); | ||
| }); |
| var wrapper = document.createElement('div'); | ||
| wrapper.className = 'pre-wrapper'; |
| var preBlocks = document.querySelectorAll('pre'); | ||
| var preBlocksLength = preBlocks.length; |
Closes #130
Adds a copy-to-clipboard button to each code block.
Implementation
_layouts/default.html, matching the existing inline-script conventionnavigator.clipboard.writeText()instead of clipboard.js to avoid a dependencymain.cssreuses the One Dark palette already in the filepre:hoveror.copy-btn:focusTesting
Verified in the browser console against https://htmlhead.dev. Buttons render, copy works, label cycles correctly.