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 the my account component - closes #2038 #2092

Merged
merged 11 commits into from Jun 6, 2019

Conversation

Projects
None yet
2 participants
@osvaldovega
Copy link
Contributor

commented Jun 4, 2019

What issue have I solved?

-- #2038

How have I implemented/fixed it?

PR includes the implementation of the new dashboard component called My Account Details, this component is just informative and will display the enable currencies that the user has, right now is based on Lisk and BTC (what can be enable/disabled) and this will show the crypto currency and the actual balance.

If the user has disabled BTC then this will not show up in the component and only LSK.

If the user logout then this component won't be visible, in other words this component only will work/show up after Sign in

How has this been tested?

  1. Sign In into the app.
  2. Go to dashboard and at the end of the URL add the V2 like this -> .../#/dashboardV2. Then you will see the new component.
  3. For enable or disable BTC, you can dispatch this function in the browser redux tool.
    For enable BTC
{ type: 'SETTINGS_UPDATED', data: { token: { list: { BTC: true } } } }

For disable BTC

{ type: 'SETTINGS_UPDATED', data: { token: { list: { BTC: false } } } }

Then you will see the change in the component.

NOTE: Is important to have the BTC flag in the local storage set it in true (btc: true) if not you wont be able to see BTC information

Review checklist

@osvaldovega osvaldovega self-assigned this Jun 4, 2019

osvaldovega added some commits Jun 4, 2019

@osvaldovega osvaldovega changed the base branch from development to 1.19.0 Jun 5, 2019

@osvaldovega osvaldovega changed the base branch from 1.19.0 to development Jun 5, 2019

@osvaldovega osvaldovega requested a review from massao Jun 5, 2019

@massao
Copy link
Contributor

left a comment

Everything is working fine, just few comments

<path fill="#F7931A" d="M47.282 29.806c-3.206 12.857-16.23 20.682-29.09 17.475C5.337 44.076-2.488 31.053.719 18.197 3.923 5.338 16.947-2.487 29.803.718c12.86 3.206 20.684 16.23 17.479 29.088z"/>
<path fill="#FFF" d="M35 21c.4-3.092-1.93-4.786-5-6l1-4-3-1-1 4c-.274.153-.956-.008-2 0l1-4-2-1-1 4c-.724.14-1.263.013-2 0l-3-1-1 3c.066-.254 1.938.188 2 0 .92.479 1.105 1.175 1 2l-1 5c-.033-.39.059-.363 0 0 .075-.343-.015-.366 0 0l-2 6c.118.745-.2 1.225-1 1 .112.08-1.747-.429-2 0l-1 3h3c.894.547 1.493.712 2 1l-1 4 3 1 1-4c.28-.246.949-.068 2 0l-1 4 2 1 1-4c4.442.528 7.677.19 9-4 1.137-3.047-.015-4.909-2-6 1.37-.515 2.611-1.632 3-4zm-6 9c-.878 3.402-6.256 1.547-8 1l1-6c2.156.467 7.747 1.393 7 5zm1-9c-.839 3.175-5.491 1.656-7 1l1-5c1.84.372 6.7 1.067 6 4z"/>
</g>
</svg>

This comment has been minimized.

Copy link
@massao

massao Jun 5, 2019

Contributor

If I'm not wrong, we already have this one in the project.

</g>
</g>
</g>
</svg>

This comment has been minimized.

Copy link
@massao

massao Jun 5, 2019

Contributor

Use some tool like SVGo to optimize the SVG.

: null
}

<RecentTransactions className={styles.marginFix} isSignIn={isLoggedIn} />

This comment has been minimized.

Copy link
@massao

massao Jun 5, 2019

Contributor

Shouldn't be better to rename the props from isSignIn to isLoggedIn? so it's consistent.
Also isSignIn, sounds like it's checking if it's the signIn flow or something like that

const token = settings.token;

const coins = Object.entries(info)
.map(coin => (coin[0] === tokenMap[coin[0]].key && token.list[coin[0]] === true) && coin[1])

This comment has been minimized.

Copy link
@massao

massao Jun 5, 2019

Contributor

Wouldn't something like

Object.entries(info)
  .map(([key, coin]) => token.list[key] && coin)
  .filter(coin => coin);

or

Object.entries(info)
  .reduce((coins, [key, coin]) => (token.list[key] ? [...coins, coin] : coins), []);

work?? Also would have a better readability than coin[0].

@@ -51,6 +51,8 @@ import checkboxFilled from '../assets/images/icons-v2/checkmark-16-filled.svg';
import checkmark from '../assets/images/icons-v2/checkmark.svg';
import newsFeedAvatar from '../assets/images/icons-v2/newsFeedsAvatar.svg';
import bookmarksIconEmptyState from '../assets/images/icons-v2/bookmarks_empty_state.svg';
import lskIcon40 from '../assets/images/icons-v2/icon-lsk.svg';
import btcIcon40 from '../assets/images/icons-v2/icon-btc.svg';

This comment has been minimized.

Copy link
@massao

massao Jun 5, 2019

Contributor

Since both are SVGS, we shouldn't be using a number on them.

@osvaldovega

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

changes done

@massao
Copy link
Contributor

left a comment

Nicely done 🥇

@massao

massao approved these changes Jun 6, 2019

@osvaldovega osvaldovega merged commit 2d0cbc3 into development Jun 6, 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

@osvaldovega osvaldovega deleted the 2038-implement-my-account-component branch Jun 6, 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.