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

(docs) Fixed the main frontend path in README #808

Merged
merged 3 commits into from
Apr 13, 2024
Merged

(docs) Fixed the main frontend path in README #808

merged 3 commits into from
Apr 13, 2024

Conversation

kkomelin
Copy link
Contributor

Description

Previously the main frontend path was nextjs/pages. Now the project switched to the Next.js App Router and the nextjs/app folder correspondingly. We need to update the README correspondingly.

Additional Information

Copy link
Collaborator

@Pabl0cks Pabl0cks left a comment

Choose a reason for hiding this comment

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

Good catch @kkomelin!! We updated it in docs but totally missed README!

In docs we changed the "Edit your frontend" line for:

  • Edit your frontend homepage at packages/nextjs/app/page.tsx. For guidance on routing and configuring pages/layouts checkout the Next.js documentation.

Maybe we could use same explanation?

@rin-st
Copy link
Collaborator

rin-st commented Apr 10, 2024

In docs we changed the "Edit your frontend" line for:

Link

@Pabl0cks what do you think about adding "Edit your smart contract tests" too? (hh part)

@kkomelin
Copy link
Contributor Author

kkomelin commented Apr 10, 2024

@Pabl0cks @rin-st Thanks for your time reviewing my PR. Feel free to edit my PR the way you think is reasonable.

@Pabl0cks
Copy link
Collaborator

@Pabl0cks what do you think about adding "Edit your smart contract tests" too? (hh part)

Yeah we can add it! Would love to check if it was not added for simplicity or if it's cool to add it with @carletex or @technophile-04

Maybe we can format it similarly to the docs as:

What's next

  • Edit your smart contract YourContract.sol in packages/hardhat/contracts
  • Edit your frontend homepage at packages/nextjs/app/page.tsx. For guidance on routing and configuring pages/layouts checkout the Next.js documentation.
  • Edit your deployment scripts in packages/hardhat/deploy
  • Edit your smart contract test in: packages/hardhat/test. To run test use yarn hardhat:test

@technophile-04
Copy link
Collaborator

if it was not added for simplicity

Not remember completely but might be the reason, since we already had docs link below it

But thinking a bit maybe we could add it what @Pabl0cks suggested (since we already have most of the points already present)

Maybe we should keep "What's next" small (maybe bold) but not as heading

Something like :

Demo :

Screenshot 2024-04-12 at 9 59 01 PM

Code :
**What's next**:

- Edit your smart contract `YourContract.sol` in `packages/hardhat/contracts`
- Edit your frontend homepage at `packages/nextjs/app/page.tsx`. For guidance on [routing](https://nextjs.org/docs/app/building-your-application/routing/defining-routes) and configuring [pages/layouts](https://nextjs.org/docs/app/building-your-application/routing/pages-and-layouts) checkout the Next.js documentation.
- Edit your deployment scripts in `packages/hardhat/deploy`
- Edit your smart contract test in: `packages/hardhat/test`. To run test use `yarn hardhat:test`

@kkomelin could you please update it?

@Pabl0cks
Copy link
Collaborator

Maybe we should keep "What's next" small (maybe bold) but not as heading

Yes! Totally. After sending my sample I was like.. maybe too big 😅

@kkomelin
Copy link
Contributor Author

I'm on it guys, will update soon.

@kkomelin
Copy link
Contributor Author

kkomelin commented Apr 13, 2024

@technophile-04 @Pabl0cks Just applied the changes you suggested. Please review

@kkomelin kkomelin requested a review from Pabl0cks April 13, 2024 05:26
Copy link
Collaborator

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

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

Thanks @kkomelin ! and also thanks all for great suggestions !

@technophile-04 technophile-04 merged commit 8e06998 into scaffold-eth:main Apr 13, 2024
1 check passed
@kkomelin
Copy link
Contributor Author

Thanks everyone for helping with that!

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.

None yet

4 participants