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

Issue 1872 - Antify Deposit/Withdraw Modals #2495

Merged
merged 5 commits into from
Mar 21, 2019

Conversation

startailcoon
Copy link
Contributor

  • Full refactor of DepositWithrawAssetSelector

Updates

  • AssetSelect updated to use correct asset name replacer
  • AmountSelect now works without label

General

Closes #1872

General

lease make sure the following is done:

Code Preparation

Please review all your changes one last time before committing

  • Check for unused code
  • No unrelated changes are included
  • None of the changed files are reformatting only
  • Code is self explanatory or documented
  • All written text is properly translated (english language)

Testing

The branch has been tested on the following browsers (desktop and mobile view)

  • Chrome
  • Opera
  • Firefox
  • Safari

User interface changes

Delete this section if there weren't any UI changes. Please make sure you tested your changes in all themes

  • Dark
  • Light
  • Midnight

Please provide screenshots/licecap of your changes below

withdrawdepositmodasmall

- Full refactor of DepositWithrawAssetSelector

Updates
- AssetSelect updated to use correct asset name replacer
- AmountSelect now works without label
This was referenced Mar 3, 2019
@sschiessl-bcp
Copy link
Contributor

Deposit and Withdraw Modals do not open from Menu anymore

@startailcoon
Copy link
Contributor Author

Strange, will investigate. Didn't see this issue, did you test it towards the recent change that fixed the issue with it?

@startailcoon
Copy link
Contributor Author

#2483

@startailcoon
Copy link
Contributor Author

Can't reproduce any issue with opening modals from menu.

withdrawdepositmodal_menu

Copy link
Contributor

@sschiessl-bcp sschiessl-bcp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Load http://localhost:8080/account/blockchainprojectsbv-test-2, go to dashboard, click deposit BTS
    image
    No barcode appears. Expectancy: A barcode with account name. Maybe out of scope

  • After having used deposit BTS, it also wants to deposit BTS for any other asset

  • please add padding when loading for deposit
    image

app/components/Modal/WithdrawModalNew.jsx Outdated Show resolved Hide resolved
app/lib/common/assetGatewayMixin.js Show resolved Hide resolved
app/lib/common/assetGatewayMixin.js Show resolved Hide resolved
@startailcoon
Copy link
Contributor Author

startailcoon commented Mar 11, 2019

No barcode appears. Expectancy: A barcode with account name. Maybe out of scope

This is due to some change made previously. We now use a component called <CryptoLinkFormatter> that formats the QR code link for various assets. Someone forgot to account for BTS not using the same format as Gateway assets. Fixed

please add padding when loading for deposit

Fixed

After having used deposit BTS, it also wants to deposit BTS for any other asset

Issue: Opening deposit modal from dashboard doesn't update deposit asset to the new one a user selects, but will retain the previously used one. A new issue will be created for this.

Added padding for loading address
@startailcoon
Copy link
Contributor Author

Sorry, must have missed posting my comments when resolving these. Both these notes are fixed as part of the following PR #2523.

@sschiessl-bcp
Copy link
Contributor

Please resolve the conflict, I'm unsure which is the newest
image

@startailcoon
Copy link
Contributor Author

Resolved @sschiessl-bcp

@sschiessl-bcp sschiessl-bcp merged commit 84dc9f0 into develop Mar 21, 2019
@sschiessl-bcp sschiessl-bcp deleted the 1872_DepositWithdrawModalAntification branch March 21, 2019 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants