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: Descriptions per Story #8527

Closed
donaldpipowitch opened this issue Oct 23, 2019 · 53 comments
Closed

addon-docs: Descriptions per Story #8527

donaldpipowitch opened this issue Oct 23, 2019 · 53 comments

Comments

@donaldpipowitch
Copy link
Contributor

donaldpipowitch commented Oct 23, 2019

Issuehunt badges

Is your feature request related to a problem? Please describe.

According to the documentation there is currently one Description section on a DocsPage. This section is filled by the JSDoc comment /** */ used by my specified component. That's awesome. I can place a general description here. (E.g. describe what my component <Button />.)

After that I have more specific stories showcasing different usages of a certain component. (E.g. showcasing <Button mode="primary" /> and <Button mode="secondary" />.)

It would be lovely if I could add a JSDoc comment to my stories so they show up on the DocsPage. E.g. something like that:

import React from 'react';
import { Button } from '../../src/components/button';

export default { title: 'Components|Button', component: Button };

export const All = () => (
  <>
    <Button mode="primary">Click me!</Button>
    {' '}
    <Button mode="secondary">Click me!</Button>
  </>
);

/**
 * Only use me once per page for the preferred user action.
 */
export const Primary = () => <Button mode="primary">Click me!</Button>;

/**
 * You can use me several times on a page for neutral actions.
 */
export const Secondary = () => <Button mode="secondary">Click me!</Button>;

Describe the solution you'd like

Everything could than be rendered like this:

image

Describe alternatives you've considered

I could use MDX files for this, but they currently lack any type checking which I'd like to keep (and I don't really want to separate every example out of the MDX file into their own files, because I don't know if this messes with things like the Source Loader for code examples 🤔). Also it's a little bit easier to migrate to for existing Storybooks instead of rewriting everything to MDX.

Are you able to assist bring the feature to reality?

Probably, I can if I find a little bit of time.


IssueHunt Summary

shilman shilman has been rewarded.

Backers (Total: $100.00)

Submitted pull Requests


Tips

@shilman
Copy link
Member

shilman commented Oct 23, 2019

@donaldpipowitch Yes, I think this is the ideal way to do story descriptions in docs, but haven't had time to put it together yet. It would be amazing if you could figure it out.

In the meantime, there's a not as nice workaround by adding the story parameter parameters.docs.storyDescription to each story. The docgen would be much nicer though!

@donaldpipowitch
Copy link
Contributor Author

@shilman Ah, that is interesting! storyDescription is not documented, right? Thank you for the workaround.

@shilman
Copy link
Member

shilman commented Oct 23, 2019

It is now 😉 #8535

The jsdoc solution you describe (and i've also wanted for awhile) is 100x better, though a bit of work to figure out.

Actually as I'm writing this, it occurs to me that it could be a pretty trivial webpack loader that either annotates the export with some __docgenInfo kind of field, or even automatically inserts the storyDescription parameter based on the comment... any interest in making that happen?

@donaldpipowitch
Copy link
Contributor Author

I'd probably try to solve #8126 first, before I can tackle this one. But that won't happen within the next two weeks I guess. (Deadline approaching😅)

But after that. Sure, why not. I guess I can also have a look how the JSDoc is currently parsed from the component itself for the main instruction to check how it was done there.

@stale stale bot added the inactive label Nov 14, 2019
@shilman shilman added the todo label Nov 14, 2019
@stale stale bot removed the inactive label Nov 14, 2019
@storybookjs storybookjs deleted a comment from stale bot Nov 14, 2019
@Smolations
Copy link

@donaldpipowitch Are you still planning to do this? It would be super awesome because my team would prefer writing stories with default syntax rather than using MDX files. 😁

@donaldpipowitch
Copy link
Contributor Author

Probably not within the next two weeks or so, but in general I'm still open for this. But if anyone else wants to tackle this first just go ahead 👍

@donaldpipowitch
Copy link
Contributor Author

But shouldn't the workaround work for you as well?

@Smolations
Copy link

@donaldpipowitch It does, but I much prefer the comment solution. The workaround isn't great for large blocks of text, which I would define like so:

MyStoryFunction.story = {
  parameters: { 
    docs: {
      storyDescription: `
        Imagine this to be a much longer block of text that spans
        several lines.
      `,
    },
  },
};

Unfortunately, using template literals results in the description being rendered as inline code, so it's all monospaced and whatnot. I have resort to using quotes, which I think we all know are lame to use for multiline blocks of text. 😋
Screen Shot 2020-01-08 at 11 39 20 AM

@shilman
Copy link
Member

shilman commented Jan 9, 2020

@Smolations you can use dedent or similar to fix that in the short term

@Smolations
Copy link

@shilman The main problem is that the description is rendered as a code block. If rendered as plain text the indentation issue becomes moot. And speaking of indentation issues, since all of my stories all defined as export function StoryName() { return (...jsx...); }, all of my "Show Code" blocks are missing a single indentation for all of the content in the function definition:

export function StoryName() { 
return (
  // ...jsx (correctly indented)...
); 
}

Just wanted to give you a lil heads up, but I can open a separate issue if you'd like me to formalize the problem. 👍

@shilman
Copy link
Member

shilman commented Jan 11, 2020

@Smolations AFAIK the descriptions are rendered as markdown.

There's already an issue for code formatting. If you up vote it that's the least duplicative way of letting us know you think it's important. If it really bothers you, PR's are always welcome in storybook

@Smolations
Copy link

Small update on this @shilman . Looks like my above storyDescription issue only happens when using multiline template literals defined inside the parameters block itself. So, to summarize:

// renders in a code block; no bueno
MyStoryFunction.story = {
  parameters: { 
    docs: {
      storyDescription: `
        Imagine this to be a much longer block of text that spans
        several lines.
      `,
    },
  },
};
// renders as normal text, as desired
MyStoryFunction.story = {
  parameters: { 
    docs: {
      storyDescription: `Imagine this to be a much longer block of text that spans several lines.`,
    },
  },
};
// renders as normal text, as desired (and my chosen approach for this in the near term)
const myStoryFunctionDescription = `
  Imagine this to be a much longer block of text that spans
  several lines.
`;

MyStoryFunction.story = {
  parameters: { 
    docs: {
      storyDescription: myStoryFunctionDescription,
    },
  },
};

Interesting that there are different results for these approaches, but it is what it is, haha. Now I have less anxiety around this as I eagerly await @donaldpipowitch to tackle this issue's feature request if/when he has time! 😋

@shilman
Copy link
Member

shilman commented Feb 12, 2020

@Smolations I think the indentation => code is a markdown thing. I'm guessing it's 3 spaces, which is why the first one is treated as code and third one as text. We use ts-dedent in storybook -- worth checking out.

I'm also really excited about this feature and will tackle this once I work through my own backlog if nobody else gets to this sooner. (wink wink @donaldpipowitch)

@donaldpipowitch
Copy link
Contributor Author

Oh that peer pressure 😆 I gave that some thoughts yesterday and have a couple of small questions.

... it could be a pretty trivial webpack loader that ... automatically inserts the storyDescription parameter based on the comment

  1. Is there a reason why you thought about a webpack loader instead of a babel plugin here? Is it because not all files which contain stories are currently processed by babel? If yes - are there plans to change that in the future?
  2. I had a look at existing loaders - afaik it's only @storybook/source-loader, right? Any reason why it lives in the lib/ directory even though it is specific to the "the storysource and docs addons". Should this new loader live in lib/ as well?
  3. I thought about how to name this loader... and thought to myself "Well, it'll transform files containing stories, so it will probably be the story-loader.", because I didn't want to make it too specific (e.g. descriptions-per-story-loader 😋) so it can be re-used later for other use cases. And than I thought: "Wait a minute. The source-loader should also only transform story files!" It is not meant to transform the sources of the things I document in Storybook (which we mostly refer to as the source code and which often lives inside src/ directories). Maybe that was one of the reasons which led to my previous MR here: Source-loader: Warn if applied to non-stories file #8773. tl;dr: Would it make sense to generalize the source-loader and rename it to story-loader? It could potentially be less confusing and easier to extend for uses cases like adding descriptions per story.
  4. If we have such a general loader for Storybook (which than also makes more sense to live inside lib/) - do we maybe want to auto-apply it to the webpack configs? I think currently you need manually add this which can also lead to smaller problems. E.g. as mentioned here https://github.com/storybookjs/storybook/blob/078366baefc781ea8abc84de48872ff3d6ce8dda/addons/storysource/README.md#install-using-preset the default to which the loader applies is test: [/\.stories\.jsx?$/],. That didn't matched to my config which led to the misconfiguration in Source-loader: Warn if applied to non-stories file #8773. But we could essentially just re-use module.exports.stories from .storybook/main.js here, because here we already have one place where we configure all of our stories. (I'd not tackle this question to solve this specific issue. It would be an issue on its own and I could create one, if you think it's worthy to track.)

@shilman
Copy link
Member

shilman commented Feb 13, 2020

@donaldpipowitch I like the way you think 😅

  1. I don't have a strong preference here. For react-docgen we use a babel plugin and for vue-docgen-api we use a webpack loader. For source-loader it's webpack. I don't fully understand which is better, but webpack seems to be a bit easier to configure in Storybook. Happy to be persuaded otherwise!
  2. Yeah source-loader is a library that's shared between two addon, so it's in lib 😄.
  3. Yeah that sounds pretty reasonable to me. @libetl @atanasster any thoughts about generalizing to story-loader?
  4. Currently the addon-docs and addon-storysource presets register source-loader. I think that you shouldn't have to "pay the price" for source-loader unless you're using an addon that uses it. But I think that configuring it using the user's main.js stories configuration would be wonderful if we can do it cleanly.

@donaldpipowitch
Copy link
Contributor Author

donaldpipowitch commented Feb 17, 2020

@shilman I had a look at the source-loader code. I saw estraverse.traverse is used a lot and that you needed to use fallback: 'iteration'. There were also some helper to get the correct parser (JS/TS/Flow)... To add a description to a story it would probably be way more straight forward to have a babel plugin which can be added to an existing babel config (either completely manually or somehow automatic - maybe in lib/core/src/server/common/babel.js...?.)

The only thing which is maybe not straight forward to do is to limit the transformation to files containing stories. AFAIK there is nothing which all files containing stories really have in common. It would be wonderful to have just some isStoryFile(somePath) API in the core which checks if a certain file contains stories or not. Not sure if this is possible, if we offer more ways than just main.js stories to load stories.

tl;dr:

  1. Maybe source-loader is a bad example to build upon as it needs to access and process the raw source code, so it can be included as source code example. That's why you have to dance around the parsers a little bit.
  2. I'd recommend to create a Babel plugin which needs to be added manually and which needs to have a config equivalent to main.js stories as one of the options it needs.
  3. At a later point in time one could maybe optimize this by automatically adding and automatically configuring this plugin?

Is that fine for you? Than I'd start writing plugin in lib/story-plugin.

If not, I'd probably just write and publish a custom plugin to do that just as a proof of concept 🤔

@shilman
Copy link
Member

shilman commented Feb 17, 2020

@donaldpipowitch what are the implications of fallback: 'iteration'?

Implementing this as a babel-plugin is a-okay with me. 💯

As it happens, I'm spending a bunch of with babel these days (e.g. #9838). We're switching over to babel-plugin-react-docgen for all the react prop table generation (js, ts, flow), and it will really simplify that whole area.

I'm not sure how we'll get users to configure it yet, but we'll have to solve that one way or another, and having a second babel plugin gets more brains to the table.

As far as naming, babel-story-description-plugin or babel-story-plugin is more specific. But feel free to keep it broad if you have more designs for extending it.

@donaldpipowitch
Copy link
Contributor Author

@donaldpipowitch what are the implications of fallback: 'iteration'?

A never mind. I think I confused estraverse here with a different lib. Sorry about that.

@izhan
Copy link

izhan commented Mar 3, 2020

Hey folks – long-time watcher of this thread here! I ended up creating a trivial Webpack loader a while back that got the job done. I only now had some extra time to put it on Github, which you can find here.

Code

// A `primary` button is used for emphasis.
export const Primary = () => <Button primary>Submit</Button>

/**
 * Use the `loading` prop to indicate progress. Typically use
 * this to provide feedback when triggering asynchronous actions.
 */
export const Loading = () => <Button loading>Loading</Button>

Output

Definitely not perfect and a bit specific to my codebase's current setup, hence some quirks here and there. But figured I'd send it out in case anyone was looking for a starting point.

Also happy to improve on it more, but looks like there's bigger, better plans brewing in this thread. Eager to help there too :)

@IamMille
Copy link

Why did this get closed @shilman?

@shilman
Copy link
Member

shilman commented Apr 13, 2020

@IamMille it's open?

@issuehunt-oss
Copy link

issuehunt-oss bot commented Jan 15, 2021

@kylemh has funded $100.00 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Jan 15, 2021
@yairEO
Copy link
Contributor

yairEO commented Jun 2, 2021

@trouba:

is it possible to pass some mdx to the component description ?

Yes, it's possible easily using raw-loader:

import notes from '!raw-loader!./notes.mdx'

export default {
  ...
  parameters: {
    docs: { 
        description: { 
            component: notes  
        } 
    },
  }
}

notes is now a variable pointing to the raw contents (string) of notes.mdx file

Ofc you can also add the parameters per-story export also:

// imagine a stories file with multiple stories such as this:
export const MyStory = (args) => <MyComponent {...args}/>
MyStory.storyName = 'My cool story';
MyStory.parameters = {
    docs: {
        storyDescription: notes // sadly accepts only String and not a function (component)
    },
}

@yairEO
Copy link
Contributor

yairEO commented Jun 8, 2021

Functions can be passed as values to parameters.docs.page:

export default {
    ...
    parameters: {
        docs: {
            page: () => (
                <>
                    <Title>My Story</Title>
                    <Description markdown={docs}/>
                    Whatever else..
                </>
            ),
        },
    },
}

But not for a specific story's description parameters.docs.description:

MyStory.parameters = {
    docs: {
        description: {
            // "story" property should also accepts a function, just like "page" for default export
            story: () => (
                <>
                    lots of complex things goes here
                </>
            ),
        },
    },
}

It would be great for story property to accept String or Function as values, for more control over story-specific descriptions

@matthew-dean
Copy link

It's unclear from this thread whether I should use docs.description.story or docs.storyDescription but neither one rendered anything. 🤷‍♂️

@shilman
Copy link
Member

shilman commented May 14, 2022

@matthew-dean working example: code / storybook

@matthew-dean
Copy link

@shilman I figured it out. I ran into the other bug (called a feature in another thread), where the first story doesn't render its description. 🤷‍♂️ This is pretty counter-intuitive to expectations, but I was able to circumvent it by rendering a custom DocsPage layout.

@shilman
Copy link
Member

shilman commented Nov 1, 2022

Olé!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-alpha.47 containing PR #19684 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Nov 1, 2022
@kylemh
Copy link
Member

kylemh commented Nov 1, 2022

Hey @shilman would you like the bounty to go to you, SB, or to @izhan who I saw you reference in the PR?

@izhan
Copy link

izhan commented Nov 1, 2022

@kylemh it 100% should not go to me, @shilman did the actual work here :)

thank you @shilman for adding this feature!

@kylemh
Copy link
Member

kylemh commented Nov 1, 2022

alrighty! just claim it by clicking the badge on OP @shilman

@issuehunt-oss
Copy link

issuehunt-oss bot commented Nov 2, 2022

@shilman has rewarded $90.00 to @shilman. See it on IssueHunt

  • 💰 Total deposit: $100.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $10.00

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Nov 2, 2022
@shilman
Copy link
Member

shilman commented Nov 2, 2022

@izhan next time i'm in SF, dinner's on me -- thank you for paving the way on this one!!! ❤️

@kylemh thanks so much for funding this -- if you make it to Taiwan on your world travels, dinner's on me here too ❤️

@rebelchris
Copy link

Sorry to reply to this closed issue, I'm on 7.alpha.50 and use comment like this:

// Some comment about this
export const LoggedOut = Template.bind({});
LoggedOut.args = {};

But it doesn't get generated, am I missing some setup?

@WalterWeidner
Copy link

WalterWeidner commented Mar 15, 2023

@shilman I figured it out. I ran into the other bug (called a feature in another thread), where the first story doesn't render its description. 🤷‍♂️ This is pretty counter-intuitive to expectations, but I was able to circumvent it by rendering a custom DocsPage layout.

@matthew-dean can you point me to that thread? I'd also be interested in your workaround. This is a pretty frustrating feature that is making it challenging to convey examples in the way I need.

@Staijn1
Copy link

Staijn1 commented Jun 8, 2023

Sorry to reply to this closed issue, I'm on 7.alpha.50 and use comment like this:

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

But it doesn't get generated, am I missing some setup?

I am also running into the same issue and all links to examples mentioned above are broken.
Have you been able to find a fix?

I am on Storybook 7.0.20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests