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

feat: add example react app to test wallet plugin #3

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Conversation

krzysu
Copy link
Contributor

@krzysu krzysu commented Oct 22, 2024

@krzysu krzysu requested a review from jdevcs October 22, 2024 13:20
@krzysu krzysu force-pushed the feat-example-app branch 2 times, most recently from 0e0fa84 to 338a5d4 Compare October 22, 2024 13:34
@krzysu
Copy link
Contributor Author

krzysu commented Oct 23, 2024

I actually transitioned to a monorepo in a follow-up PR #5

Copy link
Contributor

@danforbes danforbes left a 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
Copy link
Contributor

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?

Copy link
Contributor Author

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

Comment on lines +1 to +16
# 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.
Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

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

Comment on lines +2 to +4
"name": "web3js-react-dapp-demo",
"version": "0.1.0",
"description": "Demonstration Web3.js + React dApp",
Copy link
Contributor

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"
Copy link
Contributor

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>
Copy link
Contributor

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

Comment on lines +2 to +3
"short_name": "Web3.js + React dApp",
"name": "Web3.js + React Demonstration dApp",
Copy link
Contributor

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>
Copy link
Contributor

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!

@danforbes
Copy link
Contributor

Why doesn't the example dApp actually use the plugin?

@krzysu krzysu force-pushed the feat-init-setup branch 5 times, most recently from e62782d to 32f2226 Compare October 24, 2024 12:38
Base automatically changed from feat-init-setup to main October 24, 2024 12:39
@krzysu
Copy link
Contributor Author

krzysu commented Oct 24, 2024

@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.

@krzysu krzysu merged commit 4fbe3c0 into main Oct 24, 2024
3 checks passed
@krzysu krzysu deleted the feat-example-app branch October 24, 2024 12:55
@danforbes danforbes mentioned this pull request Oct 24, 2024
This was referenced Oct 31, 2024
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.

2 participants