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

Update payload.mdx #2821

Merged
merged 23 commits into from
May 3, 2023
Merged

Conversation

isamardzija
Copy link
Contributor

What kind of changes does this PR include?

  • New or updated content

Description

Expanded the stub with instructions how to make a new PayloadCMS collection, fetch it and build an Astro blog with it. Discord thread: https://discord.com/channels/830184174198718474/1083317986380742686

Expand the stub with instructions how to make a new PayloadCMS collection, fetch it and build an Astro blog with it.
@netlify
Copy link

netlify bot commented Mar 9, 2023

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 01d2730
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/64524562e2a79b0008466eac
😎 Deploy Preview https://deploy-preview-2821--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sarah11918 sarah11918 added the add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. label Mar 9, 2023
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Wow, @isamardzija this is looking great! 🥳

You've done a wonderful job of using our existing guides as models, and your instructions are really clear and nicely written. We're so happy to have received this contribution!

I've started the review ball rolling with some comments and suggestions below. Usually @Jutanium and/or @kevinzunigacuellar would take these instructions for a proper spin playing the role of someone with no experience trying to complete the task.

But, I added some thoughts while giving this a quick read through for them to consider as they go. We'll dive into this a little more this coming week, and I'm so excited to have this resource in docs! 🚀

src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
});
```

If you then go into the PayloadCMS Dashboard, you should see a new collection called "Posts" appear next to the "Users" collection. Enter the "Posts" collection and create a new post. After saving it, you will notice the API URL appear in the bottom right corner.
Copy link
Member

Choose a reason for hiding this comment

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

Just pointing out that I'm not sure where the "skippable steps" stop here. If someone chose a template (e.g. blog) would n original post have been created, so this step of creating a new post should also be skipped?

It also seems like the note about both services using port 3000 should not be in the "skipped" section... so I think somewhere here we might need a new subheading so it's more clear where a reader with a template should jump back in to the instructions?

(Note: I did make an earlier suggested edit that took out the instruction to "skip ahead", but then I started the next instruction with "If you didn't choose a template..." So the idea is still the same: we need to make it clear when the "skip/if you choose a template" steps stop so that people are clear about where to jump back in.)

Copy link
Member

Choose a reason for hiding this comment

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

Excellent point here, most of these steps can be skipped if the blog template is selected. Personally, I used the UI to create new content

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally, I used the UI to create new content

I think we are talking about the same thing. Do you mean the PayloadCMS Dashboard when you say "UI"?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the confusion, yes I am talking about the Dashboard UI

Comment on lines 97 to 99
If not already running, start the server using `npm run dev` again.

Open `http://localhost:3000/api/posts` in your browser. You should see a JSON file containing the post you have created as an object.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If not already running, start the server using `npm run dev` again.
Open `http://localhost:3000/api/posts` in your browser. You should see a JSON file containing the post you have created as an object.
With the dev server running, open `http://localhost:3000/api/posts` in your browser. You should see a JSON file containing the post you have created as an object.

Copy link
Member

Choose a reason for hiding this comment

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

One thing that I noticed about the rest endpoint is going to need its own type parser to convert objects to html nodes

src/content/docs/en/guides/cms/payload.mdx Show resolved Hide resolved
---
import HomeLayout from "../layouts/HomeLayout.astro";

const res = await fetch("http://localhost:5000/api/posts") // https://localhost:3000/api/posts by default
Copy link
Member

Choose a reason for hiding this comment

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

one of these is https and the other is http and I'm assuming they should be the same?

src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
Copy link
Member

@kevinzunigacuellar kevinzunigacuellar left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I added some comments that I noticed when I tried to spin up a blog template.

I think we should include a section on how to customize a collection or reference their docs site.

Comment on lines 97 to 99
If not already running, start the server using `npm run dev` again.

Open `http://localhost:3000/api/posts` in your browser. You should see a JSON file containing the post you have created as an object.
Copy link
Member

Choose a reason for hiding this comment

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

One thing that I noticed about the rest endpoint is going to need its own type parser to convert objects to html nodes

src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
});
```

If you then go into the PayloadCMS Dashboard, you should see a new collection called "Posts" appear next to the "Users" collection. Enter the "Posts" collection and create a new post. After saving it, you will notice the API URL appear in the bottom right corner.
Copy link
Member

Choose a reason for hiding this comment

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

Excellent point here, most of these steps can be skipped if the blog template is selected. Personally, I used the UI to create new content

@sarah11918
Copy link
Member

Hey @isamardzija ! I can see that Kevin has also now gone through and added some comments for you! 🥳

No rush, and I don't want this to look overwhelming 😅 ! When you're ready, you can come back and address these in bits and pieces over time. No need to fix everything at once! If you have any questions for either Kevin or I based on our comments, or need any pointers for using GitHub's interface to "accept/commit" suggestions or "resolve" issues when you're happy with whatever changes you made in response to a suggestion, just let us know!

This was a HUGE PR, so that's why there are lots of comments! This is normal, and it's normal that it takes a while to figure out all the little things. It's much different than a typo fix or something. 😂 So again, don't get discouraged by what looks like a lot of work to do. Take it one comment at a time and we'll make this a super guide for the docs! 🚀

isamardzija and others added 4 commits March 21, 2023 19:00
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@sarah11918
Copy link
Member

Just checking in, @isamardzija ! I see you've jumped in and made a lot of changes, and I think there are still some open comments here.

So, just to help keep me organized, ping me again when you've had a chance to make all the changes you want to make, or if you have questions about any of the open comments, so I can look at this fresh again! Reminder, no rush! I just want to make sure I know when I should come back to this! 😄

@isamardzija isamardzija marked this pull request as draft March 27, 2023 18:26
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Yay! More comments! I think I can see what makes sense to eat now. See if these suggestions make sense to you, @isamardzija !

src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@sarah11918
Copy link
Member

Hi @isamardzija ! I'm going to commit some changes here and see if we can get moving a little more quickly on this one! I know I have a lot of comments, and that makes it tricky to look at. So let me make some changes directly and see if we can smooth it out. 😄

@sarah11918 sarah11918 marked this pull request as ready for review May 2, 2023 13:47
@sarah11918
Copy link
Member

Ok everyone!

I made some executive decisions and streamlined the earlier part, emphasizing use a template (since that's the happy path) and added notes about needing to do things manually if you didn't use a template.

I've also pulled this out of draft, so go ahead and review! @isamardzija , please make sure I didn't change anything that breaks this. Let's make this "good enough" to publish (accurate, working) even if it's not the most comprehensive guide ever!

@isamardzija
Copy link
Contributor Author

Everything looks good to me!

I followed the current draft and successfully connected Astro and PayloadCMS in a simple test project.

Copy link
Member

@ElianCodes ElianCodes left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for this detailed guide!

I left some small suggestions for you to review. Mostly whitespace changes

src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Show resolved Hide resolved
src/content/docs/en/guides/cms/payload.mdx Outdated Show resolved Hide resolved
@sarah11918 sarah11918 merged commit 3ac0633 into withastro:main May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add new content Document something that is not in docs. May require testing, confirmation, or affect other pages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants