Skip to content

Conversation

martonp
Copy link
Collaborator

@martonp martonp commented Jun 20, 2022

Adds a disappearing message confirming that the deposit address has been copied.

@ukane-philemon
Copy link
Contributor

Nice, I was actually looking out for this :).

Comment on lines 238 to 239
navigator.clipboard.writeText(page.depositAddress.textContent || '')
Doc.show(page.copyAlert)
Copy link
Member

Choose a reason for hiding this comment

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

We should only show the "Copied!" alert if writeText says it succeeded.

navigator.clipboard.writeText(textContent).then(() => {
	Doc.show(page.copyAlert)
	...
}, (reason) => {
	console.error('Unable to copy:', reason) // or a different alert, whatever
})

decred/dcrdata@c952124

@martonp
Copy link
Collaborator Author

martonp commented Jun 21, 2022

Nice, I was actually looking out for this :).

It feels very unsatisfying to click it and have nothing happen haha.

@chappjc chappjc added this to the 0.5 milestone Jun 21, 2022
Copy link
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

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

Working for me, but the alert text is a little close to the address text div. A couple more pixels of margin and I think it would be a little better.

deposit-copy-click.mp4

@@ -92,3 +92,9 @@ table.wallets {
cursor: pointer;
color: #a8a8a8;
}

#copyAlert {
margin-left: 9px;
Copy link
Member

@chappjc chappjc Jun 22, 2022

Choose a reason for hiding this comment

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

With 9px it's right up against the div for me. Maybe 12?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

12 looks good.

@chappjc
Copy link
Member

chappjc commented Jun 22, 2022

Oh, trivial conflict though.

@martonp martonp force-pushed the copyAddressAlert branch from 8c82eb9 to 337484a Compare June 23, 2022 10:55
martonp added 3 commits June 23, 2022 18:57
Adds a disappearing message confirming that the deposit address
has been copied.
@martonp martonp force-pushed the copyAddressAlert branch from 337484a to 12dc2f1 Compare June 23, 2022 11:57
@chappjc chappjc merged commit 7a288a5 into decred:master Jun 23, 2022
@martonp martonp deleted the copyAddressAlert 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