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

chore: extend markdoc configuration #88

Closed
wants to merge 0 commits into from

Conversation

pietrodev07
Copy link

Description

@ghost
Copy link

ghost commented Jun 7, 2024

The preview deployment is in progress. 🟡

Open Build Logs

Last updated at: 2024-06-07 08:10:53 CET

@ghost
Copy link

ghost commented Jun 7, 2024

The preview deployment is in progress. 🟡

Open Build Logs

Last updated at: 2024-06-07 08:10:55 CET

@ghost
Copy link

ghost commented Jun 7, 2024

The preview deployment is in progress. 🟡

Open Build Logs

Last updated at: 2024-06-07 08:10:56 CET

@ghost
Copy link

ghost commented Jun 7, 2024

The preview deployment failed. 🔴

Open Build Logs

Last updated at: 2024-06-07 08:11:09 CET

@ghost
Copy link

ghost commented Jun 7, 2024

The preview deployment failed. 🔴

Open Build Logs

Last updated at: 2024-06-07 08:11:13 CET

@ghost
Copy link

ghost commented Jun 7, 2024

The preview deployment failed. 🔴

Open Build Logs

Last updated at: 2024-06-07 08:11:15 CET

@ghost
Copy link

ghost commented Jun 9, 2024

The preview deployment is in progress. 🟡

Open Build Logs

Last updated at: 2024-06-09 08:17:37 CET

@ghost
Copy link

ghost commented Jun 9, 2024

The preview deployment is in progress. 🟡

Open Build Logs

Last updated at: 2024-06-09 08:17:38 CET

@ghost
Copy link

ghost commented Jun 9, 2024

The preview deployment is in progress. 🟡

Open Build Logs

Last updated at: 2024-06-09 08:17:39 CET

@ghost
Copy link

ghost commented Jun 9, 2024

The preview deployment failed. 🔴

Open Build Logs

Last updated at: 2024-06-09 08:17:52 CET

@ghost
Copy link

ghost commented Jun 9, 2024

The preview deployment failed. 🔴

Open Build Logs

Last updated at: 2024-06-09 08:17:58 CET

@ghost
Copy link

ghost commented Jun 9, 2024

The preview deployment failed. 🔴

Open Build Logs

Last updated at: 2024-06-09 08:18:02 CET

@jdtjenkins
Copy link
Contributor

Hi @pietrodev07 thanks for the PR!

So just looking at this and there'll need to be a few more changes before we can merge this, but it's a good start!

  1. So you've added new props to renderMarkDoc but not updated where it's being called in the StudioCMSRenderer - packages/studioCMS/src/components/exports/StudioCMSRenderer.astro. So the renderer will need to be updated to get all the correct variables and stuff.

  2. Just curious why the schema was added/removed? What was the thinking behind that?

  3. I'd love there to be some test projects in the playgrounds folder to show off rendering for the different rendering options here, react, react-static, html.

  4. The react and @types/react packages should be optional peerDependencies in the package.json

@Adammatthiesen
Copy link
Member

Also to add to what @jdtjenkins mentioned, Please also run pnpm install to be able to update the package-lock as well! as the PR can't run test deployments without the updated lock-file! (This counts for anytime you change Deps within the repo)

Side Note: Not as important right now, but will be in the future once we have began releasing our package, is running pnpm changeset and walking through those steps to create a new Minor or Patch to the main package when making changes (Obviously we haven't released a package just yet but it is still good practice to get used to, and a great learning chance for new to coding Contributors!

Again Thank you for helping tackle this! And we cant wait to see what the final version of this will look like!

@Adammatthiesen Adammatthiesen mentioned this pull request Jun 16, 2024
17 tasks
@Adammatthiesen
Copy link
Member

@pietrodev07 Were you still wanting to take lead on this? Its been a few weeks since we last checked in. There has also been some changes to the repository that require some moving around of files and updating. Please let us know 🙏

@pietrodev07
Copy link
Author

Hi @pietrodev07 thanks for the PR!

So just looking at this and there'll need to be a few more changes before we can merge this, but it's a good start!

  1. So you've added new props to renderMarkDoc but not updated where it's being called in the StudioCMSRenderer - packages/studioCMS/src/components/exports/StudioCMSRenderer.astro. So the renderer will need to be updated to get all the correct variables and stuff.
  2. Just curious why the schema was added/removed? What was the thinking behind that?
  3. I'd love there to be some test projects in the playgrounds folder to show off rendering for the different rendering options here, react, react-static, html.
  4. The react and @types/react packages should be optional peerDependencies in the package.json
  1. I didn't notice that the StudioCMSRenderer needed to be updated.
  2. The schema changes were an error on my part; I didn't intend to make those modifications.
  3. Yes, I agree that adding test projects in the playgrounds folder would be beneficial.
  4. Thanks for pointing out the dependency configuration, I'll make those adjustments.

@pietrodev07
Copy link
Author

Also to add to what @jdtjenkins mentioned, Please also run pnpm install to be able to update the package-lock as well! as the PR can't run test deployments without the updated lock-file! (This counts for anytime you change Deps within the repo)

Side Note: Not as important right now, but will be in the future once we have began releasing our package, is running pnpm changeset and walking through those steps to create a new Minor or Patch to the main package when making changes (Obviously we haven't released a package just yet but it is still good practice to get used to, and a great learning chance for new to coding Contributors!

Again Thank you for helping tackle this! And we cant wait to see what the final version of this will look like!

ah yeah, I forgot to do this in the second clone

@pietrodev07
Copy link
Author

@pietrodev07 Were you still wanting to take lead on this? Its been a few weeks since we last checked in. There has also been some changes to the repository that require some moving around of files and updating. Please let us know 🙏

Yeah, I'm sorry, I've been super busy with my projects!
I'll make the changes you suggested by the weekend!

I apologize again...

@Adammatthiesen
Copy link
Member

@pietrodev07 Were you still wanting to take lead on this? Its been a few weeks since we last checked in. There has also been some changes to the repository that require some moving around of files and updating. Please let us know 🙏

Yeah, I'm sorry, I've been super busy with my projects! I'll make the changes you suggested by the weekend!

I apologize again...

No problem! We just wanted to make sure everything was okay, Things happen in life! And we are all here trying to better our community!

Dont forget, You are free to ask any questions on how implementation works, as this is a fairly complex project 😅

@Adammatthiesen
Copy link
Member

Adammatthiesen commented Jun 29, 2024

@pietrodev07 one small note before you get to far along.... please dont forget to rebase/update your branch with the new changes on main, that way things will start to work again... there was quite a few updates from the last big merge a few days ago, which is why things are conflicting 😅

@Adammatthiesen
Copy link
Member

Also with the new changes that would also move the MarkDoc.ts file from /utils to /utils/renderers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] Extend current markdoc support
3 participants