Add txid and explorer link in tx details page#368
Add txid and explorer link in tx details page#368louisinger wants to merge 1 commit intoArkLabsHQ:masterfrom
Conversation
📝 WalkthroughWalkthroughThis PR propagates an Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
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
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/interface/web/templates/components/feeTable_templ.go (1)
644-708: Guard against short txid slices to avoid panic.
arkTxid[:12]panics whenlen(arkTxid) < 12, which will break rendering for short/invalid IDs. Add a length-safe truncation (and apply the same change inSentTxTable).🐛 Suggested fix (apply in the .templ source and regenerate)
- if arkTxid != "" { - templ_7745c5c3_Err = tableLine("Tx ID", arkTxid[:12]+"...").Render(ctx, templ_7745c5c3_Buffer) + if arkTxid != "" { + displayTxid := arkTxid + if len(displayTxid) > 12 { + displayTxid = displayTxid[:12] + "..." + } + templ_7745c5c3_Err = tableLine("Tx ID", displayTxid).Render(ctx, templ_7745c5c3_Buffer)
🤖 Fix all issues with AI agents
In `@internal/interface/web/static/styles.css`:
- Line 1: The generated CSS contains duplicate properties (e.g., repeated
-webkit-text-decoration and outline in selectors like html,:host and focus
rules) causing lint failures; either update the CSS generation pipeline to
remove duplicate declarations (dedupe output for selectors such as html,:host,
a, and input:focus) or mark these built assets as excluded/generated in the lint
configuration so Biome ignores internal/interface/web/static/styles.css;
implement the fix by changing the source/postcss/tailwind generator to
deduplicate properties or by adding an exception rule in the linter config to
skip generated CSS files.
In `@internal/interface/web/templates/components/feeTable.templ`:
- Around line 142-147: The code slices arkTxid with arkTxid[:12] which will
panic if arkTxid is shorter than 12 chars; update the template to use a safe
truncation (e.g., a helper function safeTruncate(s, n) or inline min-length
check) when rendering the Tx ID and explorer link text so you only take up to
len(arkTxid) characters, and apply the same fix to the second occurrence around
the explorer block (the other instance at the later block referenced in the
comment); ensure you update the references where arkTxid[:12] is used and call
the safeTruncate helper (or use a conditional slice using min(len,12)) instead.
♻️ Duplicate comments (1)
internal/interface/web/templates/components/feeTable_templ.go (1)
761-825: Same txid slice safety issue as inReceivedTxTable.
| @tableLine("Tx ID", arkTxid[:12] + "...") | ||
| <div class="flex justify-between mb-2"> | ||
| <p class="text-white/50 uppercase">explorer</p> | ||
| <a href={"https://arkade.space/tx/" + arkTxid} target="_blank" rel="noopener noreferrer" class="text-blue hover:underline">View</a> |
Kukks
left a comment
There was a problem hiding this comment.
Needs to be a sane default + config option not hardcoded in the views
|
👋 @louisinger — @Kukks requested changes on this PR over a month ago. Is this still in progress, or has it been superseded? |
|
Review: Add txid and explorer link in tx details page Clean UX improvement — surfaces the Ark transaction ID and a direct link to arkade.space/tx in the transaction detail views. What changed:
Looks good, minor notes:
No security concerns. Cross-repo note: this pairs well with the arkd indexer work that makes Ark txids available through the SDK. |
|
👋 @louisinger — @Kukks had change requests on this one from back in January. The last commit was over 2 months ago. Is this still on the roadmap, or has it been superseded by other work? |
|
Hey @louisinger — @Kukks requested changes on this PR about 6 weeks ago and there's been no update since. Still planning to address this? Happy to help if needed. |
|
👋 Hey @louisinger — friendly nudge! Kukks requested changes back on Feb 11. Is this PR still active? Let us know if there's anything blocking progress. |
|
Hey @louisinger — @Kukks requested changes on this back on Feb 11 and it hasn't been updated since (~45 days). Still on your radar? |
|
Superseded by #385 — rebased onto current master, resolved conflicts, and addressed review feedback (configurable explorer URL, safe txid truncation). |
|
Hey @louisinger — just a friendly check-in. This PR has been waiting since @Kukks's CHANGES_REQUESTED on Feb 11 (48 days). The ask was: make the explorer URL a sane default + config option rather than hardcoding it in views. Is this still on your radar? |
|
Hey @louisinger — this has had a CHANGES_REQUESTED review from @Kukks since Feb 11 with no update to the branch since Jan 18. Is this still on your radar, or has it been superseded by other work? |
|
👋 Hey @louisinger — friendly nudge! Kukks requested changes back on Feb 11 (needs to be a sane default + config option, not hardcoded in the views). This has been open for ~7 weeks. Are you still planning to address this, or should we close it in favor of the newer #385? Let us know! |
@altafan @Kukks please review
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.