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.

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

Interact with contracts flow #2888

Merged
merged 57 commits into from
Dec 16, 2019
Merged

Interact with contracts flow #2888

merged 57 commits into from
Dec 16, 2019

Conversation

PeterZlodej
Copy link
Contributor

[ch2490]
[ch3419]
[ch3412]
[ch3413]
[ch3420]
[ch3422]

PeterZlodej and others added 30 commits October 29, 2019 08:10
@PeterZlodej
General steps layouts and routes setup
bb5c6b9
@PeterZlodej
Added network selector to interact step
63197c3
@PeterZlodej
Add contract selection and abi fields to form
0a6992d
@PeterZlodej
Contract selection logic
5369f02
@PeterZlodej
Use state factory for handling flow states
4669910
@PeterZlodej
Auto-select existing contract on address change
62291d8
@PeterZlodej
Created/updated contract & function dropdowns
6cba8d4
@PeterZlodej
Generate basic interaction form from the ABI
95801df
@PeterZlodej
Handle cases when input name is missing
1fa056b
@PeterZlodej
Handle interaction form submit
ae1be37
@PeterZlodej
Implemented read contract calls
1264090
@PeterZlodej
Added basic error messaging
7f9bb20
@PeterZlodej
Make dropdowns searchable
5262de4
@PeterZlodej
merge from master
81d704a
Merge branch 'interact-with-contracts-flow' of https://github.com/Pet…
5bc2706 
…erZlodej/MyCrypto into interact-with-contracts
Move SigningComponents to global types
29e59af
Initial implementation of write command for interacting with contracts
a973558
Update translations
5c017b6
@PeterZlodej
Updated styles for input/output fields
ea341ea
@PeterZlodej
Merge signing flow for write operations
f6c2876
@PeterZlodej
Updated generated form styles
8fad9bc
@PeterZlodej
Output bool field renamed
62ed9c4
Merge branch 'interact-with-contracts-flow' of https://github.com/Pet…
c78494c 
…erZlodej/MyCrypto into interact-with-contracts
@PeterZlodej
Created confirm interaction and receipt steps
e7343ab
@PeterZlodej
Added gas price, and limit fields to write
ede722d
@PeterZlodej
Added error messaging for write operation
8624a3e
@PeterZlodej
Add value field to payable functions
dde8176
@PeterZlodej
Use dropdown for inputing boolean values
e84baf1
Merge branch 'interact-with-contracts-flow' of https://github.com/Pet…
223697a 
…erZlodej/MyCrypto into interact-with-contracts
Use correct eth value, fix confirm screen data
c4773bc
@mycrypto-bot
Copy link
Collaborator

Staging build: MyCryptoBuilds

@coveralls
Copy link

coveralls commented Dec 5, 2019 •
edited

Coverage Status

Coverage increased (+0.03%) to 48.075% when pulling 7bbd9be on PeterZlodej:interact-with-contracts-flow into 5837a04 on MyCryptoHQ:master.

blurpesec
blurpesec reviewed Dec 5, 2019
View reviewed changes
Copy link
Collaborator

@blurpesec blurpesec left a comment

Choose a reason for hiding this comment

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

On the Read / Write Contract dropdown, if there are a lot of selections it can overflow the end of the page and expand it past the footer. Not sure how we expect to this act:
image

@PeterZlodej PeterZlodej marked this pull request as ready for review December 9, 2019 07:29
@blurpesec blurpesec self-requested a review December 9, 2019 23:26
blurpesec
blurpesec reviewed Dec 9, 2019
View reviewed changes
Copy link
Collaborator

@blurpesec blurpesec left a comment

Choose a reason for hiding this comment

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

I haven't experienced any usability issues with this after using it for a while to do random things.

vilsbole
vilsbole suggested changes Dec 10, 2019
View reviewed changes
common/v2/services/Store/Contract/ContractProvider.tsx Outdated Show resolved Hide resolved
common/v2/services/Store/Contract/ContractProvider.tsx Show resolved Hide resolved
common/v2/services/EthService/network/providerHandler.ts Show resolved Hide resolved
common/v2/services/EthService/contracts/ABIFunction.ts Show resolved Hide resolved
common/v2/services/ApiService/Etherscan/constants.ts Outdated Show resolved Hide resolved
common/v2/features/InteractWithContracts/helpers.ts Outdated Show resolved Hide resolved
common/v2/features/InteractWithContracts/stateFactory.tsx Outdated Show resolved Hide resolved
common/v2/features/InteractWithContracts/stateFactory.tsx Outdated Show resolved Hide resolved
common/v2/features/InteractWithContracts/stateFactory.tsx Outdated Show resolved Hide resolved
common/v2/features/InteractWithContracts/stateFactory.tsx Outdated Show resolved Hide resolved
vilsbole
vilsbole approved these changes Dec 16, 2019
View reviewed changes
Copy link
Contributor

@vilsbole vilsbole left a comment

Choose a reason for hiding this comment

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

Last week I've been encountering this bug.
Cannot set state on an unmounted component
https://www.robinwieruch.de/react-warning-cant-call-setstate-on-an-unmounted-component
This happens when, in an Effect we wait for a Promise then call setState.

I've been solving this issue by using the usePromise pattern which I copie into the project.
I'm merging this now and we can use the new hooks once they are accepted

@vilsbole vilsbole merged commit 51fa33a into MyCryptoHQ:master Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blurpesec blurpesec blurpesec left review comments

@vilsbole vilsbole vilsbole approved these changes

@FrederikBolding FrederikBolding Awaiting requested review from FrederikBolding

@amacar amacar Awaiting requested review from amacar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@PeterZlodej @mycrypto-bot @coveralls @FrederikBolding @vilsbole @amacar @blurpesec