Skip to content
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

[core] Replace storybook knobs for args #601

Merged
merged 8 commits into from
Nov 30, 2020

Conversation

tooppaaa
Copy link
Contributor

Storybook knobs are being shut down in favor of args.
https://storybook.js.org/docs/react/writing-stories/args

This is using react docgen behind the scene to generate the right controls and actions.

I tried to only change what was necessary to remove knobs without loosing any functionality in place.

While there's still work to do on this like

cleanup remaining defined actions
arrange stories to capitalize on args
Regarding performances, enabling reactDocgen only adds 0.3 secs


const env = process.env.NODE_ENV || 'development';
/* eslint-disable */
const __DEV__ = env === 'development';
const __PROD__ = env === 'production';
const isDevelopment = env === 'development';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DEV is already defined somewhere globally, typescript complains

@@ -1,3 +1,4 @@
// @ts-ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To find out why and remove

checkboxSelection: true,
disableSelectionOnClick: false,
disableMultipleColumnsSorting: false,
sortingOrder: ['asc', 'desc', 'null' as SortDirection],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

null is not displayed as a string, small trick here...

@dtassone
Copy link
Member

Thanks Tooppaaa it looks great.
I had a look at the new controls tab, it seems pretty cool, I can't wait to see it fully implemented :)

@oliviertassinari
Copy link
Member

@dtassone What's the next step on this pull request?

</div>
);
}
export const Loading = Template.bind({});
Copy link
Member

Choose a reason for hiding this comment

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

Could we wrap that to make it more understandable on what we do here?
Maybe
const getStoryTemplate = ()=> Template.bind({});

Copy link

Choose a reason for hiding this comment

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

The following is a storybook pattern:

export const Foo = Bar.bind({});
Foo.args = { ... }

We recommend everybody use this pattern if they are using Args stories, since we have some tooling in mind that will look for this pattern in static analysis...

@dtassone
Copy link
Member

@dtassone What's the next step on this pull request?

It's actually ready, beside conflicts and my little comment... I originally thought that there was more work to do on controls but it seems ready

@tooppaaa
Copy link
Contributor Author

I'll handle everything tonight / this weekend !
Happy to discuss what you imagine for next steps.

@tooppaaa
Copy link
Contributor Author

@dtassone everything should be up do date and with the same functionality as before.

@dtassone dtassone merged commit af31bf6 into mui:master Nov 30, 2020
@oliviertassinari oliviertassinari changed the title Storybook: remove knobs for args [storybook] Replace knobs for args Nov 30, 2020
@oliviertassinari oliviertassinari changed the title [storybook] Replace knobs for args [core] Replace storybook knobs for args Nov 30, 2020
@zannager zannager added the core Infrastructure work going on behind the scenes label Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants