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 my recent transactions in dashboard module - closes #2039 #2044

Merged

Conversation

3 participants
@osvaldovega
Copy link
Contributor

commented May 20, 2019

What issue have I solved?

-- #2039

How have I implemented/fixed it?

Implement new design of the recent transactions in the dashboard, supporting BTC token too.
Using BoxV2 for the container element and based on the active token will display the last 5 transactions for LSK or BTC based on that what is enable, if use change the token this will automatically update and show the list again.

There are 3 components now, one call TransactionTypeFigure, TransactionAddress and TransactionAmount that will be on charge on display the information based on the new design, this means that these component just need tx details and active token for display the information.

How has this been tested?

Right now this component was implemented but not added into the actual dashboard as the dashboard component needs to be update and rest of elements too, once the other elements are done this can be enable/add it into the dashboard.

For change the active token this action needs to be dispatch in the redux devTools in the browser.
LSK:

{type:‘SETTINGS_UPDATE_TOKEN’, data: { token: { active: ‘LSK’, list: {BTC: true, LSK: true} }}}

BTC:

{type:‘SETTINGS_UPDATE_TOKEN’, data: { token: { active: ‘BTC’, list: {BTC: true, LSK: true} }}}

NOTE: For see the transactions based on the active token this PR needs to be merge first https://github.com/LiskHQ/lisk-hub/pull/2003/files#diff-5b74ccb6f4da52978806178cbf93433e.
This is because in this PR now all the transactions will have a new property "token" will be the flag for filter the transactions.

NOTE: For now the empty state show a validation when the active token is LSK or BTC, what it does is that if the active token is LSK then a Lear more button will be display and the external link is for the LSK documentation, right now we don't have the external link for the BTC, so once the external link will be available for the BTC documentation the validation can be remove

Review checklist

osvaldovega added some commits May 20, 2019

Merge branch '2039-implement-my-recent-transactions-dashboard-module'…
… of github.com:LiskHQ/lisk-hub into 2039-implement-my-recent-transactions-dashboard-module

@osvaldovega osvaldovega requested a review from massao May 24, 2019

@massao
Copy link
Contributor

left a comment

Some few comments, and also instead of having multiple components folders like: transactionAmount, transactionAddress I would have them inside a common folder, maybe transaction/amount or something similar.

<path stroke-linecap="square" d="M8 10.5v3"/>
</g>
</g>
</svg>

This comment has been minimized.

Copy link
@massao

massao May 27, 2019

Contributor

Do we need to keep both files, the tx-2nd-passphrase and tx-2nd-passphrase2?
Also IMO, we should use some lib to optimize the SVGs, something like the SVGo.

@@ -96,6 +96,7 @@ or "warn/action" ineastd of "red/green"
--color-white-smoke: #f5f7fa;
--color-burnt-sienna: #ec6868;
--color-ufo-green: #2bd67b;
--color-amount-green: #00d563;

This comment has been minimized.

Copy link
@massao

massao May 27, 2019

Contributor

I would rename this, since in the future it may be used to something else rather than the amount.

);
};

export default TransactionAddress;

This comment has been minimized.

Copy link
@massao

massao May 27, 2019

Contributor

If i'm not wrong, we only use index file for components for HOC

@massao

This comment has been minimized.

Copy link
Contributor

commented May 27, 2019

Also you could create a new route for the dashboard, similar to what we did on other pages, dashboardV2, so it would be easier to replace on the future

@reyraa reyraa added this to Pull Requests in Version 1.18.0 via automation May 27, 2019

@osvaldovega

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2019

@massao I did the changes man, when have time check it again

@massao
Copy link
Contributor

left a comment

When set with BTC token with no transactions, it shows the message specific to LSK:
image

@osvaldovega

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2019

ok after check with Ali, I made some changes to fix the empty state.

@massao

massao approved these changes May 27, 2019

Copy link
Contributor

left a comment

👍

@osvaldovega osvaldovega merged commit e8bb748 into development May 27, 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

Version 1.18.0 automation moved this from Pull Requests to Merged Pull Requests May 27, 2019

@osvaldovega osvaldovega deleted the 2039-implement-my-recent-transactions-dashboard-module branch May 27, 2019

@reyraa reyraa added the ready label Jun 14, 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.