Skip to content

feat(examples): Add support for live and editable examples#49

Merged
dlabaj merged 3 commits intomainfrom
add-mdx
May 16, 2025
Merged

feat(examples): Add support for live and editable examples#49
dlabaj merged 3 commits intomainfrom
add-mdx

Conversation

@wise-king-sullyman
Copy link
Contributor

Closes #17 and actually also closes #23

To test pull this branch, start the dev server, and go to http://localhost:4321/components/accordion

@wise-king-sullyman wise-king-sullyman requested a review from dlabaj May 6, 2025 20:54
Copy link
Collaborator

@kmcfaul kmcfaul left a comment

Choose a reason for hiding this comment

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

After some though, I think we should probably not have a dep on the old docs framework

We could probably also port over the ast-helpers function

@@ -0,0 +1,296 @@
// TODO remove this component it is not good and I don't like that I have to add it
Copy link
Collaborator

@kmcfaul kmcfaul May 15, 2025

Choose a reason for hiding this comment

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

😆

+1 for long term, it would be nice to have a more out of the box solution and less manual resolution. I don't think it's the worst for now though.

@@ -0,0 +1,42 @@
import React, { useState } from 'react'
import { convertToReactComponent } from '@patternfly/ast-helpers'
import * as reactCoreModule from '@patternfly/react-core'
Copy link
Collaborator

@kmcfaul kmcfaul May 15, 2025

Choose a reason for hiding this comment

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

Would importing the whole module impact performance? Not sure if it's possible / how to import only what each example is using.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it should be ok as this is how the docs framework currently does it in example.js too, but I do share some of the same concern.

One advantage of this route is that we actually shouldn't need to list imports in the md files anymore, but if there is a notable performance impact we could refactor this to something that does require listing imports in the md, or maybe come up with a way of analyzing the imports of the example files themselves 🤔

export const h4 = ({children}: {children: ReactNode}) => <PFContent component="h4">{children}</PFContent>
export const h5 = ({children}: {children: ReactNode}) => <PFContent component="h5">{children}</PFContent>
export const h6 = ({children}: {children: ReactNode}) => <PFContent component="h6">{children}</PFContent>

Copy link
Collaborator

Choose a reason for hiding this comment

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

These correspond to the various ##/### syntax for the md/mdx right? We don't need other Content elements like p?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah good catch, we do probably need to add p tags if we want them to get the Content styling.

Copy link
Collaborator

@kmcfaul kmcfaul left a comment

Choose a reason for hiding this comment

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

I just realized I was leaving single comments instead of review comments, my bad.

I don't think there are any blockers, just had a few questions

@dlabaj dlabaj merged commit bbf7493 into main May 16, 2025
1 check passed
@github-actions
Copy link

🎉 This PR is included in version 1.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@wise-king-sullyman wise-king-sullyman deleted the add-mdx branch May 16, 2025 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add code editor to component page Add examples to component page

3 participants