Skip to content

Eth transfer #536

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.

Sign up for GitHub

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

Jump to bottom
Merged
phraktle merged 31 commits into from
Apr 26, 2019
Merged

Eth transfer #536

phraktle merged 31 commits into from
Apr 26, 2019

Conversation

@admrid
Copy link
Contributor

@admrid admrid commented Mar 8, 2019 •
edited
Loading

Nature of the PR: feature

Steps to reproduce:

go to /transfer and send A€ to an address that does not have ETH -not working yet -WIP

Trello card link:

https://trello.com/c/dYF3p7t7

Is connection necessary to test? If so which network?

  • local RPC
  • Rinkeby
  • Main Ethereum Network

@szerintedmi
Copy link
Member

szerintedmi commented Mar 8, 2019 •
edited
Loading

Deploy preview for augmint ready!

Built with commit 707c64f

https://deploy-preview-536--augmint.netlify.com

admrid added 17 commits March 8, 2019 16:33
@admrid
feat(ethTransfer, payeeEthBalance): init
9784557
@admrid
feat(reduxers): add reducers
57053f2
@admrid
fix(underthehood): delete hwwallet files
fa10730
@admrid
fix(staging): merge conflicts
b4819cc
@admrid
feat(EthTransferForm): pass around payee address
b43ad41
@admrid
fix(payeeEthBalance): delete unnec Promise
80073b9
@admrid
feat(payeeEthBalance): change reducer to module
1079ff5
@admrid
fix(transferTransaction): convert address toString()
075f273
@admrid
fix(): merge staging into
f10405c
@admrid
fix(deploy?): delete augmint-contracts
eba0ef2
@admrid
fix(EthtransferForm): change payee address to the correct one
a5e43d4
@admrid
fix(travis & cypress): change yarn to don't run record
4a2a538
@admrid
fix(EthereumTxStatus): add + && for receipt logic
b12da50
@admrid
fix(EthereumTxStatus): calc No.s of covered txs
c6b30e1
@admrid
fix(transfer/index): change EthTransferForm bgColor
cde494c
@admrid
fix(styles/theme): add & use secondaryXLight color
bb582a5
@admrid
fix(transfer/index): add minor changes to logic
838b3c8
@szerintedmi
Copy link
Member

szerintedmi commented Apr 17, 2019 •
edited
Loading

Looks really good!

Couple of questions / suggestions:

  • Any thoughts how it will work in conjunction with the delegated transfer?
  • copy should be shorter. How about something like:

Care to help?
The recipient doesn't have ETH. They will need some to spend A-EUR.
Send some to them.

  • Change approx to ≈
  • Why we don't have a default value entered? It's confusing that the tip in the field is an amount. I would have a default value there
  • maybe remove the block after closing the rtanscation confirmation ? Or any scenario when would do it 2x?

image

@admrid
Copy link
Contributor Author

admrid commented Apr 24, 2019

@szerintedmi i've done/changed all the things u mentioned.

regarding the delegated transfer: it seems that function is parked so i'll deal w it when it's coming

@admrid admrid marked this pull request as ready for review April 24, 2019 14:20
szerintedmi
szerintedmi previously approved these changes Apr 24, 2019
View reviewed changes
Copy link
Member

@szerintedmi szerintedmi left a comment

Choose a reason for hiding this comment

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

Great! Thanks

@szerintedmi szerintedmi self-requested a review April 24, 2019 17:41
phraktle
phraktle requested changes Apr 24, 2019
View reviewed changes
@phraktle phraktle merged commit 2020eb6 into staging Apr 26, 2019
@phraktle phraktle deleted the eth-transfer branch April 26, 2019 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phraktle phraktle phraktle approved these changes

@rszaloki rszaloki Awaiting requested review from rszaloki

@szerintedmi szerintedmi Awaiting requested review from szerintedmi

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@admrid @szerintedmi @phraktle