-
Notifications
You must be signed in to change notification settings - Fork 8
[WIP] feat(plugin): add sourceDecorator and auto argTypes #7
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
- introduce sourceDecorator to support dynamic story source rendering - Introduce preview-docs.ts to enable decorator by default - Updated example to enable autodocs and codepanel fixes stenciljs#5
f3f2211
to
9f682e4
Compare
- introduced `setCustomElementsManifest` method to laod CEM - based on @storybook/web-components, improved and adjusted for stencil - updated example stencil config to use @custom-elements-manifest/analyzer - updated my-advanced component to have more variety of props, methods, etc
682486b
to
b4d5c64
Compare
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 love to see some e2e tests that the component code is actually rendered correctly in Storybook. Can we add that to this PR?
@Component({ | ||
tag: 'my-advanced', | ||
styleUrl: 'my-advanced.css', | ||
shadow: false, |
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.
Can we set scoped: true
instead?
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.
This is actually on purpose. i wanted to test that it works for shadowless components and since we already had 2 shadow components, I made this one as false.
Are there any concerns?
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.
scoped: true
creates a shadow less component. My concern with this is that this may create a non-scoped/non-shadow component which we may deprecate in the future.
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.
My concern with this is that this may create a non-scoped/non-shadow component which we may deprecate in the future.
My whole library is like that T_T (but I guess that's my problem)
Ok fair enough, i'll change it to scoped: true
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.
Setting shadow: false
might be the same as scoped: true
, not sure
if (parameters.slots) { | ||
Object.entries(parameters.slots).forEach(([key, value]) => { | ||
// if the parameter key is 'default' don't give it a slot name so it renders just as a child | ||
const slot = key === 'default' ? undefined : key; | ||
|
||
// Handle array of values for the same slot | ||
const values = Array.isArray(value) ? value : [value]; | ||
|
||
values.forEach(item => { | ||
if (item === undefined || item === null) return; | ||
|
||
if (typeof item === 'string') { | ||
// For strings, create a vnode with the string as the children | ||
children.push(h('span', { slot }, item)); | ||
} else if (Array.isArray(item)) { | ||
// Handle nested arrays (flatten them) | ||
item.forEach(nestedItem => { | ||
if (nestedItem !== undefined && nestedItem !== null) { | ||
children.push({ | ||
...nestedItem, | ||
$attrs$: { | ||
...(nestedItem.$attrs$ || {}), | ||
slot, | ||
}, | ||
}); | ||
} | ||
}); | ||
} else { | ||
// For VNodes or other objects | ||
children.push({ | ||
...item, | ||
$attrs$: { | ||
...(item.$attrs$ || {}), | ||
slot, | ||
}, | ||
}); | ||
} | ||
}); | ||
}); | ||
} |
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.
if (parameters.slots) { | |
Object.entries(parameters.slots).forEach(([key, value]) => { | |
// if the parameter key is 'default' don't give it a slot name so it renders just as a child | |
const slot = key === 'default' ? undefined : key; | |
// Handle array of values for the same slot | |
const values = Array.isArray(value) ? value : [value]; | |
values.forEach(item => { | |
if (item === undefined || item === null) return; | |
if (typeof item === 'string') { | |
// For strings, create a vnode with the string as the children | |
children.push(h('span', { slot }, item)); | |
} else if (Array.isArray(item)) { | |
// Handle nested arrays (flatten them) | |
item.forEach(nestedItem => { | |
if (nestedItem !== undefined && nestedItem !== null) { | |
children.push({ | |
...nestedItem, | |
$attrs$: { | |
...(nestedItem.$attrs$ || {}), | |
slot, | |
}, | |
}); | |
} | |
}); | |
} else { | |
// For VNodes or other objects | |
children.push({ | |
...item, | |
$attrs$: { | |
...(item.$attrs$ || {}), | |
slot, | |
}, | |
}); | |
} | |
}); | |
}); | |
} | |
Object.entries(parameters.slots || {}).forEach(([key, value]) => { | |
// if the parameter key is 'default' don't give it a slot name so it renders just as a child | |
const slot = key === 'default' ? undefined : key; | |
// Handle array of values for the same slot | |
const values = Array.isArray(value) ? value : [value]; | |
values.forEach(item => { | |
if (item === undefined || item === null) return; | |
if (typeof item === 'string') { | |
// For strings, create a vnode with the string as the children | |
children.push(h('span', { slot }, item)); | |
} else if (Array.isArray(item)) { | |
// Handle nested arrays (flatten them) | |
item.forEach(nestedItem => { | |
if (nestedItem !== undefined && nestedItem !== null) { | |
children.push({ | |
...nestedItem, | |
$attrs$: { | |
...(nestedItem.$attrs$ || {}), | |
slot, | |
}, | |
}); | |
} | |
}); | |
} else { | |
// For VNodes or other objects | |
children.push({ | |
...item, | |
$attrs$: { | |
...(item.$attrs$ || {}), | |
slot, | |
}, | |
}); | |
} | |
}); | |
}); |
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.
For this one, no issue with your suggestion, but I also noticed that you just merged a PR that also changes this.
I handle a slightly different scenario, but I was wondering if it's worth getting David's opinion on my changes.
@davidpett thoughts? :)
@dogoku in our current use of storybook and stencil we are using this addon to get the docs and controls: https://github.com/RocketCommunicationsInc/storybook-addon-docs-stencil#readme I think it might be a good idea to see how they are documenting things. I just tried adding it to the example app here and it partially works for the docs. |
@davidpett oh cool, i didn't know this existed. they are using stencil's docs-json, but i am using CEM. I wanted to use docs-json, but since web-components did the heavy lifting, I just followed their logic and adapted it to work for stencil. Also I noticed that what you shared is a fork, that hasn't been updated for a long time. That main repo also has a custom stencil renderer function, which is also something we have. So I am seeing a significant overlap between our 2 endeavors. I guess there is a decision to be made here
@christian-bromann i defer to you |
@dogoku thanks for providing your insights here!
I personally prefer to have this plugin contain all required primitives to have a great Storybook integration. Having users to add another (not by Stencils community maintained) plugin may create friction in the future. Is there a way we can keep your suggested CEM approach while allowing users to choose a different plugin if desired?
I think this is a good idea, thoughts @davidpett ?
I think this should be always an option. |
@christian-bromann having an integrated solution is a great idea and I would love to not have to reach out to an additional addon for this functionality. I did do a quick comparison between the docs-json output target and the CEM output and although very similar, docs-json is a little more verbose and handles complex types a little better. I wonder since it is an official stencil output target if we should rely on it instead of CEM as the default? |
The |
@christian-bromann @davidpett |
Adds support for showing story source code - Fixes #5
Warning
This is still very much a WIP
Changes
.storybook/preview.ts
to enable autodocs and codepanel<my-comp />
parameters.docs.source.type = code
to instead show the complete story source code (example below)prettier/standalone
to format the generated string in the browsersetCustomElementsManifest
method to laod CEMScreenshots
Autodocs with Autogenerated Attributes from CEM
Attribute based source
Custom renderer source (for more complex demos)