refactor: Migrate modal story to pure MDX story format#520
Conversation
frankieyan
left a comment
There was a problem hiding this comment.
This is looking really good 👍
Random thoughts, but they don't need to be a part of this PR:
- I wonder if there is value in having a separate template .mdx file, because the modal here is a bit of a different use case where it's made up of several sub-components, so our ArgsTable is currently a bit different. It'd be good to have an example where our props are inferred from the components themselves
- Also if we have a separate template, should we define some placeholders for information we should fill out? ie. component description, use cases, anything else we can think of? For reference, this is the fullstack team's storybook: https://doist.github.io/marketist/?path=/docs/typography-text--standard
- How do we add the description and default fields to the args table? Is it through jsdocs, or a
defaultPropsproperty on the component?
| so <ArgsTable of={Modal} /> wouldn't work in this case. Instead, we specify the | ||
| props of one story i.e. Playground | ||
| --> | ||
| <ArgsTable story="ModalPlaygroundStory" /> |
There was a problem hiding this comment.
Would it be possible to show the individual components' props? Storybook's docs imply that this is possible, but I tried updating meta to:
<Meta
title="Modal"
component={Modal.Box}
subcomponents={{ Header: Modal.Header }}
/>
and this to <ArgsTable of={Modal.Box} />, but the prop types aren't inferred:
Also, it's a bit funny that the props here are toggleable but the story itself sits at the very bottom. If the above isn't possible, then perhaps we can list the playground story first?
There was a problem hiding this comment.
I've tried doing this as well thinking each of Modal's subcomponent types would get picked up but they don't. I've even tried importing Box directly and setting that as a subcomponent as well as ArgsTable's of value
|
TL;DR: a) I think you're right and Modal might not be the best story to use as reference and b) getting prop types inferred for this component is proving to be tricky even without the MDX implementation because its made up of multiple nested components. This may be the case with a few other components using this same approach i.e.
So I've been trying out different implementations to get prop types inferred with
If this is regarding
The descriptions get added to the table automatically when the props are inferred from the components, which sadly isn't the case in The problem with this is that we would have to have a table for each story and the same props often show up on each story. |
|
In that case the changes here for the modal looks good to me 👍 we can investigate issues with the props table separately - it would be awesome if descriptions and default values can be inferred, or if at least there is an easy way to define them. For now, what do you think about bringing the playground story closer to the props table (as the first story)? Another thing is that it looks like the stories themselves can have a name defined, rather than using the exports' names: https://storybook.js.org/docs/react/writing-stories/introduction#rename-stories. Should we do this here (with just the modal to start?) |
Yes this is a good idea, we are already setting the story's name property anyway so why not give them better names.
@frankieyan This would make sense as it could get confusing toggling the controls and not seeing the changes being reflected in any of the docs tab stories. Something to keep in mind with this though is that it would mean Playground would appear first under Modal within the sidebar |
…ed all stories inside of playground story


Short description
Converted the modal story into pure MDX format as we start to move towards MDX documented components.
Ref: https://twist.com/a/1585/ch/190200/t/2455247/
This involved extracting all functions from the helper file and placing them into the MDX file alongside the stories. I've tried to reuse as much code as possible here, defining templates where possible, but please feel free to suggest any other ways in which I could make these more reusable.
PR Checklist
Versioning
v9.1.5 will be released to include this story