ui: replace emojis with icons#80
Conversation
📝 WalkthroughWalkthroughThis PR introduces an icon system using the lucide library. A new Changes
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/settings/FirstTimeSetup.js (1)
1076-1078:⚠️ Potential issue | 🟡 MinorTor test button loses its icon after first test cycle.
On Line 1076,
originalTextis captured viatextContent, and Line 1121 restores withtextContent; this strips the SVG icon from the original label.Also applies to: 1121-1122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/FirstTimeSetup.js` around lines 1076 - 1078, The code captures and restores the button label using textContent, which strips the SVG icon; change the capture and restore to use innerHTML so the original HTML (including the SVG) is preserved — replace uses of originalText = btn.textContent and btn.textContent = ... restoration with innerHTML equivalents (keep the temporary "Testing..." state and disabled toggles on btn unchanged) so the SVG icon is retained when restoring the label for the Tor test button (references: btn and originalText in FirstTimeSetup.js).src/components/send/Send.js (1)
649-652:⚠️ Potential issue | 🟡 MinorSign/Broadcast buttons lose their icons after first action.
On Line 649 and Line 691,
textContentis cached from iconized buttons; restoring withtextContent(Line 679/Line 762) removes SVG markup permanently for that button state.♻️ Proposed fix
- const originalText = signBtn.textContent; + const originalHtml = signBtn.innerHTML; @@ - signBtn.textContent = originalText; + signBtn.innerHTML = originalHtml;- const originalText = broadcastBtn.textContent; + const originalHtml = broadcastBtn.innerHTML; @@ - broadcastBtn.textContent = originalText; + broadcastBtn.innerHTML = originalHtml;Also applies to: 679-680, 691-694, 762-763
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/send/Send.js` around lines 649 - 652, The code caches and restores button text using signBtn.textContent (and similarly broadcastBtn.textContent), which strips SVG/icon markup; change the caching/restoring to use signBtn.innerHTML (and broadcastBtn.innerHTML) so the original HTML (including SVG icons) is preserved when disabling/enabling and when setting "Signing..."/"Broadcasting..." states; update all spots that assign or restore the cached value (where originalText/originalBroadcastText are set and later restored) to use innerHTML instead of textContent.src/components/market/Market.js (1)
17-37: 🧹 Nitpick | 🔵 TrivialReplace the remaining Unified glyph with an icon helper for consistency.
Line 27 still uses a raw glyph (
'◈') while the other protocol badges use SVG helpers. This keeps one font-dependent path in the same UI area.♻️ Proposed refactor
case 'Unified': return { label: 'Unified', - icon: '◈', + icon: icons.link(14), classes: 'bg-emerald-500/20 text-emerald-400', };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/market/Market.js` around lines 17 - 37, The Unified case in the protocol switch returns a raw glyph ('◈') instead of using the SVG icon helpers used by Taproot and Legacy; update the Unified branch inside the switch (case 'Unified') to use the icons helper (e.g., icons.diamond(14) or the appropriate existing icon helper) and keep the same label and classes, ensuring the import/usage matches how icons.zap and icons.lock are used so the Unified badge is rendered via the SVG helper rather than a font glyph.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/settings/FirstTimeSetup.js`:
- Around line 811-814: The code currently injects raw error text into the DOM
via errorDiv.innerHTML (using icons.xCircle(...) and ${message}) which can
render untrusted markup; locate the error rendering in FirstTimeSetup.js where
errorDiv.innerHTML is used and replace it with DOM-safe construction: create the
container element, insert the icon in a safe way (e.g., create an icon
element/node or sanitize/parse only the known SVG string from icons.xCircle into
a DOM node), then create a separate span/div for the message and set its
textContent to message (do not interpolate message into innerHTML). Apply the
same change to the other occurrence around lines 1059–1062 so all user-facing
errors use textContent for the message and only insert trusted markup for icons
via DOM nodes.
In `@src/components/settings/Settings.js`:
- Around line 645-649: The copy-feedback code currently saves btn.textContent
which strips the SVG icon; change it to cache and restore the button's innerHTML
instead (e.g., replace originalText = btn.textContent with originalHTML =
btn.innerHTML and restore btn.innerHTML = originalHTML inside the setTimeout) in
the copy handler where icons.check(14, 'mr-1') + ' Copied!' is used; make the
same innerHTML-based change for the second occurrence of this pattern (the block
around the btn/textContent restore at the later spot).
In `@src/components/swap/Coinswap.js`:
- Around line 185-186: updateHopStatus currently writes SVG markup to an element
using textContent, so SVG strings passed from callers like icons.check(...)
render as raw text; change updateHopStatus to assign the markup to
element.innerHTML (or otherwise insert the SVG markup as HTML) instead of
textContent so the icons render correctly, and verify callers that pass SVG
strings (e.g., the places invoking icons.check(...)) keep passing their markup
unchanged.
In `@src/js/icons.js`:
- Around line 63-70: The toSvg function returns inline SVGs that should be
hidden from assistive tech by default; modify toSvg(iconData, size, cls) so the
generated <svg> includes accessibility defaults for decorative icons by adding
aria-hidden="true" and focusable="false" to the returned SVG string (keep
existing class handling and other attributes intact) so assistive technologies
won't announce these paired-with-text icons.
- Around line 14-51: The import of Lucide icons currently uses a fragile
relative path ("../../node_modules/lucide/dist/esm/lucide.js"); update the
module specifier to import from the package entrypoint (e.g., "lucide") instead.
Locate the import statement that brings in Check, CheckCircle, XCircle,
AlertTriangle, RefreshCw, Loader, ArrowDownCircle, ArrowUpCircle, Package, Save,
ExternalLink, Zap, Copy, Search, Lock, Key, KeyRound, ClipboardCopy, Info,
Timer, Link, Handshake, Receipt, FileText, CircleDollarSign, ShieldCheck,
Recycle, Globe, Inbox, Radio, Hourglass, FolderOpen, Folder, Lightbulb,
PlusCircle, PauseCircle and replace the relative node_modules path with the
package specifier so bundlers and dependency layouts resolve the public API
correctly.
---
Outside diff comments:
In `@src/components/market/Market.js`:
- Around line 17-37: The Unified case in the protocol switch returns a raw glyph
('◈') instead of using the SVG icon helpers used by Taproot and Legacy; update
the Unified branch inside the switch (case 'Unified') to use the icons helper
(e.g., icons.diamond(14) or the appropriate existing icon helper) and keep the
same label and classes, ensuring the import/usage matches how icons.zap and
icons.lock are used so the Unified badge is rendered via the SVG helper rather
than a font glyph.
In `@src/components/send/Send.js`:
- Around line 649-652: The code caches and restores button text using
signBtn.textContent (and similarly broadcastBtn.textContent), which strips
SVG/icon markup; change the caching/restoring to use signBtn.innerHTML (and
broadcastBtn.innerHTML) so the original HTML (including SVG icons) is preserved
when disabling/enabling and when setting "Signing..."/"Broadcasting..." states;
update all spots that assign or restore the cached value (where
originalText/originalBroadcastText are set and later restored) to use innerHTML
instead of textContent.
In `@src/components/settings/FirstTimeSetup.js`:
- Around line 1076-1078: The code captures and restores the button label using
textContent, which strips the SVG icon; change the capture and restore to use
innerHTML so the original HTML (including the SVG) is preserved — replace uses
of originalText = btn.textContent and btn.textContent = ... restoration with
innerHTML equivalents (keep the temporary "Testing..." state and disabled
toggles on btn unchanged) so the SVG icon is retained when restoring the label
for the Tor test button (references: btn and originalText in FirstTimeSetup.js).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a0edc0d-a599-499e-a96f-e1f706097da9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
package.jsonsrc/components/connection/ConnectionStatus.jssrc/components/log/Log.jssrc/components/market/Market.jssrc/components/receive/AddressList.jssrc/components/receive/Receive.jssrc/components/send/Send.jssrc/components/settings/FirstTimeSetup.jssrc/components/settings/Settings.jssrc/components/swap/Coinswap.jssrc/components/swap/Swap.jssrc/components/swap/SwapHistory.jssrc/components/swap/SwapReport.jssrc/components/taker/TakerInitialization.jssrc/components/wallet/TransactionsList.jssrc/js/icons.jssrc/styles/output.css
| errorDiv.innerHTML = ` | ||
| <div class="flex items-center"> | ||
| <span class="text-sm text-red-400">❌ ${message}</span> | ||
| <span class="text-sm text-red-400">${icons.xCircle(14, 'mr-1')} ${message}</span> | ||
| </div> |
There was a problem hiding this comment.
Do not inject raw error messages via innerHTML.
At Line 813 and Line 1061, runtime error text is directly interpolated into HTML. If an error string contains markup, it will be rendered. Render the message with textContent on a dedicated element instead.
🛡️ Proposed fix pattern
- errorDiv.innerHTML = `
- <div class="flex items-center">
- <span class="text-sm text-red-400">${icons.xCircle(14, 'mr-1')} ${message}</span>
- </div>
- `;
+ errorDiv.innerHTML = `
+ <div class="flex items-center">
+ ${icons.xCircle(14, 'mr-1')}
+ <span id="setup-error-text" class="text-sm text-red-400"></span>
+ </div>
+ `;
+ errorDiv.querySelector('#setup-error-text').textContent = message;As per coding guidelines "Validate all user inputs".
Also applies to: 1059-1062
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/settings/FirstTimeSetup.js` around lines 811 - 814, The code
currently injects raw error text into the DOM via errorDiv.innerHTML (using
icons.xCircle(...) and ${message}) which can render untrusted markup; locate the
error rendering in FirstTimeSetup.js where errorDiv.innerHTML is used and
replace it with DOM-safe construction: create the container element, insert the
icon in a safe way (e.g., create an icon element/node or sanitize/parse only the
known SVG string from icons.xCircle into a DOM node), then create a separate
span/div for the message and set its textContent to message (do not interpolate
message into innerHTML). Apply the same change to the other occurrence around
lines 1059–1062 so all user-facing errors use textContent for the message and
only insert trusted markup for icons via DOM nodes.
| const originalText = btn.textContent; | ||
| btn.textContent = '✓ Copied!'; | ||
| btn.innerHTML = icons.check(14, 'mr-1') + ' Copied!'; | ||
| setTimeout(() => { | ||
| btn.textContent = originalText; | ||
| }, 2000); |
There was a problem hiding this comment.
Restore button HTML, not plain text, after copy feedback.
originalText = btn.textContent (Line 645 / Line 707) drops the SVG icon, so after timeout the button no longer shows the icon label. Cache/restore innerHTML instead.
♻️ Proposed fix
- const originalText = btn.textContent;
+ const originalHtml = btn.innerHTML;
btn.innerHTML = icons.check(14, 'mr-1') + ' Copied!';
setTimeout(() => {
- btn.textContent = originalText;
+ btn.innerHTML = originalHtml;
}, 2000);- const originalText = btn.textContent;
+ const originalHtml = btn.innerHTML;
btn.innerHTML = icons.check(14, 'mr-1') + ' Copied!';
setTimeout(() => {
- btn.textContent = originalText;
+ btn.innerHTML = originalHtml;
}, 2000);Also applies to: 707-711
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/settings/Settings.js` around lines 645 - 649, The
copy-feedback code currently saves btn.textContent which strips the SVG icon;
change it to cache and restore the button's innerHTML instead (e.g., replace
originalText = btn.textContent with originalHTML = btn.innerHTML and restore
btn.innerHTML = originalHTML inside the setTimeout) in the copy handler where
icons.check(14, 'mr-1') + ' Copied!' is used; make the same innerHTML-based
change for the second occurrence of this pattern (the block around the
btn/textContent restore at the later spot).
| updateHopStatus(i, `${icons.check(14, 'mr-1')} Complete`, 'green'); | ||
| } |
There was a problem hiding this comment.
Hop status icons won’t render with the current sink.
These callsites now pass SVG markup into updateHopStatus(...), but updateHopStatus writes to textContent (Line 150). Result: raw SVG text appears instead of icons.
♻️ Proposed fix
- hopStatus.textContent = statusText;
+ hopStatus.innerHTML = statusText;Also applies to: 321-322, 346-347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/swap/Coinswap.js` around lines 185 - 186, updateHopStatus
currently writes SVG markup to an element using textContent, so SVG strings
passed from callers like icons.check(...) render as raw text; change
updateHopStatus to assign the markup to element.innerHTML (or otherwise insert
the SVG markup as HTML) instead of textContent so the icons render correctly,
and verify callers that pass SVG strings (e.g., the places invoking
icons.check(...)) keep passing their markup unchanged.
| import { | ||
| Check, | ||
| CheckCircle, | ||
| XCircle, | ||
| AlertTriangle, | ||
| RefreshCw, | ||
| Loader, | ||
| ArrowDownCircle, | ||
| ArrowUpCircle, | ||
| Package, | ||
| Save, | ||
| ExternalLink, | ||
| Zap, | ||
| Copy, | ||
| Search, | ||
| Lock, | ||
| Key, | ||
| KeyRound, | ||
| ClipboardCopy, | ||
| Info, | ||
| Timer, | ||
| Link, | ||
| Handshake, | ||
| Receipt, | ||
| FileText, | ||
| CircleDollarSign, | ||
| ShieldCheck, | ||
| Recycle, | ||
| Globe, | ||
| Inbox, | ||
| Radio, | ||
| Hourglass, | ||
| FolderOpen, | ||
| Folder, | ||
| Lightbulb, | ||
| PlusCircle, | ||
| PauseCircle, | ||
| } from '../../node_modules/lucide/dist/esm/lucide.js'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the officially supported ES module import path for the lucidenpm package, and is importing fromnode_modules/lucide/dist/esm/lucide.js considered a stable/public API?
💡 Result:
The officially supported ES module import path for the lucide npm package is import { createIcons, Menu } from 'lucide';. Icons and functions are imported directly from the package specifier 'lucide', which resolves to the ESM build via standard Node.js module resolution (likely configured in package.json "module" or "exports" fields). Official documentation consistently shows this usage for Vanilla JS/ESM environments, emphasizing tree-shaking by importing specific icons. Importing from node_modules/lucide/dist/esm/lucide.js is not considered a stable/public API. This is an internal dist path exposed due to the package structure (confirmed by npm files listing dist/esm/), but docs never recommend direct dist imports. Similar paths like dist/esm/lucide-react.js appear in issues as internal resolutions, and direct dist usage (e.g., dynamicIconImports.js) leads to errors or requires workarounds, indicating it's unsupported for public consumption. Use the package specifier for stability.
Citations:
- 1: https://www.npmjs.com/package/lucide
- 2: https://lucide.dev/guide/lucide/getting-started
- 3: https://github.com/lucide-icons/lucide/tree/main/packages/lucide
- 4: https://v0.lucide.dev/guide/packages/lucide
- 5: Must use import to load ES Module lucide-icons/lucide#2734
- 6: Generate ESM/CJS variants of
dynamicIconImportslucide-icons/lucide#2437
Replace the relative node_modules import path with the package specifier.
Line 14-51 imports lucide via a relative filesystem path (../../node_modules/lucide/dist/esm/lucide.js) instead of the official package entrypoint. This is fragile and unsupported—the dist/ directory is an internal implementation detail, not a public API. Direct dist imports can break across bundler configurations and when dependency layouts change.
🔧 Proposed fix
-} from '../../node_modules/lucide/dist/esm/lucide.js';
+} from 'lucide';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { | |
| Check, | |
| CheckCircle, | |
| XCircle, | |
| AlertTriangle, | |
| RefreshCw, | |
| Loader, | |
| ArrowDownCircle, | |
| ArrowUpCircle, | |
| Package, | |
| Save, | |
| ExternalLink, | |
| Zap, | |
| Copy, | |
| Search, | |
| Lock, | |
| Key, | |
| KeyRound, | |
| ClipboardCopy, | |
| Info, | |
| Timer, | |
| Link, | |
| Handshake, | |
| Receipt, | |
| FileText, | |
| CircleDollarSign, | |
| ShieldCheck, | |
| Recycle, | |
| Globe, | |
| Inbox, | |
| Radio, | |
| Hourglass, | |
| FolderOpen, | |
| Folder, | |
| Lightbulb, | |
| PlusCircle, | |
| PauseCircle, | |
| } from '../../node_modules/lucide/dist/esm/lucide.js'; | |
| import { | |
| Check, | |
| CheckCircle, | |
| XCircle, | |
| AlertTriangle, | |
| RefreshCw, | |
| Loader, | |
| ArrowDownCircle, | |
| ArrowUpCircle, | |
| Package, | |
| Save, | |
| ExternalLink, | |
| Zap, | |
| Copy, | |
| Search, | |
| Lock, | |
| Key, | |
| KeyRound, | |
| ClipboardCopy, | |
| Info, | |
| Timer, | |
| Link, | |
| Handshake, | |
| Receipt, | |
| FileText, | |
| CircleDollarSign, | |
| ShieldCheck, | |
| Recycle, | |
| Globe, | |
| Inbox, | |
| Radio, | |
| Hourglass, | |
| FolderOpen, | |
| Folder, | |
| Lightbulb, | |
| PlusCircle, | |
| PauseCircle, | |
| } from 'lucide'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/js/icons.js` around lines 14 - 51, The import of Lucide icons currently
uses a fragile relative path ("../../node_modules/lucide/dist/esm/lucide.js");
update the module specifier to import from the package entrypoint (e.g.,
"lucide") instead. Locate the import statement that brings in Check,
CheckCircle, XCircle, AlertTriangle, RefreshCw, Loader, ArrowDownCircle,
ArrowUpCircle, Package, Save, ExternalLink, Zap, Copy, Search, Lock, Key,
KeyRound, ClipboardCopy, Info, Timer, Link, Handshake, Receipt, FileText,
CircleDollarSign, ShieldCheck, Recycle, Globe, Inbox, Radio, Hourglass,
FolderOpen, Folder, Lightbulb, PlusCircle, PauseCircle and replace the relative
node_modules path with the package specifier so bundlers and dependency layouts
resolve the public API correctly.
| function toSvg(iconData, size, cls) { | ||
| const children = iconData.map(nodeToString).join(''); | ||
| const baseClass = 'inline-block align-middle flex-shrink-0'; | ||
| const classAttr = cls | ||
| ? `class="${baseClass} ${cls}"` | ||
| : `class="${baseClass}"`; | ||
| return `<svg xmlns="http://www.w3.org/2000/svg" width="${size}" height="${size}" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" ${classAttr}>${children}</svg>`; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add decorative-icon accessibility defaults to generated SVG.
Since these icons are primarily paired with visible text labels, defaulting them to hidden for assistive tech avoids unnecessary screen-reader noise.
♿ Proposed refactor
- return `<svg xmlns="http://www.w3.org/2000/svg" width="${size}" height="${size}" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" ${classAttr}>${children}</svg>`;
+ return `<svg xmlns="http://www.w3.org/2000/svg" width="${size}" height="${size}" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true" focusable="false" ${classAttr}>${children}</svg>`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function toSvg(iconData, size, cls) { | |
| const children = iconData.map(nodeToString).join(''); | |
| const baseClass = 'inline-block align-middle flex-shrink-0'; | |
| const classAttr = cls | |
| ? `class="${baseClass} ${cls}"` | |
| : `class="${baseClass}"`; | |
| return `<svg xmlns="http://www.w3.org/2000/svg" width="${size}" height="${size}" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" ${classAttr}>${children}</svg>`; | |
| } | |
| function toSvg(iconData, size, cls) { | |
| const children = iconData.map(nodeToString).join(''); | |
| const baseClass = 'inline-block align-middle flex-shrink-0'; | |
| const classAttr = cls | |
| ? `class="${baseClass} ${cls}"` | |
| : `class="${baseClass}"`; | |
| return `<svg xmlns="http://www.w3.org/2000/svg" width="${size}" height="${size}" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true" focusable="false" ${classAttr}>${children}</svg>`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/js/icons.js` around lines 63 - 70, The toSvg function returns inline SVGs
that should be hidden from assistive tech by default; modify toSvg(iconData,
size, cls) so the generated <svg> includes accessibility defaults for decorative
icons by adding aria-hidden="true" and focusable="false" to the returned SVG
string (keep existing class handling and other attributes intact) so assistive
technologies won't announce these paired-with-text icons.
2 participants
Replace emojis with icons to avoid font issues
Summary by CodeRabbit