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

(Fix) Update DApp loading code considering upcoming Breaking Change in MetaMask (Branch Master) #1171

Merged
merged 10 commits into from
Oct 26, 2018

Conversation

mariano-aguero
Copy link
Contributor

@mariano-aguero mariano-aguero commented Oct 12, 2018

Closes #1165 .

Modify the "MetaMask not found" error page to display a generic error text stating "Wallet not found, or access to Ethereum account not granted"

This is for the master branch, there will be another PR for the branch 2.0

screenshot_20181012_152956

@coveralls
Copy link

coveralls commented Oct 12, 2018

Pull Request Test Coverage Report for Build 3263

  • 16 of 39 (41.03%) changed or added relevant lines in 2 files are covered.
  • 625 unchanged lines in 18 files lost coverage.
  • Overall coverage increased (+4.05%) to 25.883%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/stores/Web3Store.js 4 12 33.33%
src/react-web3/Web3Provider.js 12 27 44.44%
Files with Coverage Reduction New Missed Lines %
src/components/Common/StepNavigation.js 1 66.67%
src/components/stats/index.js 1 0.0%
src/components/stepOne/index.js 1 8.11%
src/react-web3/Web3Provider.js 1 36.47%
src/components/Common/WhenFieldChanges.js 2 14.29%
src/components/stepThree/GasPriceInput.js 2 0.0%
src/components/stepThree/StepThreeForm.js 2 12.5%
src/components/stepTwo/index.js 2 7.41%
src/components/Common/TierBlock.js 3 9.68%
src/components/Common/TxProgressStatus.js 3 0.0%
Totals Coverage Status
Change from base Build 3099: 4.05%
Covered Lines: 736
Relevant Lines: 2944

💛 - Coveralls

Copy link

@dennis00010011b dennis00010011b left a comment

Choose a reason for hiding this comment

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

@mariano-aguero
Manage page isn't fully rendered unless clicking button 'Finalize'

screen shot 2018-10-15 at 9 53 49 am

Steps:

  1. Create crowdsale: Minted, 1 tier, no whitelist
  2. Open Manage page, observe(see screenshot)
  3. Click button 'Finalize'-page is rendered after that

@mariano-aguero mariano-aguero changed the title (Fix) Update DApp loading code considering upcoming Breaking Change in MetaMask (Fix) Update DApp loading code considering upcoming Breaking Change in MetaMask (Branch Master) Oct 15, 2018
@vbaranov
Copy link
Collaborator

@mariano-aguero

Double dots:
screen shot 2018-10-16 at 13 54 55

and let's change the link to Token Wizard wiki

@mariano-aguero
Copy link
Contributor Author

@vbaranov

Double dots:
and let's change the link to Token Wizard wiki

Done!

@mariano-aguero
Copy link
Contributor Author

@dennis00010011b

Manage page isn't fully rendered unless clicking button 'Finalize'

It is not an error related to the changes made it, anyway I solved in this branch

Regards

@mariano-aguero
Copy link
Contributor Author

@dennis00010011b

Do not merge until @fernandomg do the code review

Copy link
Contributor

@fernandomg fernandomg left a comment

Choose a reason for hiding this comment

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

@mariano-aguero
Everything is working fine, but there are a couple of things that I'm not completely sure about.

src/react-web3/Web3Provider.js Show resolved Hide resolved
this.setState({
web3: window.web3
})
this.fetchAccounts()
Copy link
Contributor

Choose a reason for hiding this comment

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

Here fetchAccounts is being called, but it's called aswell inside the interval defined in the line 91, which is called after calling checkWeb3, in the line 40

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fernandomg

Removed the interval

src/react-web3/Web3Provider.js Show resolved Hide resolved
this.setState({
approvePermissions: false
})
console.log(err.message)
Copy link
Contributor

Choose a reason for hiding this comment

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

logger

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fernandomg
If my memory doesn't fail me, logger is used in the branch 2.0, please correct me if I'm wrong

Greetings

@dennis00010011b
Copy link

I can't get "MetaMask not found" error page with local build. As a result app crashed on Step 2.
https://www.useloom.com/share/8d421012f09940f291ad75c79fb91015

screen shot 2018-10-19 at 10 35 48 am

@mariano-aguero
Copy link
Contributor Author

@dennis00010011b

I can't get "MetaMask not found" error page with local build. As a result app crashed on Step 2.

Fixed

@fernandomg fernandomg merged commit 758c842 into master Oct 26, 2018
@ghost ghost removed the awaiting for review label Oct 26, 2018
@fernandomg fernandomg deleted the issue#1165 branch October 26, 2018 17:09
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.

5 participants