-
Notifications
You must be signed in to change notification settings - Fork 204
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
📚 Documentation: Tutorial for Next JS #179
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
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
Hi @gewenyu99 , I have added all the changes you asked for
Here is the current preview of the project |
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 |
Hi @gewenyu99 I have added the raw screenshot images the way you showed. Let me know if this works. |
@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! |
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! |
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? |
Here's an idea @Mridul2820 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 :) |
…into nextjs-tutorial
@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! |
Cool @gewenyu99 I will remove the SSR and add Also, I found the same kind of configuration for users and documents in the |
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! |
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.
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.
Great @gewenyu99 , Thanks :) |
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. |
@gewenyu99 Here is my discord username mridul2820 |
Reaching out soon! |
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