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

Docs: Add ArgTypes API reference #22970

Merged
merged 6 commits into from
Jun 13, 2023
Merged

Docs: Add ArgTypes API reference #22970

merged 6 commits into from
Jun 13, 2023

Conversation

kylegach
Copy link
Contributor

@kylegach kylegach commented Jun 7, 2023

Closes #17681

What I did

  • Remove previous ArgTypes guide
  • Add/update relevant snippets
  • Move CSF page out of Stories sub-section in TOC
    • Remove now-empty Stories sub-section

How to test

  1. Follow the steps in the contributing instructions for this branch, api-reference-arg-types
    • There are a lot of embedded snippets, so this step is critical!
  2. Check for:
    a. Completeness
    - Is every property documented fully?
    b. Correctness
    - Types
    - Required or not
    - Default values
    - Descriptions
    - Example snippets
    c. Consistency in structure

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@kylegach kylegach added documentation patch:yes Bugfix & documentation PR that need to be picked to main branch labels Jun 7, 2023
@kylegach kylegach self-assigned this Jun 7, 2023

Specifies the type of control used to change the arg value with the [controls addon](../essentials/controls.md). Here are the available types, `ControlType`, grouped by the type of data they handle:

| Data type | ControlType | Description |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same table as in the old argtypes.md, organized more sensibly.


Default: [Inferred](#automatic-argtype-inference)

TK <!-- TODO: Not sure how this differs from `table.type.summary` -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Highlighting this TODO, for which I need an answer, please.

Copy link
Member

Choose a reason for hiding this comment

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

TLDR: The type field is semantic and the table.type.summary is what's shown in the UI.

It's a little convoluted and I hope we can streamline this in 8.0.

  1. The component is analyzed using some kind of docgen (react-docgen/compodoc/etc.) All of these tools have different formats.
  2. Those all get summarized in the type field. Users can also specify the type field themselves, in case docgen got it wrong, or there is no docgen for your particular framework/renderer.
  3. Storybook uses the type field to auto-generate the control field (which can also be manually overwritten/refined)
  4. Storybook also uses the type field to autogenerate the table.type.summary/details fields for display (which can also be manually overwritten/refined)

There are two ways of specifying type:

// longhand
export default {
  argTypes: { 
     foo: { type: { name: 'number' } },
  }
}
// shorthand
export default {
  argTypes: { 
     foo: { type: 'number' },
  }
}

Why not just use the shorthand? Well, there is some support for structured types like Object/Array. This was overengineered in retrospect since we don't take advantage of that data yet. We will in the future, but I don't know when. If I could turn back time, I would just use the shorthand.

Also worth noting that we're planning on introducing an even shorter shorthand in 7.2 which will look like:

export default {
  argTypes: {
     foo: 'number',
     bar: 'action', // <= this is new
  } 
}

We'll probably create an RFC for this change. The rationale behind it is a bit complex but it has to do with performance, story portability, etc. (making it easier to not have to run docgen at all in some cases). Should not affect the current docs, but just wanted to give the full picture that this will be changing soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just use the shorthand? Well, there is some support for structured types like Object/Array. This was overengineered in retrospect since we don't take advantage of that data yet. We will in the future, but I don't know when. If I could turn back time, I would just use the shorthand.

I agree with you @shilman for a long time it was confusing for me, I thought it is a new API. I think we need to normalize and simplify the API, and it should be always an option to override values.

Another important point @shilman, should we separate the docgen plugins, right you are kind of normalizing react and vue, the caveat here is not all renderers have the same architecture

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the detailed answer! 👏

Updates pushed in 55bfceb.

@kylegach kylegach marked this pull request as ready for review June 7, 2023 22:12
@kylegach kylegach requested review from jonniebigodes and shilman June 7, 2023 22:12
@kylegach kylegach force-pushed the api-reference-arg-types branch 2 times, most recently from 3d4a32c to c237479 Compare June 7, 2023 22:41
kylegach added a commit to storybookjs/frontpage that referenced this pull request Jun 7, 2023
@chakAs3
Copy link
Contributor

chakAs3 commented Jun 8, 2023

Hi @kylegach is this just for react or agnostic ?

Copy link
Contributor

@chakAs3 chakAs3 left a comment

Choose a reason for hiding this comment

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

this PR is important for me. we may be depending on each other, with my last 2 PRs that deal with ArgTypes even if it is just docs

docs/snippets/common/arg-types-in-story.ts.mdx Outdated Show resolved Hide resolved
@kylegach
Copy link
Contributor Author

kylegach commented Jun 8, 2023

@chakAs3

Hi @kylegach is this just for react or agnostic ?

It's intended to be agnostic. The argTypes API itself is definitely agnostic. The snippets demonstrating how to use it should be available for all necessary renderers (because the demonstrate using the meta, they're available for Angular, Web Components, and "common", which should cover everything else).

@kylegach
Copy link
Contributor Author

kylegach commented Jun 8, 2023

@chakAs3

this PR is important for me. we may be depending on each other, with my last 2 PRs that deal with ArgTypes even if it is just docs

Thanks for the heads up and your review! If you link them here, I'm happy to take a look to confirm whether we need to be concerned about any overlap.

@kylegach kylegach force-pushed the api-reference-arg-types branch from c237479 to d4dfbd6 Compare June 8, 2023 15:32
@chakAs3
Copy link
Contributor

chakAs3 commented Jun 8, 2023

Hi @kylegach this is the PR #22912,
I want to make sure I can get the correct Arg Types from most renderers, there is no breaking change tho. if the control types are not supplied by the renderer it will fall the old behavior.
I need your input and probably @MichaelArestad . design wise

@kylegach
Copy link
Contributor Author

kylegach commented Jun 9, 2023

@chakAs3 — Interesting! Looks like all that will be necessary docs-wise is a new row in the table of allowable control types. Thanks!

@chakAs3
Copy link
Contributor

chakAs3 commented Jun 9, 2023

@chakAs3 — Interesting! Looks like all that will be necessary docs-wise is a new row in the table of allowable control types. Thanks!

Yes, it is union control, so now you switch between allowable controls, I pick the one you want in case you have an Arg of type union ( number | string | boolean | ....) may be we need to limit the number or come up with a layout that can accommodate numerous controls.

@kylegach kylegach force-pushed the api-reference-arg-types branch from d4dfbd6 to 55bfceb Compare June 9, 2023 21:21
docs/api/arg-types.md Outdated Show resolved Hide resolved
docs/api/arg-types.md Outdated Show resolved Hide resolved
@kylegach kylegach force-pushed the api-reference-arg-types branch from 55bfceb to 9e44678 Compare June 12, 2023 22:37
@kylegach kylegach changed the title Add ArgTypes API reference Docs: Add ArgTypes API reference Jun 12, 2023
Copy link
Contributor

@jonniebigodes jonniebigodes left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the feedback so promptly. Appreciate it. I'm good with it.

type?: { summary?: string; detail?: string };
},
type?: SBType | SBScalarType['name'];
defaultValue?: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

As the defaultValue is deprecated, probably not a bad idea to reference it as such and point directly to the section or remove it. WDYT?

Copy link
Contributor

@jonniebigodes jonniebigodes left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the feedback so promptly. Appreciate it. All good on my end

@kylegach kylegach merged commit e39cc9f into next Jun 13, 2023
@kylegach kylegach deleted the api-reference-arg-types branch June 13, 2023 14:33
github-actions bot pushed a commit that referenced this pull request Jun 13, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 13, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
kylegach added a commit to storybookjs/frontpage that referenced this pull request Jun 13, 2023
github-actions bot pushed a commit that referenced this pull request Jun 13, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
shilman pushed a commit that referenced this pull request Jun 14, 2023
Docs: Add ArgTypes API reference
(cherry picked from commit e39cc9f)
@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jun 14, 2023
@jonniebigodes jonniebigodes mentioned this pull request Dec 1, 2023
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ArgTypes documentation
4 participants