Generate source code previews #1285
Conversation
|
Paul-Hebert
left a comment
Paul-Hebert
left a comment
There was a problem hiding this comment.
This is really clever @calebeby !
I assume it's a known short-coming that many of the code previews are loading a Storybook documentation-specific /demo/ twig partial? (I'm not sure what we could do about that besides providing a custom code sample for those stories so I don't think that should be a blocker.)
LGTM!
| // the arguments and input path are stored in the window.__twig_inputs__ variable. | ||
| // __twig_inputs__ is a map between the output HTML and and objects with the arguments and input paths | ||
| // Here, since we have the rendered HTML, we can look up what the arguments and path were | ||
| // that correspond to that output |
There was a problem hiding this comment.
This is really interesting! Clever!
There was a problem hiding this comment.
Thanks! This is the second approach, the first approach was to have the twig function return an object like this:
{
toString() {
return '...html...'
},
path: '...',
args: { ... }
}which I think is a nicer solution, and it didn't have the limitation of not working for the stories which use useEffect / other hooks. But then I realized it breaks for the canvas page, because the canvas page has a typeof check and it doesn't allow non-strings (even though an object with a toString method can be implicitly coerced into a string)
| Object.keys(args).length > 0 | ||
| ? ` with ${JSON.stringify(args, null, 2)}` | ||
| : ''; | ||
| return `{% include '${path}'${argsString} only %}`; |
There was a problem hiding this comment.
Is there ever a use case where we wouldn't want to use only? 🤔
If so, we might want to add a param to control this.
There was a problem hiding this comment.
I think it is best practice for includes to avoid implicitly passing through variables. For our docs, I put the only there so that if someone copy-pastes it it will probably "do the right thing", and they have to opt-in to the implicit passing through behavior. I can't think of any of our demos where someone using our patterns would always or usually want to have the implicit passing through behavior.
There was a problem hiding this comment.
Yeah that makes sense 👍
There was a problem hiding this comment.
I think that makes sense. We can always revisit later, and this is probably a safer starting point than leaving it off.
Yeah I talked through this with Tyler and it sounds like it is fine that it shows the include for the demo, and we can override the story source in those cases |
@calebeby I think it's fine to close that issue, this satisfies the intent for sure |
tylersticka
left a comment
tylersticka
left a comment
There was a problem hiding this comment.
Deferring final approval to @cloudfour/dev, this is a great step forward!
calebeby
changed the title
Overview
Closes #1004 - @tylersticka is it fine to close that issue or should we leave it open until we manually go through and update whichever stories need manual source code snippets?
Screenshots
Testing
Different cases to test the "show code" functionality:
v-next)