-
Notifications
You must be signed in to change notification settings - Fork 32
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
refactor!: Story children
no longer forwards args
& storyContext
& add new props - asChild
and template
#228
base: next
Are you sure you want to change the base?
Conversation
fix type errors & add `TemplateSnippet` & document
@JReinhold I need a second eye. I'm not sure if I have overlooked anything. The only thing I didn't try to achieve in this PR is allowing passing a string to |
children
becomes static & add template
propchildren
becomes static & add new props - asChild
and template
children
becomes static & add new props - asChild
and template
children
no longer forwards args
& storyContext
& add new props - asChild
and template
@JReinhold Both are related to the Story source code. Case 1When there's a <Story name="Long content">The very long content</Story> And the expected output is: <Button onclick={onClickFn}>The very long content</Story> Case 2This is a small "nit" pick on the visual details. New lines |
src/runtime/Story.svelte
Outdated
{:else if children && isSnippet(children)} | ||
{#if asChild} | ||
{@render children()} | ||
{:else if renderer.storyContext.component} | ||
<renderer.storyContext.component {...renderer.args}> | ||
{@render children()} | ||
</renderer.storyContext.component> | ||
{:else} | ||
{@render children()} | ||
{/if} | ||
{:else if storiesTemplate} | ||
{@render storiesTemplate(renderer.args, renderer.storyContext)} |
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.
@dominikg Does this seem correct to you?
{@render children()} | ||
{/if} | ||
{:else if storiesTemplate} | ||
{@render storiesTemplate(renderer.args, renderer.storyContext)} |
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.
does this include children
in the args somewhere? (not sure if dealbreaker when not but what would happen with this?
{#snippet template(args)}
<h1>button</h1>
<Button {...args}/>
{/snippet}
<Story {template}>
Hello!
</Story>
would Hello! be put in the children prop of Button in the template?
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.
No, it doesn't include - I'm testing it right now.
The following template will crash at runtime using examples/Button.stories.svelte
, because it is trying to call {@render}
on undefined children
.
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.
However, if you provide children
via meta, for example:
const { Story } = defineMeta({
args: {
children: createRawSnippet(() => ({ render: () => "Hello!" })),
}
});
Then yes, the Hello!
will be included.
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 think in this case, when users provide both template
and children
snippets... we should probably throw a documented error. WDYT?
At the current moment, I managed to create an error on the TypeScript level.
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.
fine with template and children colliding in this case, as long as you can still forward children through args manually. An error thats helpful enough to prevent the user from doing this is good.
Co-authored-by: Dominik G. <[email protected]>
Note
Concluded by feedback discussion thread on Discord
TODO
examples/
template
snippet prop forStory
asChild
flag prop forStory
children
no longer forwardsargs
&storyContext
& add new props -asChild
andtemplate
#228 (comment)children
andtemplate
are provided together ortemplate
&asChild
📦 Published PR as canary version:
5.0.0--canary.228.e697925.0
✨ Test out this PR locally via: