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

Migrate dashboard to the new design - closes #1936 #2125

Merged
merged 27 commits into from Jun 20, 2019

Conversation

Projects
None yet
5 participants
@osvaldovega
Copy link
Contributor

commented Jun 11, 2019

What issue have I solved?

-- #1936

How have I implemented/fixed it?

Update the routes file for replace the actual dashboard to the new dashboard.
Bookmark will display based on the active token.
Wallet details will show ONLY when user signed in.
Recent transactions (last 5) will display based on active token.

Recent transaction logic update to display information properly when active token change

NOTE:
./src/bookmarks folder needs to be deleter after implement Bookmarks page

How has this been tested?

  1. Login into the app.
  2. Check the component wallet details show properly information same as in the top bar for LSK and BTC.
  3. Check the list of recent tx, tops 5 tx will be shown.
  4. Community feed show properly the information and click on each message will open a new window.
  5. Bookmarks component shows a max of 5 bookmarks.
  6. Click on any bookmark will redirect to the specific account.

The dashboard show information for LSK and BTC so for see the BTC part you need to Add btc to localStorage: localStorage.setItem('btc', true); and for switch between BTC and LSK you need to dispatch this function from the redux devTool in the browser.

For LSK
{type:'SETTINGS_UPDATED', data: { token: { active: 'LSK' } } }

For BTC
{type:'SETTINGS_UPDATED', data: { token: { active: 'BTC' } } }

NOTE: For now the View All button in bookmarks component will he hide because there is no bookmark page for now, once we implement bookmarks page the button will be enable it and redirect to that page

Review checklist

slaweet and others added some commits Jun 6, 2019

@osvaldovega osvaldovega self-assigned this Jun 11, 2019

@yasharAyari
Copy link
Member

left a comment

the community feed box shouldn't expand when a user clicks on the load more button. the hight of the box of community feed should be fixed and it must have a scrollbar if it is needed.

osvaldovega added some commits Jun 11, 2019

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

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

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

@massao
Copy link
Contributor

left a comment

Everything is working nicely, just some small changes due to code consistency.

@@ -119,24 +98,5 @@ describe('Dashboard Bookmarks', () => {
cy.get(ss.bookmarkAccount).eq(4).should('be.visible');
cy.get(ss.bookmarkAccount).eq(6).should('be.not.visible');
});

This comment has been minimized.

Copy link
@massao

massao Jun 14, 2019

Contributor

Update the comment and the assertions of the test to reflect the changes.

@@ -1,4 +1,5 @@
import React from 'react';
import svg from '../../utils/svgIcons';

This comment has been minimized.

Copy link
@massao

massao Jun 14, 2019

Contributor

Should be using Icons toolbox component

@@ -1,6 +1,5 @@
import React, { Component } from 'react';
import Box from '../../boxV2';
import removeDuplicateTransactions from '../../../utils/transactions';
import TransactionList from './transactionList';
import EmptyState from '../../emptyStateV2';
import svg from '../../../utils/svgIcons';

This comment has been minimized.

Copy link
@massao

massao Jun 14, 2019

Contributor

Replace with Icon toolbox component

import logo from '../../assets/images/Lisk-Logo.svg';
import ShowMore from '../showMore';
import EmptyState from '../emptyStateV2';
import svg from '../../utils/svgIcons';

This comment has been minimized.

Copy link
@massao

massao Jun 14, 2019

Contributor

Replace with Icon toolbox component

@@ -1,4 +1,5 @@
import React from 'react';
import svg from '../../utils/svgIcons';

This comment has been minimized.

Copy link
@massao

massao Jun 14, 2019

Contributor

Replace with Icon toolbox component

osvaldovega added some commits Jun 14, 2019

@massao
Copy link
Contributor

left a comment

Nice work! 👍

@massao massao requested a review from Efefefef Jun 17, 2019

@Efefefef
Copy link
Contributor

left a comment

🐛 The width and height is not like in the design
image

🐛 There is no border under the last recent tx
image

🐛 Change your active token to BTC and wait for recent txs to change. Thaaat does not really happen. Than change page and go back -> you will see LSK recent txs
image

@Efefefef

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

@reyraa Onboarding banner texts are not relevant. Examples:
Following a delegate
Requesting a feature

image

image

@Efefefef

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

🐛 The BTC addresses should be aligned in the transactions column
image

@osvaldovega

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2019

Hi @Efefefef I update the PR after your request changes.
About the text display in the onbording component I had to create this follow up ticket #2153

@Efefefef

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

🐛 Margin
image

🐛 Button size is too big
image

🐛 Height of rows with 2 lines of info is different
image

🐛 Margin
image

osvaldovega added some commits Jun 19, 2019

Merge branch '1936-migrate-dashboard-to-the-new-design' of github.com…
…:LiskHQ/lisk-hub into 1936-migrate-dashboard-to-the-new-design

osvaldovega added some commits Jun 19, 2019

@mixin contentNormal bold;

width: 85px;
height: 36px;

This comment has been minimized.

Copy link
@slaweet

slaweet Jun 20, 2019

Member

@osvaldovega we have classes created my @massao for button sizes

<PrimaryButtonV2 > Large </PrimaryButtonV2>
<PrimaryButtonV2 className='medium'> Medium </PrimaryButtonV2>
<PrimaryButtonV2 className='small'> Small </PrimaryButtonV2>
<PrimaryButtonV2 className='extra-small'> Extra Small </PrimaryButtonV2>

This comment has been minimized.

Copy link
@massao

massao Jun 20, 2019

Contributor

Also, I'm not sure if the contentNormal mixin is needed in this case, since all buttons, should already have the correct font style, so it's easier to update in the future if needed

This comment has been minimized.

Copy link
@slaweet

slaweet Jun 20, 2019

Member

Different button sizes have different font size, so I understand that Osvaldo needed the mixin when not using the size className. With the className, it's not needed anymore.

@Efefefef

This comment has been minimized.

Copy link
Contributor

commented Jun 20, 2019

Please move to Pending QA when its ready

@osvaldovega

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2019

I fixed the problem related the height of the Wallet details component when the BTC is not enabled

@osvaldovega osvaldovega requested a review from Efefefef Jun 20, 2019

@Efefefef
Copy link
Contributor

left a comment

🐛 Pending state when switching active token should not say 'No transactions yet'

@osvaldovega

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2019

I just create the follow up ticket for that part, here is the issue link #2156

@Efefefef

This comment has been minimized.

Copy link
Contributor

commented Jun 20, 2019

Good 👍

@Efefefef Efefefef added the ready label Jun 20, 2019

@osvaldovega osvaldovega merged commit 16d4736 into development Jun 20, 2019

4 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
coverage/coveralls Coverage remained the same at 94.736%
Details

@osvaldovega osvaldovega deleted the 1936-migrate-dashboard-to-the-new-design branch Jun 20, 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.