-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add related content component #212
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
Conversation
…on/related-content
|
Note to selves: I could not get .remove(
(node) => node.nodeName === 'H2' && node.textContent === 'For more help' && !node.nextElementSibling
)``` |
jerelmiller
left a comment
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.
Had a few questions for you.
src/components/MDXContainer.js
Outdated
| children, | ||
| className, | ||
| components, | ||
| relatedContent = {}, |
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 would avoid setting this default to an empty object. If you happen to hit this use case, React will complain about trying to render objects.
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 a bit mixed on this approach. While this solves the issue of making sure the content shows up in the correct container with the correct styles applied, this doesn't allow us any customization on the "extra" UI that is no longer markdown. Any additional UI that we want to add for any template would need its own prop.
What about potentially creating a body prop for the MDX body content and instead reserving children for this use case? This gives us a pretty good way to customize this component with extra UI without needing to open up new props every time we need to do so. This might look like this in practice:
import MDXContainer from './MDXContainer'
// ...
// Body with no additional UI
<MDXContainer body={body} />
// Body with additional UI
<MDXContainer body={body}>
{moreHelpExists ? null : <RelatedContent />}
</MDXContainer>This means that the MDXContainer doesn't have to care about the type of component that is rendered underneath the markdown. Thoughts?
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 see what you're saying. makes sense to me. i like that extra flexibility in putting any additional stuff in children. doing that now.
one question about that. would would the PropType be for children then? would it still be children: PropTypes.node
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.
Yep! PropTypes.node works great!
| replacement: (content) => | ||
| `<div className="landing-page-tile">\n\n${content}\n\n</div>\n\n`, | ||
| }) | ||
| .addRule('moreHelp', { |
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.
🎉
| } = mdx; | ||
|
|
||
| const moreHelpExists = mdxAST.children.find( | ||
| (node) => node.type === 'heading' && toString(node) === 'For more help' |
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.
[nit] Would it make sense to pull the For more help out to a constant at the top of the file so that its easy to reference/tweak?
|
Very odd that would't work. Thats ok. |
feat(IC): add content
Closes #194
Adds a
<RelatedContent />component to all pages that do not already have a For more help section populated. If just<h2>For more help</h2>element is present without content following it, it's removed during migration so thebasicDoc.jstemplate knows to insert the default content via the<RelatedContent />component.This also adds a prop to
MDXContainercomponent soRelatedContentis contained within the container to take advantage of all the CSS rules.Example of doc that has custom For more help content pre-populated:
https://docs-preview.newrelic.com/docs/browser/new-relic-browser/page-load-timing-resources/session-tracking