-
Notifications
You must be signed in to change notification settings - Fork 206
Add copy SVG to clipboard option on icon click #412
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
Conversation
Co-authored-by: hawkticehurst <39639992+hawkticehurst@users.noreply.github.com>
|
@copilot for the UI changes can we actually instead show a two icon buttons at the bottom of each card that let's you copy SVG or copy name? Use this screenshot as a reference for the design:
You should use the default codicons copy icon for copying the icon name, and I'll leave it to your discretion to pick an icon from the codicons library that makes sense for copying SVG |
Co-authored-by: hawkticehurst <39639992+hawkticehurst@users.noreply.github.com>
Co-authored-by: hawkticehurst <39639992+hawkticehurst@users.noreply.github.com>
Updated the UI to show two icon buttons at the bottom of each card that appear on hover (commit 2d1da31):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds the ability to copy an icon's SVG markup to clipboard from the preview website. The implementation embeds all SVG content into the HTML at build time as a JavaScript object, with a fallback to fetch individual SVG files if needed.
Key Changes:
- New copy functionality with two buttons per icon: one for copying the icon name and one for copying the SVG markup
- Build-time SVG data embedding via a new script that follows the existing metadata embedding pattern
- Updated UI with hover-triggered copy buttons instead of click-on-icon behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| src/template/preview.hbs | Added CSS styles for copy buttons, HTML structure for dual-button interface, and JavaScript event handlers for copying icon names and SVG content with clipboard API and fallback support |
| scripts/embed-svg-data.js | New build script that reads all SVG files from src/icons and embeds their content into the HTML file as a JavaScript object |
| package.json | Integrated the embed-svg-data script into the build pipeline after embed-metadata |
Comments suppressed due to low confidence (1)
src/template/preview.hbs:673
- The fetch promise chain doesn't check if the response is successful before calling response.text(). If the fetch returns a 404 or other error status, the SVG content might be an error page HTML. Add a check for response.ok before processing the response.
fetch('icons/' + iconName + '.svg')
.then(response => response.text())
.then(svg => {
copyToClipboard(svg, iconName + ' (SVG)');
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function copyToClipboard(text, displayText) { | ||
| if (navigator.clipboard && navigator.clipboard.writeText) { | ||
| navigator.clipboard.writeText(text).then(function() { | ||
| notificationText.innerHTML = displayText; |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The notification message uses the icon name directly in innerHTML without sanitization. While icon names are likely controlled data from the build process, if there's any possibility of user-controlled data in icon names, this could be an XSS vulnerability. Consider using textContent instead of innerHTML for displaying the icon name.
| copyToClipboard(svgData[iconName], iconName + ' (SVG)'); | ||
| } else { | ||
| // Fallback: try to fetch SVG if not in embedded data | ||
| fetch('icons/' + iconName + '.svg') |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fetch fallback for SVG data uses a relative path 'icons/' + iconName + '.svg' but there's no verification that this path is correct or that the icons directory is available at this location in the deployed site. Consider documenting where these files should be located or ensuring the build process copies them to the expected location.
This issue also appears in the following locations of the same file:
- line 670
| <div class="copy-buttons"> | ||
| <button class="copy-btn copy-name-btn" title="Copy icon name" aria-label="Copy icon name"> | ||
| <i class="codicon codicon-copy" aria-hidden="true"></i> | ||
| </button> | ||
| <button class="copy-btn copy-svg-btn" title="Copy SVG" aria-label="Copy SVG"> | ||
| <i class="codicon codicon-file-code" aria-hidden="true"></i> | ||
| </button> | ||
| </div> |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The copy buttons are placed inside the .icon div but outside any semantic grouping. Consider wrapping the buttons in a container with role="group" and aria-label="Copy actions" to provide better context for screen reader users about what these buttons do in relation to the icon.
| copyNameBtn.onclick = function(e){ | ||
| e.stopPropagation(); | ||
| let iconName = this.closest('.icon').getAttribute('data-name'); | ||
| copyToClipboard(iconName, iconName); | ||
| }; | ||
| // Copy SVG button | ||
| let copySvgBtn = icon.querySelector('.copy-svg-btn'); | ||
| copySvgBtn.onclick = function(e){ | ||
| e.stopPropagation(); | ||
| let iconName = this.closest('.icon').getAttribute('data-name'); | ||
| if (svgData[iconName]) { | ||
| copyToClipboard(svgData[iconName], iconName + ' (SVG)'); | ||
| } else { | ||
| // Fallback: try to fetch SVG if not in embedded data | ||
| fetch('icons/' + iconName + '.svg') | ||
| .then(response => response.text()) | ||
| .then(svg => { | ||
| copyToClipboard(svg, iconName + ' (SVG)'); | ||
| }) | ||
| .catch(error => { | ||
| console.warn('Could not fetch SVG for ' + iconName, error); | ||
| notificationText.innerHTML = 'SVG not available'; | ||
| animateNotification(); | ||
| }); | ||
| } | ||
| }; |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code doesn't check if the querySelector calls return null before assigning onclick handlers. If the template structure changes or an icon doesn't have the expected buttons, this will throw an error. Add null checks before assigning the onclick handlers.
| copyNameBtn.onclick = function(e){ | |
| e.stopPropagation(); | |
| let iconName = this.closest('.icon').getAttribute('data-name'); | |
| copyToClipboard(iconName, iconName); | |
| }; | |
| // Copy SVG button | |
| let copySvgBtn = icon.querySelector('.copy-svg-btn'); | |
| copySvgBtn.onclick = function(e){ | |
| e.stopPropagation(); | |
| let iconName = this.closest('.icon').getAttribute('data-name'); | |
| if (svgData[iconName]) { | |
| copyToClipboard(svgData[iconName], iconName + ' (SVG)'); | |
| } else { | |
| // Fallback: try to fetch SVG if not in embedded data | |
| fetch('icons/' + iconName + '.svg') | |
| .then(response => response.text()) | |
| .then(svg => { | |
| copyToClipboard(svg, iconName + ' (SVG)'); | |
| }) | |
| .catch(error => { | |
| console.warn('Could not fetch SVG for ' + iconName, error); | |
| notificationText.innerHTML = 'SVG not available'; | |
| animateNotification(); | |
| }); | |
| } | |
| }; | |
| if (copyNameBtn) { | |
| copyNameBtn.onclick = function(e){ | |
| e.stopPropagation(); | |
| let iconName = this.closest('.icon').getAttribute('data-name'); | |
| copyToClipboard(iconName, iconName); | |
| }; | |
| } | |
| // Copy SVG button | |
| let copySvgBtn = icon.querySelector('.copy-svg-btn'); | |
| if (copySvgBtn) { | |
| copySvgBtn.onclick = function(e){ | |
| e.stopPropagation(); | |
| let iconName = this.closest('.icon').getAttribute('data-name'); | |
| if (svgData[iconName]) { | |
| copyToClipboard(svgData[iconName], iconName + ' (SVG)'); | |
| } else { | |
| // Fallback: try to fetch SVG if not in embedded data | |
| fetch('icons/' + iconName + '.svg') | |
| .then(response => response.text()) | |
| .then(svg => { | |
| copyToClipboard(svg, iconName + ' (SVG)'); | |
| }) | |
| .catch(error => { | |
| console.warn('Could not fetch SVG for ' + iconName, error); | |
| notificationText.innerHTML = 'SVG not available'; | |
| animateNotification(); | |
| }); | |
| } | |
| }; | |
| } |
| display: inline-block; | ||
| margin: 8px; | ||
| } | ||
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of '.icon:hover { cursor: pointer; }' means icons no longer show a pointer cursor on hover, which might reduce discoverability of the copy functionality. Consider adding 'cursor: pointer' to the '.icon:hover .copy-buttons' selector or keeping a hover indicator on the icon itself.
| .icon:hover { | |
| cursor: pointer; | |
| } |
| console.error('HTML file not found:', htmlPath); | ||
| process.exit(1); | ||
| } | ||
|
|
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script checks if the HTML file exists but doesn't verify that the icons directory exists before attempting to read it. This could result in a cryptic error if the directory is missing. Consider adding a check similar to the HTML file verification.
| if (!fs.existsSync(iconsDir)) { | |
| console.error('Icons directory not found:', iconsDir); | |
| process.exit(1); | |
| } |
| function copyToClipboard(text, displayText) { | ||
| if (navigator.clipboard && navigator.clipboard.writeText) { | ||
| navigator.clipboard.writeText(text).then(function() { | ||
| notificationText.innerHTML = displayText; |
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
DOM text
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
In general, the fix is to avoid interpreting untrusted strings as HTML. Instead of assigning potentially tainted text to element.innerHTML, use element.textContent (or an equivalent safe text API) so that any HTML meta-characters are treated as plain text rather than markup.
Concretely, in src/template/preview.hbs, the function copyToClipboard uses notificationText.innerHTML = displayText; in three places (success in the modern clipboard path, success in the fallback, and in the SVG fetch error path). We should change all of these assignments to use notificationText.textContent = displayText;. This preserves the existing behavior of displaying the same text to the user while preventing any embedded HTML inside displayText (or the literal 'SVG not available') from being interpreted as markup. No additional methods or imports are needed; textContent is a standard DOM property. This single change addresses both reported alert variants because both ultimately use displayText in the same sink.
-
Copy modified line R632 -
Copy modified line R640 -
Copy modified line R648 -
Copy modified line R681
| @@ -629,7 +629,7 @@ | ||
| function copyToClipboard(text, displayText) { | ||
| if (navigator.clipboard && navigator.clipboard.writeText) { | ||
| navigator.clipboard.writeText(text).then(function() { | ||
| notificationText.innerHTML = displayText; | ||
| notificationText.textContent = displayText; | ||
| animateNotification(); | ||
| }).catch(function() { | ||
| // Fallback to old method | ||
| @@ -637,7 +637,7 @@ | ||
| copier.select(); | ||
| copier.setSelectionRange(0, 99999); | ||
| document.execCommand('copy'); | ||
| notificationText.innerHTML = displayText; | ||
| notificationText.textContent = displayText; | ||
| animateNotification(); | ||
| }); | ||
| } else { | ||
| @@ -645,7 +645,7 @@ | ||
| copier.select(); | ||
| copier.setSelectionRange(0, 99999); | ||
| document.execCommand('copy'); | ||
| notificationText.innerHTML = displayText; | ||
| notificationText.textContent = displayText; | ||
| animateNotification(); | ||
| } | ||
| } | ||
| @@ -678,7 +678,7 @@ | ||
| }) | ||
| .catch(error => { | ||
| console.warn('Could not fetch SVG for ' + iconName, error); | ||
| notificationText.innerHTML = 'SVG not available'; | ||
| notificationText.textContent = 'SVG not available'; | ||
| animateNotification(); | ||
| }); | ||
| } |
| copier.select(); | ||
| copier.setSelectionRange(0, 99999); | ||
| document.execCommand('copy'); | ||
| notificationText.innerHTML = displayText; |
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
DOM text
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
General approach: Avoid interpreting untrusted text as HTML. Instead of assigning displayText (which may come from data-name) to notificationText.innerHTML, assign it to notificationText.textContent so that any meta-characters are treated as literal text rather than markup. This preserves the visual behavior (showing a notification with the icon name or message) while eliminating the XSS sink.
Detailed best fix: In copyToClipboard, all three assignments to notificationText.innerHTML = displayText; should be changed to notificationText.textContent = displayText;. Likewise, in the SVG fetch catch handler, notificationText.innerHTML = 'SVG not available'; should be changed to notificationText.textContent = 'SVG not available';. These are all in src/template/preview.hbs around lines 631–641 and 679–682. No additional functions or imports are needed; textContent is a standard DOM property and behaves the same across modern browsers. This change does not alter the logic except to prevent HTML parsing of the message text.
-
Copy modified line R632 -
Copy modified line R640 -
Copy modified line R648 -
Copy modified line R681
| @@ -629,7 +629,7 @@ | ||
| function copyToClipboard(text, displayText) { | ||
| if (navigator.clipboard && navigator.clipboard.writeText) { | ||
| navigator.clipboard.writeText(text).then(function() { | ||
| notificationText.innerHTML = displayText; | ||
| notificationText.textContent = displayText; | ||
| animateNotification(); | ||
| }).catch(function() { | ||
| // Fallback to old method | ||
| @@ -637,7 +637,7 @@ | ||
| copier.select(); | ||
| copier.setSelectionRange(0, 99999); | ||
| document.execCommand('copy'); | ||
| notificationText.innerHTML = displayText; | ||
| notificationText.textContent = displayText; | ||
| animateNotification(); | ||
| }); | ||
| } else { | ||
| @@ -645,7 +645,7 @@ | ||
| copier.select(); | ||
| copier.setSelectionRange(0, 99999); | ||
| document.execCommand('copy'); | ||
| notificationText.innerHTML = displayText; | ||
| notificationText.textContent = displayText; | ||
| animateNotification(); | ||
| } | ||
| } | ||
| @@ -678,7 +678,7 @@ | ||
| }) | ||
| .catch(error => { | ||
| console.warn('Could not fetch SVG for ' + iconName, error); | ||
| notificationText.innerHTML = 'SVG not available'; | ||
| notificationText.textContent = 'SVG not available'; | ||
| animateNotification(); | ||
| }); | ||
| } |
| document.execCommand('copy'); | ||
| notificationText.innerHTML = name; | ||
| notificationText.innerHTML = displayText; |
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
DOM text
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
In general, to fix “DOM text reinterpreted as HTML” issues, avoid passing untrusted strings to APIs that interpret them as HTML (innerHTML, outerHTML, jQuery’s html(), etc.). Instead, insert them as plain text using textContent, innerText, or appropriate escaping, so that any meta-characters are rendered literally rather than executed.
For this specific code, the displayText argument to copyToClipboard is derived from data-name and then used for user-visible notifications only. There is no requirement here to support HTML formatting in the notification; it should just display plain text like the icon name or “SVG not available”. Therefore, the best fix is to replace notificationText.innerHTML = displayText; with notificationText.textContent = displayText; in all places where displayText (or a string built from it) is shown. This preserves all current behavior (user still sees the same message, clipboard functionality is unchanged), but ensures that any special characters in data-name are not interpreted as HTML.
Concretely, in src/template/preview.hbs:
- In the
copyToClipboardfunction:- On the success path of
navigator.clipboard.writeText, changenotificationText.innerHTML = displayText;tonotificationText.textContent = displayText;. - On the catch (fallback) path, change the second
notificationText.innerHTML = displayText;similarly. - In the branch for environments without
navigator.clipboard, changenotificationText.innerHTML = displayText;likewise.
- On the success path of
- In the
fetcherror handler in the SVG copy code, changenotificationText.innerHTML = 'SVG not available';tonotificationText.textContent = 'SVG not available';.
No new methods, imports, or definitions are required; textContent is a standard DOM property.
-
Copy modified line R632 -
Copy modified line R640 -
Copy modified line R648 -
Copy modified line R681
| @@ -629,7 +629,7 @@ | ||
| function copyToClipboard(text, displayText) { | ||
| if (navigator.clipboard && navigator.clipboard.writeText) { | ||
| navigator.clipboard.writeText(text).then(function() { | ||
| notificationText.innerHTML = displayText; | ||
| notificationText.textContent = displayText; | ||
| animateNotification(); | ||
| }).catch(function() { | ||
| // Fallback to old method | ||
| @@ -637,7 +637,7 @@ | ||
| copier.select(); | ||
| copier.setSelectionRange(0, 99999); | ||
| document.execCommand('copy'); | ||
| notificationText.innerHTML = displayText; | ||
| notificationText.textContent = displayText; | ||
| animateNotification(); | ||
| }); | ||
| } else { | ||
| @@ -645,7 +645,7 @@ | ||
| copier.select(); | ||
| copier.setSelectionRange(0, 99999); | ||
| document.execCommand('copy'); | ||
| notificationText.innerHTML = displayText; | ||
| notificationText.textContent = displayText; | ||
| animateNotification(); | ||
| } | ||
| } | ||
| @@ -678,7 +678,7 @@ | ||
| }) | ||
| .catch(error => { | ||
| console.warn('Could not fetch SVG for ' + iconName, error); | ||
| notificationText.innerHTML = 'SVG not available'; | ||
| notificationText.textContent = 'SVG not available'; | ||
| animateNotification(); | ||
| }); | ||
| } |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>


Adds the ability to copy an icon's SVG markup to clipboard from the preview website, in addition to the existing copy-icon-name functionality.
Changes
src/template/preview.hbs: Add two icon buttons at the bottom of each card for copying icon name and SVGscripts/embed-svg-data.js: New build script that embeds all SVG content into the HTML as a JS objectpackage.json: Addembed-svg-datascript to build pipelineHow it works
SVG data is embedded at build time via a placeholder pattern (matching the existing metadata embedding approach). Each icon card displays two buttons on hover:
copycodicon to copy the icon name to clipboardfile-codecodicon to copy the SVG markup to clipboardOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.