Page MenuHomePhabricator

[explorer] Add copy paste button for addresses on transaction page
ClosedPublic

Authored by bytesofman on Aug 8 2024, 00:04.

Details

Summary

It would be useful to copy paste addresses when viewing a transaction. Add button for this.

Test Plan

preview and test feature

Diff Detail

Repository
rABC Bitcoin ABC
Branch
explorer-copy-address
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 29925
Build 59385: Build Diff
Build 59384: arc lint + arc unit

Event Timeline

  • fix flex issue when address text wraps
  • add to inputs
  • more responsiveness testing

better styling, add copy button for inputs as well

bytesofman published this revision for review.Aug 8 2024, 16:41
bytesofman added inline comments.
web/explorer/explorer-server/code/common.js
417 ↗(On Diff #49096)

we cannot get addresses by ID without programmatically generating nonce IDs, and some txs would have 1000s.

So we add a new function to copy a given param.

It is "better" to copy "the text that is there on the page." But, in this case, they are both generated from the same variable in the template.

web/explorer/explorer-server/templates/base.html
1 ↗(On Diff #49096)

I think auto change from linter as this page has not been edited in some time.

105 ↗(On Diff #49096)

not sure what the original hash value was. I took the same chars from sha256sum code/common.js output.

The important thing is the value is different from the "old" deployed version. So the site will not use the cached version and will get the new function.

emack requested changes to this revision.Aug 9 2024, 04:37
emack added a subscriber: emack.
emack added inline comments.
web/explorer/explorer-server/code/common.js
421 ↗(On Diff #49096)

What's the need behind the 1.3 second delay here?

This revision now requires changes to proceed.Aug 9 2024, 04:37
bytesofman marked an inline comment as done.
bytesofman added inline comments.
web/explorer/explorer-server/code/common.js
421 ↗(On Diff #49096)

tbh I didn't even see this, I just directly copied this function from the existing copyText and only changed what is being copied.

looking into it -- we should keep it.

What's happening here

  • User hovers over copy paste icon. Tooltip is "copy to clipboard"
  • User clicks copy paste icon. Text is instantly copied to clipboard. Tooltip changes to "Copied!"
  • In 1.3s, the tooltip changes back to "copy to clipboard"

1.3s seems like a decent value, already what the app has been using. Since this function is not trying to alter the behavior of the app, just cover the ability to copy a new variable, makes sense to keep it the same.

Fabien requested changes to this revision.Aug 10 2024, 08:53

This doesn't work for me. I see the icon and the tooltip but on click I get TypeError: navigator.clipboard is undefined (Firefox on linux)

web/explorer/explorer-server/code/common.js
418 ↗(On Diff #49096)

Please use brackets, it's hard to know where the scope of this if ends

425 ↗(On Diff #49096)

navigator.clipboard is undefined at this line

This revision now requires changes to proceed.Aug 10 2024, 08:53

This doesn't work for me. I see the icon and the tooltip but on click I get TypeError: navigator.clipboard is undefined (Firefox on linux)

Does copy paste in other places work, like address at top of address page, or txid at top of tx page?

use brackets to clean up copyString function

emack requested changes to this revision.Aug 11 2024, 12:31

Clicking the copy icon for txid at the top gives this

image.png (103×527 px, 9 KB)

Whilst clicking the copy icon for the addresses triggers the tooltip but no response. Have verified address has not been copied to clipboard.

This revision now requires changes to proceed.Aug 11 2024, 12:31

Clicking the copy icon for txid at the top gives this

image.png (103×527 px, 9 KB)

Whilst clicking the copy icon for the addresses triggers the tooltip but no response. Have verified address has not been copied to clipboard.

I can't repeat this with cargo run locally .. possibly there is some functionality missing in the preview site. will test

Clicking the copy icon for txid at the top gives this

image.png (103×527 px, 9 KB)

Whilst clicking the copy icon for the addresses triggers the tooltip but no response. Have verified address has not been copied to clipboard.

Ok I was able to repeat this in a diff that makes no changes from master except a token test text change

D16622

So -- there is some issue where the JS does not work on explorer previews, prob related to not being https, would have to troubleshoot -- anyway not related to this diff

test with cargo run locally

Clicking the copy icon for txid at the top gives this

image.png (103×527 px, 9 KB)

Whilst clicking the copy icon for the addresses triggers the tooltip but no response. Have verified address has not been copied to clipboard.

Ok I was able to repeat this in a diff that makes no changes from master except a token test text change

D16622

So -- there is some issue where the JS does not work on explorer previews, prob related to not being https, would have to troubleshoot -- anyway not related to this diff

test with cargo run locally

OK I confirm this is the issue, the clipboard is only defined for https or localhost so it can't work on preview but it works just fine locally.

This revision is now accepted and ready to land.Aug 12 2024, 14:59