Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement BTC transaction detail page - Closes #1929 #1992

Merged
merged 11 commits into from May 9, 2019

Conversation

Projects
None yet
2 participants
@slaweet
Copy link
Member

commented May 9, 2019

What issue have I solved?

#1929

How have I implemented/fixed it?

I implemented the page according to the design, except for:

  • the title is capitalized ("Transfer Transaction") to be consistent with other titles
  • amount value is 18px to be consistent with style guide and other pages (agreed by Dominic: https://lightcurve.slack.com/archives/CGBKV6GCA/p1557393358003900 )
  • Tooltip for "Confirmations" is not implemented because we had only LSK-specific copy (see row 14), waiting for a copy that works also for BTC.

How has this been tested?

  1. Go to https://jenkins.lisk.io/test/lisk-hub/PR-1992/
  2. log it to testnet
  3. Set localStorage.setItem('btc', true);
  4. Go to https://jenkins.lisk.io/test/lisk-hub/PR-1992/#/explorer/transactions/553787aa91a55e0385beb81a0a549f02cb3be8492b452fe8b09c1729165a8362
  5. see there a BTC transaction details.

Review checklist

slaweet added some commits May 8, 2019

@slaweet slaweet self-assigned this May 9, 2019

slaweet added some commits May 9, 2019

@slaweet slaweet requested a review from osvaldovega May 9, 2019

@slaweet slaweet marked this pull request as ready for review May 9, 2019

@osvaldovega

This comment has been minimized.

Copy link
Contributor

commented May 9, 2019

Vit I just checked the PR and everything is ok, however Dom told that there is one thing on design, so I'm just showing you what problem.

The transaction ID is long so instead of have it center and we can align it to the left like this.

Actual.
Screen Shot 2019-05-09 at 14 27 37

Will be nice

Screen Shot 2019-05-09 at 14 28 12

But again this is just a minor thing based on design, so let me know what do you think.
Your PR looks good.

@osvaldovega
Copy link
Contributor

left a comment

🏆good

@osvaldovega
Copy link
Contributor

left a comment

perfect

@osvaldovega osvaldovega added the ready label May 9, 2019

@slaweet slaweet merged commit b6978a1 into development May 9, 2019

3 checks passed

Jenkins e2e tests e2e tests passed
Details
Jenkins test deployment Commit was deployed to test
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details

@slaweet slaweet deleted the 1929-btc-tranasction-detail-page branch May 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.