-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add example react app to test wallet plugin #3
Conversation
0e0fa84
to
338a5d4
Compare
I actually transitioned to a monorepo in a follow-up PR #5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nitpicky comments about updating boilerplate. Glad the template can be helpful!
@@ -0,0 +1,21 @@ | |||
MIT License | |||
|
|||
Copyright (c) 2024 web3js-react-dapp-demo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it important to change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed example app license as I don't think it is relevant here, I kept plugin license in the repo's root
# Web3.js + React Demonstration dApp | ||
|
||
This project is a demonstration dApp built with [Web3.js](https://web3js.org/) | ||
and [React](https://react.dev/). | ||
|
||
- [Web3.js Docs](https://docs.web3js.org/) | ||
- [React Docs](https://react.dev/learn) | ||
|
||
This project was bootstrapped with | ||
[Create React App](https://github.com/facebook/create-react-app). | ||
|
||
## Getting Started | ||
|
||
Install the project dependencies with a dependency manager like npm or Yarn. Use | ||
`npm start` to start a local development server and navigate to | ||
http://localhost:3000 to view the dApp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to update this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would you like to update here? the example app's purpose is to allow for manual tests of the wallet rpc plugin with different extension wallets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created this PR #10
"name": "web3js-react-dapp-demo", | ||
"version": "0.1.0", | ||
"description": "Demonstration Web3.js + React dApp", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would update the name and description here
<meta name="theme-color" content="#000000" /> | ||
<meta | ||
name="description" | ||
content="Web3.js + React demonstration dApp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to update
work correctly both with client-side routing and a non-root public URL. | ||
Learn how to configure a non-root public URL by running `npm run build`. | ||
--> | ||
<title>Web3.js + React Demonstration dApp</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to update
"short_name": "Web3.js + React dApp", | ||
"name": "Web3.js + React Demonstration dApp", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to update
|
||
return ( | ||
<main> | ||
<h1>Web3.js + React Minimal dApp Template</h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to change this in the template!
Why doesn't the example dApp actually use the plugin? |
e62782d
to
32f2226
Compare
Signed-off-by: Kris Urbas <[email protected]>
338a5d4
to
244812b
Compare
@danforbes thanks for the review, I will apply all your comments in a follow up PR #5 as I'm afraid the changes might get lost as I restructured a lot of files later. The usage of a plugin is coming in later PRs as well. |
based on Dan's https://github.com/web3/create-web3js-dapp