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

Addon-docs: Refactor Props to use ArgsTable #10341

Merged
merged 10 commits into from
Apr 15, 2020
Merged

Addon-docs: Refactor Props to use ArgsTable #10341

merged 10 commits into from
Apr 15, 2020

Conversation

shilman
Copy link
Member

@shilman shilman commented Apr 7, 2020

Issue: #6639

What I did

This is the first of a series of PRs to add controls (formerly known as "knobs") to the Props block:

  • Convert Props to use ArgsTable (this PR)
  • Add Controls to ArgsTable
  • Change parameterEnhancers to a more restrictive ArgTypes enhancer
  • Performance improvements

This is the largest PR. It contains:

  • New type system for ArgTypes
  • New <Props story="story name" /> API
  • New ArgsTable, a refactor of PropsTable that's based on ArgTypes
  • New parameter enhancers to enable type inference from:
    • components
    • args
    • argTypes
  • Conversion to ArgTypes for all docs-supported frameworks

It is demonstrated in the props.stories.mdx demo, which will subsequently be enhanced with Controls in the follow-on PR

How to test

run official-storybook and navigate to Addons/Docs/props

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Bunch of smaller comments. Looks good.

Haven't tested it yet; I'll play with it as part of the controls PR

addons/docs/src/blocks/Props.tsx Outdated Show resolved Hide resolved
addons/docs/src/frameworks/common/enhanceArgTypes.ts Outdated Show resolved Hide resolved
const { parameters } = context;
const { component, subcomponents, args = {}, argTypes = {}, docs = {} } = parameters;
const { extractArgTypes } = docs;
// if (context.storyFn.length === 0 || !component || !extractArgTypes) {
Copy link
Member

Choose a reason for hiding this comment

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

We should decide if we do this.

addons/docs/src/frameworks/common/enhanceArgTypes.ts Outdated Show resolved Hide resolved

extractedArgTypes = Object.entries(componentArgTypes).reduce((acc, [label, compTypes]) => {
Object.entries(compTypes).forEach(([key, argType]) => {
const subLabel = label === 'Primary' ? key : camelCase(`${label} ${key}`);
Copy link
Member

Choose a reason for hiding this comment

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

The primary component doesn't get its arg names mangled?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I guess this needs to be thought of in combination with the story/primary component arg combination idea.

addons/docs/src/frameworks/common/enhanceArgTypes.test.ts Outdated Show resolved Hide resolved
export const inferArgTypes = (args: Args) => {
if (!args) return {};
return Object.entries(args).reduce((acc, [name, arg]) => {
if (arg != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (arg != null) {
if (arg !== null) {

Copy link
Member Author

Choose a reason for hiding this comment

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

!= null handles both null and undefined

Copy link
Member

@tmeasday tmeasday Apr 13, 2020

Choose a reason for hiding this comment

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

Ughh, me no likey

Comment on lines +6 to +11
// import camelCase from 'lodash/camelCase';
// Object.entries(subTypes).forEach(([key, val]) => {
// const subLabel = camelCase(`${label} ${key}`);
// acc[subLabel] = { ...val, table: { ...val.table, tab: label } };
// });

Copy link
Member

Choose a reason for hiding this comment

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

?

lib/components/src/blocks/ArgsTable/ArgsTable.tsx Outdated Show resolved Hide resolved
lib/components/src/blocks/ArgsTable/ArgsTable.tsx Outdated Show resolved Hide resolved
@shilman shilman merged commit 293c1a2 into next Apr 15, 2020
@stof stof deleted the 6639-args-table branch May 25, 2022 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants