Skip to content

A few storybook migrations - #48037

Merged
molly-moen merged 5 commits into
stagingfrom
molly/storybook-migrations
Sep 14, 2022
Merged

A few storybook migrations#48037
molly-moen merged 5 commits into
stagingfrom
molly/storybook-migrations

Conversation

@molly-moen

Copy link
Copy Markdown
Contributor

Moving to the new api for storybook for:

  • ConfirmDeleteButton
  • CrossTabChart
  • ExternalRedirectDialog

Links

@molly-moen
molly-moen requested review from a team and maddiedierker September 9, 2022 21:24
const Template = args => <ConfirmDeleteButton {...args} />;

export const BasicExample = Template.bind({});
BasicExample.args = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a scenario like this where there's only only story, is it better to list the args separate like this or to just include them in the template itself?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, my thought was to keep it separate to make it easier to add more stories in the future, but I am open to suggestion.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same question. I had 2 files with just one story each and included the args in the template. I'm good either way. @madelynkasula do you have a preference?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also listening in to this conversation. I went with Molly's route for consistency and expandability.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my thought was to keep it separate to make it easier to add more stories in the future

+1 to molly's comment. if the props are something that all stories will want (e.g., the isOpen prop for a dialog will always be true so we can render the dialog in every story), i hard-code those args in the template. otherwise, i keep them in individual stories.

i lean toward the latter (keeping props in individual stories) because it's easier to move these props into the template later than it is to do the reverse refactor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @madelynkasula ! I was also wondering about naming convention for a component with only story - I see default was used sometimes in the old Storybook version, or the name of the component itself. I ended up using BasicExample but someone also use Basic. Is there a preference?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmmm, that's a good question, alice. i think using the name of the component is redundant since all stories are nested under the component name. otherwise, it doesn't really matter, but we should set a convention to be consistent! let's go with "BasicExample" unless others have opinions

</div>
));
export default {
title: 'CrossTabCharts',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: title should be the same as component name

const Template = args => <ConfirmDeleteButton {...args} />;

export const BasicExample = Template.bind({});
BasicExample.args = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my thought was to keep it separate to make it easier to add more stories in the future

+1 to molly's comment. if the props are something that all stories will want (e.g., the isOpen prop for a dialog will always be true so we can render the dialog in every story), i hard-code those args in the template. otherwise, i keep them in individual stories.

i lean toward the latter (keeping props in individual stories) because it's easier to move these props into the template later than it is to do the reverse refactor

@molly-moen
molly-moen merged commit c8f41b8 into staging Sep 14, 2022
@molly-moen
molly-moen deleted the molly/storybook-migrations branch September 14, 2022 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants