Skip to content

[MDX] Docs: Custom components for imported mdx#5055

Closed
christian-hackyourshack wants to merge 4 commits intowithastro:mainfrom
christian-hackyourshack:mdx-custom-components-docs
Closed

[MDX] Docs: Custom components for imported mdx#5055
christian-hackyourshack wants to merge 4 commits intowithastro:mainfrom
christian-hackyourshack:mdx-custom-components-docs

Conversation

@christian-hackyourshack
Copy link
Copy Markdown
Contributor

@christian-hackyourshack christian-hackyourshack commented Oct 11, 2022

Changes

  • fixes a few formatting issues in the README
  • updates section about custom components in imported mdx

It's just a documentation change, hence a changeset is not required.

Closes #5027

Testing

It's just a documentation change, hence tests are not required.

Docs

It's just a documentation change.

@christian-hackyourshack christian-hackyourshack requested a review from a team as a code owner October 11, 2022 14:55
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Oct 11, 2022

⚠️ No Changeset found

Latest commit: 8ff4028

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Copy Markdown
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.

Reviewing on behalf of Docs Maintainers.

All the small edits look fine! I've left a suggestion re: the new content, but am happy to let @bholmesdev edit/approve as he sees fit. (I believe he was part of the original conversation, so tagging Ben, with the context, in.)

@christian-hackyourshack
Copy link
Copy Markdown
Contributor Author

I cannot find, why the mergeability check fails. I already tried to rebase the branch to the latest main, but it still fails. Any hints?

@bluwy
Copy link
Copy Markdown
Member

bluwy commented Oct 13, 2022

@christian-hackyourshack it's likely an error in the CI script and not the PR. The changes looks good to me 👍 cc @matthewp

Comment on lines +342 to +348
When rendering imported MDX content, custom components can be passed via the `components` prop.

You can also use exported custom components from an MDX file, by importimg them along with `Content` and pass them into the same `components` prop.
Copy link
Copy Markdown
Contributor

@bholmesdev bholmesdev Oct 14, 2022

Choose a reason for hiding this comment

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

Thanks for tagging me on this one @sarah11918! I'll admit this is pretty tricky behavior, and the example describes it really well. Wanted to clear up the wording to say "no, the export trick doesn't work unless you do this." Also want to add "see the example" since it's pretty necessary to understand how it works. Lmk if that works.

Suggested change
When rendering imported MDX content, custom components can be passed via the `components` prop.
You can also use exported custom components from an MDX file, by importimg them along with `Content` and pass them into the same `components` prop.
When rendering imported MDX content, custom components can be passed via the `components` prop.
Note: An MDX file's exported components will _not_ be used unless you manually import and pass them via the `components` property! See the example below:

Copy link
Copy Markdown
Contributor

@bholmesdev bholmesdev left a comment

Choose a reason for hiding this comment

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

Left a comment on wording around import / export components. Otherwise, thanks for this important docs change 👏

@bholmesdev
Copy link
Copy Markdown
Contributor

I cannot find, why the mergeability check fails. I already tried to rebase the branch to the latest main, but it still fails. Any hints?

Hm, that is pretty odd. I'd probably try another rebase since it's been a few days and we have some patches in. Otherwise, I'll ping core next week to see what's up

- whitespace at line end
- blank lines around lists and code fences
- casing of internal references
- fixed docs to clearly state, that custom components **must** be passed
  as props
- extended the example to show, the additional potential of passing
  components as props and how to reuse exported components
Accepting incoming suggestions from Sarah. Thanks!

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@christian-hackyourshack
Copy link
Copy Markdown
Contributor Author

Replaced this with PR #5190 that does not fail mergeability check.

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

Labels

pkg: integration Related to any renderer integration (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent MDX rendering between pages and <Content>

4 participants