-
Notifications
You must be signed in to change notification settings - Fork 182
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 TypeScript #269
base: main
Are you sure you want to change the base?
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.
Hey, great job, some changes are required. The Ideas interface you defined is inconsistent used. Some places it's used with the $id
field, some places it's declared without the $id
field.
Take a look, make sure the code works.
const title = 'Tutorials' + DOCS_TITLE_SUFFIX; | ||
const description = DEFAULT_DESCRIPTION; | ||
const ogImage = DEFAULT_HOST + '/images/open-graph/docs.png'; | ||
const title = 'Tutorials' + DOCS_TITLE_SUFFIX; |
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.
Can you remove all the white space changes?
import { DOCS_TITLE_SUFFIX } from '$routes/titles'; | ||
import { MainFooter } from '$lib/components'; | ||
import { DEFAULT_DESCRIPTION, DEFAULT_HOST } from '$lib/utils/metadata'; | ||
import { DOCS_TITLE_SUFFIX } from '$routes/titles'; |
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.
Why were these changed?
|
||
``` | ||
|
||
Then, optionally render the `Login` component if the path is `/login`, otherwise render the `Home` component. |
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.
Why optionally?
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.
Since we have given the steps to design 2 Login components, the user can choose which one suits their project the best.
step: 5 | ||
--- | ||
|
||
In our app we want to have a navigation bar that is always visible. We will add it to the `App` component and use the `useUser` hook to show either: |
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.
Can we also navigate to the homepage automatically if already logged in?
It's confusing without this behavior based on how you built this app 👀
import { databases } from "../appwrite"; | ||
import { ID, Query } from "appwrite"; | ||
|
||
export const IDEAS_DATABASE_ID = "<YOUR_DATABASE_ID>"; // Replace with your database ID |
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.
This should be defined and init in appwrite.js
ID.unique(), | ||
idea | ||
); | ||
setIdeas((currentIdeas) => [response.$id, ...currentIdeas].slice(0, 10)); |
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.
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'm also facing the same Issue... Can you please guide on how to fix this?
Hey, When it is merged, we'll contact you for Appwrite-specific Hacktoberfest swag. Thanks for helping us improve Appwrite! |
Thank you so much for adding the label. I can assure you I'm working on fixing all these issues and will make a PR as soon as possible. |
Hello @PrerakMathur20 , Thank you for your contribution to Hacktoberfest 2023! We've noticed that your PR is still pending and requires some updates based on our engineering team's feedback. We would love to see your PR successfully merged and send you the Appwrite swag as a token of appreciation. To remain eligible for the swag, please address the pending suggestions and/or ensure the tests pass by Friday, November 17th. If the PR isn't updated by then, we will unfortunately have to close it due to the end of the Hacktoberfest event. Looking forward to your updates and thank you! |
@PrerakMathur20 is attempting to deploy a commit to the appwrite Team on Vercel. A member of the Team first needs to authorize it. |
@gewenyu99 Thanks a lot for all the insightful reviews. My apologies for the delay in getting back to them. To create the TypeScript tutorial, I have taken inspiration from the React Tutorial, since they're not that different when it comes to converting JS to TS. Here For all the reviews that I haven't resolved yet, all of them are inspired by the tutorial of React. The same goes for the |
What does this PR do?
Provides step-by-step tutorial to build an WebApp with React TypeScript using AppWrite
Test Plan
Pure static documentation. Tested and Verified Locally.
Home
->Docs
->Tutorials
->TypeScript
Related PRs and Issues
Closes 📚 Documentation: Tutorial for Appwrite with Typescript
Have you read the Contributing Guidelines on issues?
Yes.