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

📚 Documentation: Tutorial for Next JS #179

Merged
merged 42 commits into from
Oct 30, 2023

Conversation

Mridul2820
Copy link

What does this PR do?

Added Tutorial for a functioning app with Next JS in the Docs

Test Plan

The Tutorial can be found in the route /docs/tutorials/nextjs/step-1

Related PRs and Issues

#80

Have you read the Contributing Guidelines on issues?

Yes

@vercel
Copy link

vercel bot commented Oct 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 27, 2023 6:08am

Copy link
Contributor

@gewenyu99 gewenyu99 left a comment

Choose a reason for hiding this comment

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

Another issue I see is that this is using Tailwind. We prefer very little styling or to use https://pink.appwrite.io/.

We prefer to not recommend style library in docs, which is out of scope.

Also pay attention to Titles and headings, should be sentence case

src/routes/docs/tutorials/nextjs/step-1/+page.markdoc Outdated Show resolved Hide resolved
static/images/docs/tutorials/expense-tracker.png Outdated Show resolved Hide resolved
src/routes/docs/tutorials/nextjs/step-4/+page.markdoc Outdated Show resolved Hide resolved
src/routes/docs/tutorials/nextjs/step-4/+page.markdoc Outdated Show resolved Hide resolved
@Mridul2820
Copy link
Author

Hi @gewenyu99 , I have added all the changes you asked for

  • Added all the images in 1400x900 viewport
  • Titles are now in sentence case
  • Changed Next JS to Next.js
  • Used https://pink.appwrite.io/. for styling and styles are minimal with the Appwrite theme
  • Increased the Read Time
  • Changed user to Walter O'Brien for screenshots

Here is the current preview of the project

expense-tracker

@gewenyu99
Copy link
Contributor

Hi @gewenyu99 , I have added all the changes you asked for

  • Added all the images in 1400x900 viewport
  • Titles are now in sentence case
  • Changed Next JS to Next.js
  • Used https://pink.appwrite.io/. for styling and styles are minimal with the Appwrite theme
  • Increased the Read Time
  • Changed user to Walter O'Brien for screenshots

Here is the current preview of the project

expense-tracker

Hi I meant these images should be uncropped. We have a tool that crops them or frames them consistently, can I have the originals please? Thanks! What ever the output of the screenshot tool from your browser gives you!

Thanks ❤️

Screen.Recording.2023-10-04.at.11.17.48.AM.mov

@Mridul2820
Copy link
Author

Hi @gewenyu99 I have added the raw screenshot images the way you showed. Let me know if this works.

@gewenyu99
Copy link
Contributor

@Mridul2820 Thank you so much.

Which browsers and operating system do you use? The links in the top nav of the console is blue for some reason, this appears to be a bug on our Console that we might need to fix.

I will retake these screenshots for you sometime later, because it doesn't look right on your screen (which is our fault :P)

Thanks!

@Mridul2820
Copy link
Author

I have used Mac M1 and chrome browser to take the screenshots

Let me know how we can fix the screenshot issue :)

@gewenyu99
Copy link
Contributor

I have used Mac M1 and chrome browser to take the screenshots

Let me know how we can fix the screenshot issue :)

That's really weird. The text color from the links are wrong, it should be white, not pink. No idea why they're wrong. I might have to take these screenshots myself, so it's out of your hand.

When I get some time I will be reviewing the writing in more detail, thanks!

@ArmanNik
Copy link
Member

ArmanNik commented Oct 5, 2023

Hey @Mridul2820 thank you for your contribution 🙌

I'm trying to figure out why the styles are not loading correctly for you. Do you get the same issue if you open the console on an incognito page?

@gewenyu99
Copy link
Contributor

Here's an idea @Mridul2820
https://github.com/appwrite/website/pull/275/files#diff-3780ba26d123aa2fd91e5a4906501aa4128fcaaa6f2c76c6e4f956de547cb84e

Take a look here, they explain the code in bits, but always include completed code at end of discussion, so it's easier to follow :)

@gewenyu99 gewenyu99 changed the base branch from main to hacktoberfest-accepted October 29, 2023 16:31
@gewenyu99 gewenyu99 changed the base branch from hacktoberfest-accepted to main October 29, 2023 16:41
@gewenyu99 gewenyu99 changed the base branch from main to hacktoberfest-accepted October 29, 2023 16:41
@gewenyu99
Copy link
Contributor

gewenyu99 commented Oct 29, 2023

@Mridul2820 I think this is a good contribution, but it's blocked by a couple of things.

Some of the blockers are just that Appwrite doesn't play amazingly with SSR, and you've chosen to go with SSR for auth, so we can't merge this until SSR is released.

Some other blockers are just the complexity of this project. I think I'll take this and work with some Appwrite engineers to reduce the scope of this app.

Some other issues as well. With SSR, the serverside client should really be using an Appwrite Server SDK. There shouldn't be a passing of userId into the server side via Post, it should be re-verified and fetching using an server side account.get with a JWT.

The permission configuration is also not correct, if you have more than one user, you can read all the users' data, which is insecure.

If you'd like to continue working on this, feel free, but I believe this contribution is valuable enough for us as is.

Thank you so much for your patience and hard work!

@gewenyu99 gewenyu99 marked this pull request as draft October 29, 2023 19:37
@Mridul2820
Copy link
Author

Cool @gewenyu99

I will remove the SSR and add useContext

Also, I found the same kind of configuration for users and documents in the sveltekit tutorial earlier

@gewenyu99
Copy link
Contributor

Cool @gewenyu99

I will remove the SSR and add useContext

Also, I found the same kind of configuration for users and documents in the sveltekit tutorial earlier

Yeah, I'll accept your PR for hacktoberfest and keep it open for now.

We will add SSR to Appwrite properly, so we can take advantage of the amazing app you built.

Don't change your app for now, I think using SSR is correct way to use Next.js, the problem in on our end, and we'll work to address it.

I've marking this PR hacktoberfest-accepted so you'll get a valid contribution out of this.

Thanks so much!

@gewenyu99 gewenyu99 changed the base branch from hacktoberfest-accepted to feat-next-js-tutorial October 30, 2023 21:20
@gewenyu99 gewenyu99 marked this pull request as ready for review October 30, 2023 21:21
Copy link
Contributor

@gewenyu99 gewenyu99 left a comment

Choose a reason for hiding this comment

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

For the purpose of this Hacktoberfest contribution, this is ready to be merged into a branch. I'll be taking over with the team to make some final touches when we have better SSR support and release this tutorial.

@gewenyu99 gewenyu99 merged commit 105126f into appwrite:feat-next-js-tutorial Oct 30, 2023
2 checks passed
@Mridul2820
Copy link
Author

Mridul2820 commented Oct 31, 2023

Great @gewenyu99 , Thanks :)

@tessamero tessamero linked an issue Nov 2, 2023 that may be closed by this pull request
2 tasks
@gewenyu99
Copy link
Contributor

Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship.

Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag.

@Mridul2820
Copy link
Author

Discord username

@gewenyu99 Here is my discord username mridul2820

@gewenyu99
Copy link
Contributor

Reaching out soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📚 Documentation: Tutorial for Next JS
5 participants