Page MenuHomePhabricator

[Cashtab] Tx History collapse menu
ClosedPublic

Authored by kieran709 on Feb 25 2022, 22:13.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC2524ad02e1b8: [Cashtab] Tx History collapse menu
Summary

Tapping a tx in tx history extends a collapse which reveals: a link to the tx that opens the block explorer in a new tab and a copy txid button which fires a notification when the txid has been copied to the user's clipboard. Related to task T1774.

Test Plan

cd web/cashtab
npm start
on the main wallet tab, click on a transaction
onClick, the tx card should expand, revealing a link to the tx on the block explorer>
click on the link, it should open the block explorer in a new tab
click on the Copy Txid button, it should fire a success notification
paste the txid somewhere to ensure it was copied to the clipboard.
send XEC, check that the new transaction panel expands on click
receive XEC, check that the new transaction panel expands on click

Diff Detail

Repository
rABC Bitcoin ABC
Branch
tx-history-collapse
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 18392
Build 36581: Build Diffcashtab-tests
Build 36580: arc lint + arc unit

Event Timeline

bytesofman added inline comments.
web/cashtab/src/components/Home/Tx.js
673 ↗(On Diff #32538)

Create these icons with their required css in the CustomIcons.js file under components/common. Then export them here.

688 ↗(On Diff #32538)

Create these icons with their required css in the CustomIcons.js file under components/common. Then export them here.

This revision now requires changes to proceed.Feb 25 2022, 22:36

Respond to review feedack, rebase & update snapshots.

bytesofman requested changes to this revision.Mar 1 2022, 22:01

image.png (217×61 px, 2 KB)

The antd down arrow is still visible -- please remove (without removing it from other Collapse items)

This revision now requires changes to proceed.Mar 1 2022, 22:01

Responding to review feedback.

bytesofman requested changes to this revision.Mar 3 2022, 21:05
bytesofman added inline comments.
web/cashtab/src/components/Home/Tx.js
661

we use react-copy-to-clipboard for copying addresses on the Receive screen -- why not here?

If this approach is preferable (getting rid of a dependency is in general always a good option, so this is likely better), then we should create a task to migrate away from react-copy-to-clipboard.

Please

  1. Evaluate this approach vs react-copy-to-clipboard
  2. Make a recommendation on which is better (in a comment in this diff)
  3. Apply that method here + create a task to standardize across Cashtab, if necessary
678

use the param in currency object from Ticker.js, do not hardcode blockexplorer link

This revision now requires changes to proceed.Mar 3 2022, 21:05

Having looked into it further, I think it will be better to use react-copy-to-clipboard throughout the app as the navigator.clipboard.writeText() method will not work consistently across all browsers. This change will be reflected in my incoming revision.

Replaced navigator copy to clipboard method with react-copy-to-clipboard. Removed hardcoded explorer url and replaced it with param in currency object.

This revision is now accepted and ready to land.Mar 4 2022, 19:47
bytesofman requested changes to this revision.Mar 4 2022, 19:48

one nit, see inline

web/cashtab/src/components/Common/StyledCollapse.js
107 ↗(On Diff #32595)

Delete this commented out css

This revision now requires changes to proceed.Mar 4 2022, 19:48

Removed commented code per review feedback.

This revision is now accepted and ready to land.Mar 4 2022, 21:17
This revision was automatically updated to reflect the committed changes.