Skip to content

Add Copy Tx ID button to transaction-list-item-details#6146

Merged
whymarrh merged 2 commits intoMetaMask:developfrom
chikeichan:txid
Feb 17, 2019
Merged

Add Copy Tx ID button to transaction-list-item-details#6146
whymarrh merged 2 commits intoMetaMask:developfrom
chikeichan:txid

Conversation

@chikeichan
Copy link
Copy Markdown
Contributor

copy

@bdresser @cjeria
See attached gif. I added a "copy to clipboard" to tx detail for user to copy their tx id. I thought this fits the existing UX better than adding another component showing the tx hash. What do you think?

Copy link
Copy Markdown
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

One small thing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we move this into the callback for the above setState call? We'd want this to run after the copy?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yea make sense, i will move it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated

@bdresser
Copy link
Copy Markdown
Contributor

bdresser commented Feb 14, 2019

LGTM @chikeichan -- does this work out okay when we're also showing the "Cancel" and "Speed Up" buttons?

danjm
danjm previously approved these changes Feb 15, 2019
Copy link
Copy Markdown
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

I think this is good to go, once Whymarrh's comment is addressed.

@damster7

This comment has been minimized.

@damster7

This comment has been minimized.

@whymarrh whymarrh merged commit b58a9bd into MetaMask:develop Feb 17, 2019
@danfinlay danfinlay mentioned this pull request Feb 19, 2019
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.

5 participants