search

brownie-mix / react-mix

Everything you need to use React with Brownie!
62 stars 19 forks source link

Repo rework: simple functional component version, works on private and public testnets, clearer readme #9

Closed jmcook1186 closed 3 years ago

jmcook1186 commented 3 years ago

Hello,

I had a hack at this repo because I thought it could be a bit cleaner and easier to follow for those looking for a simple react template to build from. So, I am proposing a new App.js with what I think is an easier-to-follow structure.

It works on a local Ganache instance as well as across the public testnets and works well with MetaMask. I updated the README with detailed usage instructions.

This version takes the 'functional component' style in App.js, resolving issue #8 (although I understand if class style is preferred) and only uses the Solidity contract from the original repo.

Hope this is helpful - if not no hard feelings if the PR is not merged!

Cheers

PatrickAlphaC commented 3 years ago

You and I think alike! I've also been meaning to give this repo a rework. I'll take a look into the code.

I think ideally, we move to a typescript & useDapp model with hooks instead of the class-based react. I'm torn between typescript and JS for this though. What do you think?

jmcook1186 commented 3 years ago

You're the expert! But I do think K.I.S.S. is the right guiding principle for this mix. I went with a JS functional component version because I thought it was the simplest, most understandable way to bootstrap a minimal front-end.

PatrickAlphaC commented 3 years ago

Not a front end expert sadly :( Just a smart contracts expert ahah.

I agree, K.I.S.S is the best! But we also want the best practices.

We'll merge, since it looks good, I love the little changes you added! But ideally we move to at least hooks at some point (can leave typescript off)

Thank you so much for this PR!!

PatrickAlphaC commented 3 years ago

Ah we will have to revert, I was too optimistic, we need the vyper pieces in here.

PatrickAlphaC commented 3 years ago

I did a force push to just put it back where it was before this PR. We need to keep vyper in here, vyper is a multi-language tool!

jmcook1186 commented 3 years ago

ah ok, sure. Should I add the Vyper content back in and raise a new PR?

PatrickAlphaC commented 3 years ago

If you'd like 🙏

At some point I'll go back and add hooks instead, I think hooks are becoming the web standard, and are really effective for working with events and notifications with web3.