Skip to content
This repository has been archived by the owner on Jun 29, 2020. It is now read-only.

(Feature) Show spinner when confirming and removing addresses #116

Merged
merged 2 commits into from
Apr 25, 2018

Conversation

fvictorio
Copy link
Contributor

After confirming or removing an address, wait for the transaction to be mined before hiding the spinner.

This is not a big change, but it seems to add some level of async that made a lot of test fail. I added a bunch of setTimeouts to make them pass (so that the expects are called after every async funtion was called), but I'm not a fan of this solution. Please let me know if you know a better way to do this.

@ghost ghost assigned fvictorio Apr 23, 2018
@ghost ghost added the in progress label Apr 23, 2018
@coveralls
Copy link

Pull Request Test Coverage Report for Build 368

  • 29 of 34 (85.29%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 74.206%

Changes Missing Coverage Covered Lines Changed/Added Lines %
web-dapp/src/components/MyAddressesPage.js 3 5 60.0%
web-dapp/src/components/ConfirmationPage.js 26 29 89.66%
Totals Coverage Status
Change from base Build 366: -0.05%
Covered Lines: 881
Relevant Lines: 1136

💛 - Coveralls

@phahulin
Copy link
Contributor

@fvictorio I get strange errors on the last step when deploying contracts to ganache via ./node_modules/.bin/truffle migrate

Saving artifacts...

/.../poa-popa/blockchain/node_modules/web3/lib/web3/errors.js:35
        return new Error(message);
               ^
Error: Invalid JSON RPC response: ""

could you check if you get the same error?

@fvictorio
Copy link
Contributor Author

No, that's not happening to me.

Maybe you can try removing all node_modules directories (rm node_modules blockchain/node_modules web-dapp/node_modules) and re-installing (just one npm install from the root of the project should re-install everything)?

@fvictorio
Copy link
Contributor Author

@phahulin Were you able to make it work?

@phahulin phahulin merged commit 963198d into master Apr 25, 2018
@phahulin phahulin deleted the show-spinners branch April 25, 2018 09:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants