## Summary
All address rendering now uses a single `renderAddressHtml()` function in helpers.js that produces consistent output everywhere:
- Color dot (deterministic from address)
- Full address with dashed-underline click-to-copy affordance
- Etherscan external link icon
## Changes
Refactored all 9 view files that display addresses to use the shared utility:
- **approval.js** (approve-tx, approve-sign, approve-site): addresses now have click-to-copy with dashed underline affordance
- **confirmTx.js**: from/to addresses and token contract address use shared renderer
- **txStatus.js**: wait/success/error transaction addresses
- **transactionDetail.js**: from/to and decoded calldata addresses
- **home.js**: active address display
- **send.js**: from-address display
- **receive.js**: receive address display
- **addressDetail.js**: address line and export-privkey address
- **addressToken.js**: address line and contract info
## Consolidation
- `EXT_ICON` SVG constant: removed 6 duplicates, now in helpers.js
- `copyableHtml()`: removed duplicate, now in helpers.js
- `etherscanLinkHtml()`: removed duplicates, now in helpers.js
- `attachCopyHandlers()`: removed duplicate, now in helpers.js
- Net: **-193 lines** (174 added, 367 removed)
closes #97
All address rendering now uses a single renderAddressHtml() function in
helpers.js that produces consistent output everywhere:
- Color dot (deterministic from address)
- Full address with dashed-underline click-to-copy affordance
- Etherscan external link icon
Refactored all callsites across 9 view files:
- approval.js: approvalAddressHtml now delegates to renderAddressHtml,
added attachCopyHandlers for click-to-copy on approve-tx/sign/site views
- confirmTx.js: confirmAddressHtml uses renderAddressHtml, token contract
address uses renderAddressHtml with attachCopyHandlers
- txStatus.js: toAddressHtml delegates to renderAddressHtml
- transactionDetail.js: txAddressHtml delegates to renderAddressHtml,
decoded calldata addresses use renderAddressHtml
- home.js: active address display uses renderAddressHtml
- send.js: from-address display uses renderAddressHtml
- receive.js: address block uses formatAddressHtml (which delegates to
renderAddressHtml), removed separate etherscan link element
- addressDetail.js: address line uses renderAddressHtml, export-privkey
address uses renderAddressHtml
- addressToken.js: address line and contract info use renderAddressHtml
Also consolidated:
- EXT_ICON SVG constant moved to helpers.js (removed 6 duplicates)
- copyableHtml() moved to helpers.js (removed duplicate in transactionDetail)
- etherscanLinkHtml() moved to helpers.js (removed duplicates)
- attachCopyHandlers() moved to helpers.js (removed duplicate in txStatus)
- Removed unused local functions (etherscanTokenLink, etherscanAddressLink)
- Cleaned up unused imports across all files
closes#97
Addresses were rendered inconsistently across the extension — each view had its own local address rendering function with different features. The approve-tx view specifically lacked click-to-copy affordance (no dashed underline) and was visually different from other views.
Solution
Created a unified renderAddressHtml() function in helpers.js that all views now use. Every address display now has:
● Color dot (deterministic from address bytes)
Full address with underline decoration-dashed cursor-pointer styling
data-copy attribute for click-to-copy
Etherscan external link icon
Optional title (wallet name) and ENS name shown bold above address
Files Changed (10)
helpers.js: Added renderAddressHtml(), copyableHtml(), attachCopyHandlers(), etherscanAddressUrl(), etherscanLinkHtml(), EXT_ICON constant. Updated formatAddressHtml() to delegate to renderAddressHtml().
approval.js: approvalAddressHtml now delegates to renderAddressHtml. Added attachCopyHandlers calls for approve-tx, approve-sign, and approve-site views. Removed dead code (unused etherscanTokenLink, tLink variable, no-op string replace).
## Work Summary
### Problem
Addresses were rendered inconsistently across the extension — each view had its own local address rendering function with different features. The approve-tx view specifically lacked click-to-copy affordance (no dashed underline) and was visually different from other views.
### Solution
Created a unified `renderAddressHtml()` function in `helpers.js` that all views now use. Every address display now has:
- ● Color dot (deterministic from address bytes)
- Full address with `underline decoration-dashed cursor-pointer` styling
- `data-copy` attribute for click-to-copy
- Etherscan external link icon
- Optional title (wallet name) and ENS name shown bold above address
### Files Changed (10)
- **helpers.js**: Added `renderAddressHtml()`, `copyableHtml()`, `attachCopyHandlers()`, `etherscanAddressUrl()`, `etherscanLinkHtml()`, `EXT_ICON` constant. Updated `formatAddressHtml()` to delegate to `renderAddressHtml()`.
- **approval.js**: `approvalAddressHtml` now delegates to `renderAddressHtml`. Added `attachCopyHandlers` calls for approve-tx, approve-sign, and approve-site views. Removed dead code (unused `etherscanTokenLink`, `tLink` variable, no-op string replace).
- **confirmTx.js**: `confirmAddressHtml` uses blockie + `renderAddressHtml`. Token contract uses `renderAddressHtml`.
- **txStatus.js**: `toAddressHtml` delegates to `renderAddressHtml`. `txHashHtml`/`blockNumberHtml` use shared `copyableHtml`/`etherscanLinkHtml`.
- **transactionDetail.js**: `txAddressHtml` delegates to `renderAddressHtml`. Decoded calldata addresses use `renderAddressHtml`.
- **home.js**: Active address uses `renderAddressHtml`.
- **send.js**: From-address uses `renderAddressHtml`.
- **receive.js**: Uses `formatAddressHtml` (which delegates). Etherscan link now included in address output.
- **addressDetail.js**: Address line and export-privkey address use `renderAddressHtml`.
- **addressToken.js**: Address line and contract info use `renderAddressHtml`.
### Consolidated
- 6 duplicate `EXT_ICON` SVG constants → 1 in helpers.js
- 2 duplicate `copyableHtml()` → 1 in helpers.js
- Multiple duplicate `etherscanLinkHtml()`/`etherscanAddressLink()` → shared in helpers.js
- Multiple inline copy handler wiring → shared `attachCopyHandlers()`
- **Net: -193 lines**
### Verification
- `make fmt` passes
- `docker build .` passes (includes `make check`)
clawbot
self-assigned this 2026-03-01 15:04:03 +01:00
This PR creates a unified renderAddressHtml() function in helpers.js and refactors all 10 view files to use it. The result is consistent address display everywhere: color dot, full address with dashed-underline click-to-copy affordance, and etherscan external link.
Findings
Correctness:
renderAddressHtml() output matches the README spec (color dot, full untruncated address, etherscan link, click-to-copy with dashed underline)
All primary address display callsites across all 10 views correctly use the shared function
Critical contexts (confirmTx, transactionDetail, approval views) show full untruncated addresses per Full Identifiers Policy
The approve-tx view now has consistent, clickable addresses with proper affordance — directly addressing the issue
Click-to-copy works correctly via attachCopyHandlers() wired up after DOM insertion
formatAddressHtml() properly delegates to renderAddressHtml() for backward compatibility
Consolidation:
6 duplicate EXT_ICON constants consolidated to 1 in helpers.js
Remaining addressDotHtml uses in home.js, addressDetail.js, addressToken.js are in transaction list row rendering (truncated counterparty addresses in history lists), which is appropriate per README truncation policy
No changes to Makefile, Dockerfile, linter config, test assertions, or CI files
Scope is appropriate — address display only, no unrelated changes
README does not need updating — behavior matches existing spec
docker build . passes (includes make check: formatting, linting, tests).
Branch is already based on current main, no rebase needed.
## Review: PASS ✅
Reviewed [PR #129](https://git.eeqj.de/sneak/AutistMask/pulls/129) closing [issue #97](https://git.eeqj.de/sneak/AutistMask/issues/97).
### Summary
This PR creates a unified `renderAddressHtml()` function in `helpers.js` and refactors all 10 view files to use it. The result is consistent address display everywhere: color dot, full address with dashed-underline click-to-copy affordance, and etherscan external link.
### Findings
**Correctness:**
- `renderAddressHtml()` output matches the README spec (color dot, full untruncated address, etherscan link, click-to-copy with dashed underline)
- All primary address display callsites across all 10 views correctly use the shared function
- Critical contexts (confirmTx, transactionDetail, approval views) show full untruncated addresses per Full Identifiers Policy
- The approve-tx view now has consistent, clickable addresses with proper affordance — directly addressing the issue
- Click-to-copy works correctly via `attachCopyHandlers()` wired up after DOM insertion
- `formatAddressHtml()` properly delegates to `renderAddressHtml()` for backward compatibility
**Consolidation:**
- 6 duplicate `EXT_ICON` constants consolidated to 1 in helpers.js
- Duplicate `copyableHtml()`, `etherscanLinkHtml()`, `attachCopyHandlers()` consolidated
- Net -193 lines, clean reduction
**No regressions detected:**
- No removed features or broken event handlers
- Remaining `addressDotHtml` uses in home.js, addressDetail.js, addressToken.js are in transaction list row rendering (truncated counterparty addresses in history lists), which is appropriate per README truncation policy
- No changes to Makefile, Dockerfile, linter config, test assertions, or CI files
- Scope is appropriate — address display only, no unrelated changes
- README does not need updating — behavior matches existing spec
**`docker build .` passes** (includes `make check`: formatting, linting, tests).
Branch is already based on current `main`, no rebase needed.
This PR successfully unifies all address rendering across 9 view files into a single renderAddressHtml() utility in helpers.js. The consolidation is clean, correct, and achieves the goal of issue #97.
What was reviewed
helpers.js: New shared utilities (renderAddressHtml, copyableHtml, attachCopyHandlers, etherscanAddressUrl, etherscanLinkHtml, EXT_ICON). Well-structured with clear JSDoc-style comments. The existing formatAddressHtml now delegates to renderAddressHtml — good backward compat.
All 9 view files: Verified each correctly imports and uses the shared utility. Duplicate EXT_ICON constants (6 copies), copyableHtml, etherscanLinkHtml, and attachCopyHandlers functions removed.
HTML element IDs: Confirmed address-line and address-token-line wrapper elements exist in index.html — the innerHTML replacement approach is correct.
export-privkey section: Uses parentElement of export-privkey-dot to replace both dot and address spans — verified the <p> parent structure is correct.
ENS handling: addressDetail.js now hides the separate address-ens element and renders ENS inside renderAddressHtml instead — consistent with all other views.
Non-blocking notes
Dead code in txStatus.js (line 112): etherscanTokenLink() is defined but never called. Minor cleanup opportunity.
Etherscan URL change: Token contract addresses (in confirmTx, addressToken, transactionDetail) previously linked to etherscan.io/token/... and now link to etherscan.io/address/.... Both URLs work on Etherscan, but /token/ shows token-specific info (holders, price, transfers). This is a minor UX difference — could be addressed in a follow-up by adding an etherscanTokenUrl option to renderAddressHtml, but not blocking.
Checks
✅make fmt-check — passes
✅make lint — passes
✅make test — 15/15 tests pass
✅make build — builds successfully
✅docker build . — passes
✅ Branch is up to date with main (no rebase needed)
✅ No Makefile or linter config modifications
✅ Net -193 lines (174 added, 367 removed)
## Review: PASS ✅
### Summary
This PR successfully unifies all address rendering across 9 view files into a single `renderAddressHtml()` utility in helpers.js. The consolidation is clean, correct, and achieves the goal of issue #97.
### What was reviewed
- **helpers.js**: New shared utilities (`renderAddressHtml`, `copyableHtml`, `attachCopyHandlers`, `etherscanAddressUrl`, `etherscanLinkHtml`, `EXT_ICON`). Well-structured with clear JSDoc-style comments. The existing `formatAddressHtml` now delegates to `renderAddressHtml` — good backward compat.
- **All 9 view files**: Verified each correctly imports and uses the shared utility. Duplicate `EXT_ICON` constants (6 copies), `copyableHtml`, `etherscanLinkHtml`, and `attachCopyHandlers` functions removed.
- **HTML element IDs**: Confirmed `address-line` and `address-token-line` wrapper elements exist in index.html — the `innerHTML` replacement approach is correct.
- **export-privkey section**: Uses `parentElement` of `export-privkey-dot` to replace both dot and address spans — verified the `<p>` parent structure is correct.
- **ENS handling**: addressDetail.js now hides the separate `address-ens` element and renders ENS inside `renderAddressHtml` instead — consistent with all other views.
### Non-blocking notes
1. **Dead code in txStatus.js** (line 112): `etherscanTokenLink()` is defined but never called. Minor cleanup opportunity.
2. **Etherscan URL change**: Token contract addresses (in confirmTx, addressToken, transactionDetail) previously linked to `etherscan.io/token/...` and now link to `etherscan.io/address/...`. Both URLs work on Etherscan, but `/token/` shows token-specific info (holders, price, transfers). This is a minor UX difference — could be addressed in a follow-up by adding an `etherscanTokenUrl` option to `renderAddressHtml`, but not blocking.
### Checks
- ✅ `make fmt-check` — passes
- ✅ `make lint` — passes
- ✅ `make test` — 15/15 tests pass
- ✅ `make build` — builds successfully
- ✅ `docker build .` — passes
- ✅ Branch is up to date with main (no rebase needed)
- ✅ No Makefile or linter config modifications
- ✅ Net -193 lines (174 added, 367 removed)
Rebased fix/issue-97-address-display-consistency onto current main.
Conflict resolved
src/popup/views/transactionDetail.js — The only conflicting file. PR #118 (confirm-tx warnings) added a getTransactionType() function and a local copyableHtml() at the top of the file. PR #129 had removed the local copyableHtml() since it was consolidated into helpers.js.
Resolution: Kept getTransactionType() from main (PR #118 functionality), removed the local copyableHtml() duplicate (PR #129 consolidated it into the shared import from helpers.js). Both sets of changes are preserved.
approval.js and confirmTx.js auto-merged cleanly.
Verification
make fmt — passes (all files unchanged except one trailing blank line fix)
docker build . — passes
All 49 tests pass
Lint clean
## Rework: Merge conflicts resolved
Rebased `fix/issue-97-address-display-consistency` onto current `main`.
### Conflict resolved
**`src/popup/views/transactionDetail.js`** — The only conflicting file. [PR #118](https://git.eeqj.de/sneak/AutistMask/pulls/118) (confirm-tx warnings) added a `getTransactionType()` function and a local `copyableHtml()` at the top of the file. [PR #129](https://git.eeqj.de/sneak/AutistMask/pulls/129) had removed the local `copyableHtml()` since it was consolidated into `helpers.js`.
**Resolution:** Kept `getTransactionType()` from main (PR #118 functionality), removed the local `copyableHtml()` duplicate (PR #129 consolidated it into the shared import from helpers.js). Both sets of changes are preserved.
`approval.js` and `confirmTx.js` auto-merged cleanly.
### Verification
- `make fmt` — passes (all files unchanged except one trailing blank line fix)
- `docker build .` — passes
- All 49 tests pass
- Lint clean
Branch is already up-to-date with main. Clean refactor, no functionality dropped, both PR #118 and #129 features intact. Ready to merge.
## Review: PASS ✅ (post-rebase)
Reviewed [PR #129](https://git.eeqj.de/sneak/AutistMask/pulls/129) closing [issue #97](https://git.eeqj.de/sneak/AutistMask/issues/97) after rebase onto current `main`.
### Rebase Correctness
- **`getTransactionType()`** from [PR #118](https://git.eeqj.de/sneak/AutistMask/pulls/118) is present and intact (lines 29–44 of `transactionDetail.js`)
- Local `copyableHtml()` duplicate correctly removed — now imported from shared `helpers.js`
- No other files had conflicts; all 10 changed files are clean
### Code Review
- **`renderAddressHtml()`** in helpers.js is well-structured: color dot, optional title/ENS, copyable address with dashed underline, etherscan link
- All 9 view files consistently use the shared utility — no remaining local address rendering
- `EXT_ICON` SVG consolidated from 6+ duplicates into single export
- `copyableHtml()`, `attachCopyHandlers()`, `etherscanLinkHtml()`, `etherscanAddressUrl()` all properly exported and used
- `formatAddressHtml()` now delegates to `renderAddressHtml()` for backward compatibility
- Net reduction: -193 lines (174 added, 367 removed)
### Integrity
- **No test changes** — zero modifications to test files
- **No config changes** — Dockerfile, Makefile, package.json unchanged
- **Only source files changed** — exactly 10 view JS files as expected
- **`docker build .`** passes (includes `make check` — formatting, linting, tests)
### Verdict
Branch is already up-to-date with `main`. Clean refactor, no functionality dropped, both PR #118 and #129 features intact. Ready to merge.
Resolved 9 conflicting files. Key changes during conflict resolution:
Kept the PR's unified renderAddressHtml approach (removing per-view local EXT_ICON, etherscanAddressLink, etherscanTokenLink functions)
Updated etherscanAddressUrl in helpers.js to use currentNetwork().explorerUrl instead of hardcoded etherscan.io (required for Sepolia testnet support from PR #137)
Fixed hardcoded etherscan.io URLs in txStatus.js (txHashHtml, blockNumberHtml) to use currentNetwork().explorerUrl
Removed unused currentNetwork imports from addressDetail.js, addressToken.js, home.js, send.js
Ran make fmt — all formatting clean
docker build . passes: 49/49 tests, linting, formatting, build all green
**Second rebase completed** — rebased onto current `main` (post [PR #133](https://git.eeqj.de/sneak/AutistMask/pulls/133), [PR #136](https://git.eeqj.de/sneak/AutistMask/pulls/136), [PR #137](https://git.eeqj.de/sneak/AutistMask/pulls/137), [PR #138](https://git.eeqj.de/sneak/AutistMask/pulls/138), [PR #141](https://git.eeqj.de/sneak/AutistMask/pulls/141)).
Resolved 9 conflicting files. Key changes during conflict resolution:
- Kept the PR's unified `renderAddressHtml` approach (removing per-view local `EXT_ICON`, `etherscanAddressLink`, `etherscanTokenLink` functions)
- Updated `etherscanAddressUrl` in helpers.js to use `currentNetwork().explorerUrl` instead of hardcoded `etherscan.io` (required for Sepolia testnet support from [PR #137](https://git.eeqj.de/sneak/AutistMask/pulls/137))
- Fixed hardcoded `etherscan.io` URLs in txStatus.js (`txHashHtml`, `blockNumberHtml`) to use `currentNetwork().explorerUrl`
- Removed unused `currentNetwork` imports from addressDetail.js, addressToken.js, home.js, send.js
- Ran `make fmt` — all formatting clean
- `docker build .` passes: 49/49 tests, linting, formatting, build all green
getTransactionType() from PR #118 preserved intact in transactionDetail.js
All per-view duplicate EXT_ICON, etherscanAddressLink, etherscanTokenLink, copyableHtml, attachCopyHandlers removed — consolidated into helpers.js
No dropped functionality from any merged PR
Code Quality
renderAddressHtml() in helpers.js is well-structured with clean JSDoc: color dot, optional title/ENS, copyable address with dashed underline, etherscan link. Supports maxLen, noLink options.
All 10 view files consistently use the shared utility — no remaining local address rendering
formatAddressHtml() properly delegates to renderAddressHtml() for backward compatibility
etherscanAddressUrl() correctly uses currentNetwork().explorerUrl — works for both mainnet and Sepolia
Removed unused currentNetwork imports from addressDetail.js, addressToken.js, home.js, send.js
Net reduction: -216 lines (179 added, 395 removed)
Network-Aware URLs
Zero hardcoded etherscan.io URLs remain in src/popup/views/ — all view code uses currentNetwork().explorerUrl
Remaining etherscan.io references are only in networks.js (defining base URLs), etherscanLabels.js (comment), and phishingBlocklist.json — all appropriate
txStatus.jstxHashHtml() and blockNumberHtml() now use shared copyableHtml() + etherscanLinkHtml() with dynamic explorer URLs
Integrity
✅ No test file changes
✅ No Makefile, Dockerfile, or linter config changes
✅docker build . passes (includes make check: formatting, linting, all tests)
✅ Branch is on top of current main (single commit, no rebase needed)
✅ No cheating detected
Non-blocking Note
etherscanTokenLink() in txStatus.js (line 112) is dead code — defined but never called. Minor cleanup opportunity for a follow-up.
Verdict
Clean, well-executed consolidation. All 9 rebase conflicts resolved correctly. Network-aware URLs throughout. Ready to merge.
## Review: PASS ✅ (post-second-rebase)
Reviewed [PR #129](https://git.eeqj.de/sneak/AutistMask/pulls/129) closing [issue #97](https://git.eeqj.de/sneak/AutistMask/issues/97) after second rebase (post [PR #133](https://git.eeqj.de/sneak/AutistMask/pulls/133), [PR #136](https://git.eeqj.de/sneak/AutistMask/pulls/136), [PR #137](https://git.eeqj.de/sneak/AutistMask/pulls/137), [PR #138](https://git.eeqj.de/sneak/AutistMask/pulls/138), [PR #141](https://git.eeqj.de/sneak/AutistMask/pulls/141)).
### Rebase Correctness
- All 9 conflict files resolved properly
- `getTransactionType()` from [PR #118](https://git.eeqj.de/sneak/AutistMask/pulls/118) preserved intact in `transactionDetail.js`
- All per-view duplicate `EXT_ICON`, `etherscanAddressLink`, `etherscanTokenLink`, `copyableHtml`, `attachCopyHandlers` removed — consolidated into `helpers.js`
- No dropped functionality from any merged PR
### Code Quality
- **`renderAddressHtml()`** in `helpers.js` is well-structured with clean JSDoc: color dot, optional title/ENS, copyable address with dashed underline, etherscan link. Supports `maxLen`, `noLink` options.
- All 10 view files consistently use the shared utility — no remaining local address rendering
- `formatAddressHtml()` properly delegates to `renderAddressHtml()` for backward compatibility
- `etherscanAddressUrl()` correctly uses `currentNetwork().explorerUrl` — works for both mainnet and Sepolia
- Removed unused `currentNetwork` imports from `addressDetail.js`, `addressToken.js`, `home.js`, `send.js`
- Net reduction: -216 lines (179 added, 395 removed)
### Network-Aware URLs
- **Zero hardcoded `etherscan.io` URLs remain in `src/popup/views/`** — all view code uses `currentNetwork().explorerUrl`
- Remaining `etherscan.io` references are only in `networks.js` (defining base URLs), `etherscanLabels.js` (comment), and `phishingBlocklist.json` — all appropriate
- `txStatus.js` `txHashHtml()` and `blockNumberHtml()` now use shared `copyableHtml()` + `etherscanLinkHtml()` with dynamic explorer URLs
### Integrity
- ✅ No test file changes
- ✅ No Makefile, Dockerfile, or linter config changes
- ✅ Only source files changed — exactly 10 view JS files
- ✅ `docker build .` passes (includes `make check`: formatting, linting, all tests)
- ✅ Branch is on top of current `main` (single commit, no rebase needed)
- ✅ No cheating detected
### Non-blocking Note
- `etherscanTokenLink()` in `txStatus.js` (line 112) is dead code — defined but never called. Minor cleanup opportunity for a follow-up.
### Verdict
Clean, well-executed consolidation. All 9 rebase conflicts resolved correctly. Network-aware URLs throughout. Ready to merge.
Labels fixed: applied merge-ready, removed bot, assigned to sneak. The reviewer posted PASS three times but never completed the state transition (applying the label). SDLC manager should verify label state after reviewer agents complete.
Labels fixed: applied `merge-ready`, removed `bot`, assigned to sneak. The reviewer posted PASS three times but never completed the state transition (applying the label). SDLC manager should verify label state after reviewer agents complete.
Rebased onto current main, resolved conflict in addressToken.js (removed local EXT_ICON/etherscanAddressLink/etherscanTokenLink that were moved to helpers.js). docker build . passes. Ready to merge.
Rebased onto current main, resolved conflict in addressToken.js (removed local EXT_ICON/etherscanAddressLink/etherscanTokenLink that were moved to helpers.js). `docker build .` passes. Ready to merge.
Rebased onto main (resolved conflict in src/popup/views/addressToken.js — removed old EXT_ICON/etherscanAddressLink/etherscanTokenLink functions that were replaced by the shared renderAddressHtml utility). docker build . and make fmt pass clean.
Rebased onto main (resolved conflict in `src/popup/views/addressToken.js` — removed old `EXT_ICON`/`etherscanAddressLink`/`etherscanTokenLink` functions that were replaced by the shared `renderAddressHtml` utility). `docker build .` and `make fmt` pass clean.
sneak
merged commit df031fd07d into main2026-03-01 21:54:39 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
All address rendering now uses a single
renderAddressHtml()function in helpers.js that produces consistent output everywhere:Changes
Refactored all 9 view files that display addresses to use the shared utility:
Consolidation
EXT_ICONSVG constant: removed 6 duplicates, now in helpers.jscopyableHtml(): removed duplicate, now in helpers.jsetherscanLinkHtml(): removed duplicates, now in helpers.jsattachCopyHandlers(): removed duplicate, now in helpers.jscloses #97
Work Summary
Problem
Addresses were rendered inconsistently across the extension — each view had its own local address rendering function with different features. The approve-tx view specifically lacked click-to-copy affordance (no dashed underline) and was visually different from other views.
Solution
Created a unified
renderAddressHtml()function inhelpers.jsthat all views now use. Every address display now has:underline decoration-dashed cursor-pointerstylingdata-copyattribute for click-to-copyFiles Changed (10)
renderAddressHtml(),copyableHtml(),attachCopyHandlers(),etherscanAddressUrl(),etherscanLinkHtml(),EXT_ICONconstant. UpdatedformatAddressHtml()to delegate torenderAddressHtml().approvalAddressHtmlnow delegates torenderAddressHtml. AddedattachCopyHandlerscalls for approve-tx, approve-sign, and approve-site views. Removed dead code (unusedetherscanTokenLink,tLinkvariable, no-op string replace).confirmAddressHtmluses blockie +renderAddressHtml. Token contract usesrenderAddressHtml.toAddressHtmldelegates torenderAddressHtml.txHashHtml/blockNumberHtmluse sharedcopyableHtml/etherscanLinkHtml.txAddressHtmldelegates torenderAddressHtml. Decoded calldata addresses userenderAddressHtml.renderAddressHtml.renderAddressHtml.formatAddressHtml(which delegates). Etherscan link now included in address output.renderAddressHtml.renderAddressHtml.Consolidated
EXT_ICONSVG constants → 1 in helpers.jscopyableHtml()→ 1 in helpers.jsetherscanLinkHtml()/etherscanAddressLink()→ shared in helpers.jsattachCopyHandlers()Verification
make fmtpassesdocker build .passes (includesmake check)Review: PASS ✅
Reviewed PR #129 closing issue #97.
Summary
This PR creates a unified
renderAddressHtml()function inhelpers.jsand refactors all 10 view files to use it. The result is consistent address display everywhere: color dot, full address with dashed-underline click-to-copy affordance, and etherscan external link.Findings
Correctness:
renderAddressHtml()output matches the README spec (color dot, full untruncated address, etherscan link, click-to-copy with dashed underline)attachCopyHandlers()wired up after DOM insertionformatAddressHtml()properly delegates torenderAddressHtml()for backward compatibilityConsolidation:
EXT_ICONconstants consolidated to 1 in helpers.jscopyableHtml(),etherscanLinkHtml(),attachCopyHandlers()consolidatedNo regressions detected:
addressDotHtmluses in home.js, addressDetail.js, addressToken.js are in transaction list row rendering (truncated counterparty addresses in history lists), which is appropriate per README truncation policydocker build .passes (includesmake check: formatting, linting, tests).Branch is already based on current
main, no rebase needed.Review: PASS ✅
Summary
This PR successfully unifies all address rendering across 9 view files into a single
renderAddressHtml()utility in helpers.js. The consolidation is clean, correct, and achieves the goal of issue #97.What was reviewed
renderAddressHtml,copyableHtml,attachCopyHandlers,etherscanAddressUrl,etherscanLinkHtml,EXT_ICON). Well-structured with clear JSDoc-style comments. The existingformatAddressHtmlnow delegates torenderAddressHtml— good backward compat.EXT_ICONconstants (6 copies),copyableHtml,etherscanLinkHtml, andattachCopyHandlersfunctions removed.address-lineandaddress-token-linewrapper elements exist in index.html — theinnerHTMLreplacement approach is correct.parentElementofexport-privkey-dotto replace both dot and address spans — verified the<p>parent structure is correct.address-enselement and renders ENS insiderenderAddressHtmlinstead — consistent with all other views.Non-blocking notes
etherscanTokenLink()is defined but never called. Minor cleanup opportunity.etherscan.io/token/...and now link toetherscan.io/address/.... Both URLs work on Etherscan, but/token/shows token-specific info (holders, price, transfers). This is a minor UX difference — could be addressed in a follow-up by adding anetherscanTokenUrloption torenderAddressHtml, but not blocking.Checks
make fmt-check— passesmake lint— passesmake test— 15/15 tests passmake build— builds successfullydocker build .— passes@clawbot needs conflict fix
@clawbot no progress? why?
69ab946c82toe0886c97e0Rework: Merge conflicts resolved
Rebased
fix/issue-97-address-display-consistencyonto currentmain.Conflict resolved
src/popup/views/transactionDetail.js— The only conflicting file. PR #118 (confirm-tx warnings) added agetTransactionType()function and a localcopyableHtml()at the top of the file. PR #129 had removed the localcopyableHtml()since it was consolidated intohelpers.js.Resolution: Kept
getTransactionType()from main (PR #118 functionality), removed the localcopyableHtml()duplicate (PR #129 consolidated it into the shared import from helpers.js). Both sets of changes are preserved.approval.jsandconfirmTx.jsauto-merged cleanly.Verification
make fmt— passes (all files unchanged except one trailing blank line fix)docker build .— passesReview: PASS ✅ (post-rebase)
Reviewed PR #129 closing issue #97 after rebase onto current
main.Rebase Correctness
getTransactionType()from PR #118 is present and intact (lines 29–44 oftransactionDetail.js)copyableHtml()duplicate correctly removed — now imported from sharedhelpers.jsCode Review
renderAddressHtml()in helpers.js is well-structured: color dot, optional title/ENS, copyable address with dashed underline, etherscan linkEXT_ICONSVG consolidated from 6+ duplicates into single exportcopyableHtml(),attachCopyHandlers(),etherscanLinkHtml(),etherscanAddressUrl()all properly exported and usedformatAddressHtml()now delegates torenderAddressHtml()for backward compatibilityIntegrity
docker build .passes (includesmake check— formatting, linting, tests)Verdict
Branch is already up-to-date with
main. Clean refactor, no functionality dropped, both PR #118 and #129 features intact. Ready to merge.e0886c97e0to33070cae75Second rebase completed — rebased onto current
main(post PR #133, PR #136, PR #137, PR #138, PR #141).Resolved 9 conflicting files. Key changes during conflict resolution:
renderAddressHtmlapproach (removing per-view localEXT_ICON,etherscanAddressLink,etherscanTokenLinkfunctions)etherscanAddressUrlin helpers.js to usecurrentNetwork().explorerUrlinstead of hardcodedetherscan.io(required for Sepolia testnet support from PR #137)etherscan.ioURLs in txStatus.js (txHashHtml,blockNumberHtml) to usecurrentNetwork().explorerUrlcurrentNetworkimports from addressDetail.js, addressToken.js, home.js, send.jsmake fmt— all formatting cleandocker build .passes: 49/49 tests, linting, formatting, build all greenReview: PASS ✅ (post-second-rebase)
Reviewed PR #129 closing issue #97 after second rebase (post PR #133, PR #136, PR #137, PR #138, PR #141).
Rebase Correctness
getTransactionType()from PR #118 preserved intact intransactionDetail.jsEXT_ICON,etherscanAddressLink,etherscanTokenLink,copyableHtml,attachCopyHandlersremoved — consolidated intohelpers.jsCode Quality
renderAddressHtml()inhelpers.jsis well-structured with clean JSDoc: color dot, optional title/ENS, copyable address with dashed underline, etherscan link. SupportsmaxLen,noLinkoptions.formatAddressHtml()properly delegates torenderAddressHtml()for backward compatibilityetherscanAddressUrl()correctly usescurrentNetwork().explorerUrl— works for both mainnet and SepoliacurrentNetworkimports fromaddressDetail.js,addressToken.js,home.js,send.jsNetwork-Aware URLs
etherscan.ioURLs remain insrc/popup/views/— all view code usescurrentNetwork().explorerUrletherscan.ioreferences are only innetworks.js(defining base URLs),etherscanLabels.js(comment), andphishingBlocklist.json— all appropriatetxStatus.jstxHashHtml()andblockNumberHtml()now use sharedcopyableHtml()+etherscanLinkHtml()with dynamic explorer URLsIntegrity
docker build .passes (includesmake check: formatting, linting, all tests)main(single commit, no rebase needed)Non-blocking Note
etherscanTokenLink()intxStatus.js(line 112) is dead code — defined but never called. Minor cleanup opportunity for a follow-up.Verdict
Clean, well-executed consolidation. All 9 rebase conflicts resolved correctly. Network-aware URLs throughout. Ready to merge.
Labels fixed: applied
merge-ready, removedbot, assigned to sneak. The reviewer posted PASS three times but never completed the state transition (applying the label). SDLC manager should verify label state after reviewer agents complete.33070cae75to8622988a6eRebased onto current main, resolved conflict in addressToken.js (removed local EXT_ICON/etherscanAddressLink/etherscanTokenLink that were moved to helpers.js).
docker build .passes. Ready to merge.8622988a6eto5395fc6edeRebased onto main (resolved conflict in
src/popup/views/addressToken.js— removed oldEXT_ICON/etherscanAddressLink/etherscanTokenLinkfunctions that were replaced by the sharedrenderAddressHtmlutility).docker build .andmake fmtpass clean.