search

carbon-design-system / carbon-platform

The "next" version of the Carbon Design System website, as a platform.
https://next.carbondesignsystem.com
Apache License 2.0
21 stars 5 forks source link

Convert MDX components in web-app to `mdx-components` package #1103

Closed jharvey10 closed 1 year ago

jharvey10 commented 2 years ago

🦖@alisonjoseph 🐳 @francinelucca 🐯@jdharvey-ibm 🐢 @andreancardona

Markdown components

Core Carbon components

Q: Why are these here? Why are the carbon components as-is not sufficient to host our carbon documentation and other MDX? Can these be removed in favor of the base carbon components?

Gatsby theme components

Internal Components

Deferred

Convert to js pages (https://github.com/carbon-design-system/carbon-platform/issues/1446)

alisonjoseph commented 2 years ago

Why are these here? Why are the carbon components as-is not sufficient to host our carbon documentation and other MDX? Can these be removed in favor of the base carbon components?

We are just using the core Carbon components without modification, they are imported into the components file so they can be using within any mdx file without importing at the top. (required currently to import some mdx files direct from the Carbon website). Is there a better way to make certain core components easily available for use?

Some of the components in the "Gatsby theme component" list aren't actually in the Gastby theme, and just pulled direct from the current Carbon website. Not sure if that really matters for our purposes, but thought I'd point it out.

Will also need to add StorybookDemo to the list

jharvey10 commented 2 years ago

@alisonjoseph Okay awesome. I think with some of this remote mdx work we're doing, we'll be able to keep that no-need-to-import behavior and wrap up things even more concisely in the mdx-components package (by just re-exporting some of the carbon components as an esmodule).

alisonjoseph commented 2 years ago

Question, do we want customized site-specific mdx components in this package? For example the following components are specific to the core Carbon docs, and wouldn't be easily reused. If we do want them in here perhaps we can categorize them differently in the storybook?

francinelucca commented 2 years ago

My thought is we probably don't need them to be available to everyone so I wouldn't put them on mdx-components off the bat. Unless there's a technical constraint in which ALL components rendered by mdx remote pages need to live in the mdx-components package, then I guess we'd have no choice since we'll use them for our own remote pages. @jdharvey-ibm is there a constraint to do that?

jharvey10 commented 2 years ago

@francinelucca @alisonjoseph Ideally we would have all of the components that are rendered via MDX included in the mdx-components package. That enables us to do things like offload the rendering of the MDX to a separate service so we can cache things asynchronously.

It might not make sense from a user perspective to have these ancillary components in the storybook as well, since we wouldn't really expect others to use them, but from a Platform technical perspective, that'll be super beneficial to have them all in that package.

I like the idea of having them be in a separate category in the storybook, or we could even have a separate storybook build altogether which hides them in the one that is hosted on platform.carbondesignsystem.com.

alisonjoseph commented 2 years ago

@jdharvey-ibm that makes sense to bring them over then, What about an "Internal" category in storybook for those components? Adding another separate storybook seems like adding unnecessary complexity.

jharvey10 commented 2 years ago

@alisonjoseph That'd be perfect!

francinelucca commented 2 years ago

I'm confused as to what component we mean by "MdxWrapper" , is that the MdxPage ? that seems like it has a lot of dependencies with the web app 🤔 (nav data, exceptions, pageheaders...)

alisonjoseph commented 2 years ago

@francinelucca did we rename MdxWrapper, I don't see it anymore. If its the page, then I think that stays in the web-app.

francinelucca commented 2 years ago

yeah I'm pretty positive we removed MdxWrapper in favor of MdxPage and agree it should stay in the web app