Skip to content

Conversation

martonp
Copy link
Collaborator

@martonp martonp commented Jun 1, 2022

Screen Shot 2022-06-02 at 11 45 52 AM

@@ -232,6 +233,10 @@ export default class WalletsPage extends BasePage {
Doc.hide(this.page.forms)
}

copyAddress () {
navigator.clipboard.writeText(this.page.depositAddress.textContent || '')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. https://caniuse.com/mdn-api_clipboard_writetext
I'm assuming 'click' counts as a "user gesture event handlers such as pointerdown or pointerup." so it will work on Safari. Can anyone test on Safari?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works on Safari. I put the icon inside the box.. I think it looks better.

Copy link
Member

@buck54321 buck54321 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chappjc
Copy link
Member

chappjc commented Jun 2, 2022

What host makes that happen @buck54321 ? A non-loopback addr?

@buck54321
Copy link
Member

http://[::]:54321

The test app server

@chappjc
Copy link
Member

chappjc commented Jun 2, 2022

So is that a chrome bug with ipv6? That looks like it should qualify as a secure context/origin.

@buck54321
Copy link
Member

Looks like this does the trick.

https://github.com/martonp/dcrdex/compare/copyAddress...buck54321:martonp-copy-addr?expand=1

@chappjc
Copy link
Member

chappjc commented Jun 2, 2022

Ah, yes! I read it as [::1] which basically the opposite of [::].

@buck54321
Copy link
Member

buck54321 commented Jun 2, 2022

Actually, this would prevent me from copying if I access the client from, say, my tablet on the same network.

Maybe navigator.clipboard won't work for us?

@chappjc
Copy link
Member

chappjc commented Jun 2, 2022

I think it's OK. The user can get around that other ways, including putting an https server in front of it or changing the browser flags et.
If navigator.clipboard isn't available, they can still manually select and copy like it was before.

@buck54321
Copy link
Member

Is it unsafe to use execCommand with a hard-coded argument? If it's safe, might as well do it that way, no?

@chappjc
Copy link
Member

chappjc commented Jun 2, 2022

execCommand and friends are deprecated: https://developer.mozilla.org/en-US/docs/Web/API/document/execCommand

Deprecated: This feature is no longer recommended. Though some browsers might still support it, it may have already been removed from the relevant web standards, may be in the process of being dropped, or may only be kept for compatibility purposes. Avoid using it, and update existing code if possible; see the compatibility table at the bottom of this page to guide your decision. Be aware that this feature may cease to work at any time.

I'm switching dcrdata over and it's working alright there: https://github.com/decred/dcrdata/pull/1917/files

@martonp
Copy link
Collaborator Author

martonp commented Jun 3, 2022

I pulled your commit in @buck54321

Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working well.

I think it would be nice if there was indication that the copy worked. Like a message that says Copied! or something. dcrdata has this if you click on the copy icon https://dcrdata.decred.org/address/DsZJ3hUqfaGR4GbwvLDLaSxTGJvYk6ypk4K

@chappjc chappjc added this to the 0.5 milestone Jun 9, 2022
@chappjc chappjc merged commit 258a22f into decred:master Jun 9, 2022
@martonp martonp deleted the copyAddress branch January 20, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants