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 form step 1/3 of send BTC - Closes #1889 #2017

Merged
merged 17 commits into from May 16, 2019

Conversation

Projects
None yet
2 participants
@massao
Copy link
Contributor

commented May 15, 2019

What issue have I solved?

#1889

How have I implemented/fixed it?

  • Removed unnecessary HOC from the form;
  • Updated form to get activeToken from store, if localStorage has the 'btc' key set;
  • Adjusted bookmarks to only show bookmarks with same token as active token;
  • Adjusted address validation to be token based;
  • Adjusted amount validation to be based on the balance of the active token;
  • Implemented Dynamic Fee for BTC;
  • Created Selector component to be easier to reuse on the future;

Know Issues

  • At the moment we don't have an API to provide the "estimated processing time", so it is not displayed.
  • We currently don't have the BTC/LSK switcher on the send, we are considering that the token is already set before going to the send page.

How has this been tested?

  • Go to /wallet and go to send page;
  • It should be exactly the same as before;
  • Set localStorage.setItem('btc', true);;
  • Update the active token to 'BTC':
    • $r.store.dispatch({type:'SETTINGS_UPDATE_TOKEN', data: { token: { active: 'BTC', list: {BTC: true, LSK: true} }}}) Note that React dev tools have to be active for $r to be defined.
    • Or dispatching {type:'SETTINGS_UPDATE_TOKEN', data: { token: { active: 'BTC', list: {BTC: true, LSK: true} }}} through Redux DevTools.
  • The page should show the recipient and amount fields, and after inputing the amount, should fetch the dynamic Fee, and display it on the page.

Review checklist

massao added some commits May 15, 2019

@massao massao self-assigned this May 15, 2019

massao added some commits May 15, 2019

@massao massao requested a review from slaweet May 15, 2019

@slaweet
Copy link
Member

left a comment

I pushed commits to solve these:

  • 🐛 The error mesage was saying "LSK" even for BTC Screenshot 2019-05-16 at 09 03 18
  • 🐛 Wallet page without BTC enabled had "cannot read balance of undefined" error, which caused e2e tests to fail.

massao added some commits May 16, 2019

@massao massao force-pushed the 1889-implement-form-1-3-send-BTC branch from 75cbdce to 720f0a9 May 16, 2019

@slaweet slaweet force-pushed the 1889-implement-form-1-3-send-BTC branch from 5e3991c to 3f0efdf May 16, 2019

@slaweet
Copy link
Member

left a comment

Thank you @massao

@slaweet slaweet added the ready label May 16, 2019

@massao massao merged commit db3a2eb into development May 16, 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.723%
Details

@massao massao deleted the 1889-implement-form-1-3-send-BTC branch May 16, 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.