-
Notifications
You must be signed in to change notification settings - Fork 20
feat: Editable Composed Stories #81
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
base: main
Are you sure you want to change the base?
Conversation
|
I have confirmed that I can in fact use the live code editor if both storybooks have been setup as per the README instructions. I've also added an example and how to run it |
| setupCompositionImports({ | ||
| 'my-library': MyLibrary, | ||
| }); | ||
| ``` |
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 don't really like the host storybook having to do extra stuff to support this. I wonder if it's worth it to try and refactor things.
Imagine if the preview frame already has everything needed to render a component including all dependencies. It just receives code from the editor through channel messages, no more cross frame global references (createStore) and no more dependencies (availableImports).
Getting the editor to have correct types for a library would require messages being passed from the preview frame to the parent frame with the editor. I think type definitions can be passed as a string through channel messages from the preview frame.
It's not clear to me how to set up the preview frame with all dependencies pre-added/bundled. Finding a good place for that would be nice. It does stop the story from defining its own dependencies which may be a problem for some users though...
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 taking a shot at the refactor and I'm curious about your last line:
It does stop the story from defining its own dependencies which may be a problem for some users though...
Can you elaborate?
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.
You can currently define available imports (dependencies) per story. If all available imports/dependencies were defined in the same place for a storybook, some users may find this is not compatible with whatever they may have done before.
Edit: I see you kept availableImports. I wonder how those work with a composed storybook.
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.
Okay so I also added more comments on what I did there.
|
Tried my hand at the refactor. It is a much cleaner implementation. I have a few apps I need to try this on will report back later on them. (or push more changes) |
|
Tested my apps and wow that was alot. It should work in full now and even pass type hinting context in the composed SB |
|
@JeremyRH I think it can be reviewed again. |
|
Thank you for all your work on this! I'll try to find time to test this before the new year but am on vacation until January 3rd. Here is my plan:
|
Please take a look and see if this makes sense.
I haven't fully tested it as yet but I have rendered stories from a composed library with this feature and I do see the code and example.
I'll wire it up later and confirm if the editing part works as expected.