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

Use parameters for info addon #3697

Merged
merged 15 commits into from
Jun 9, 2018
Merged
84 changes: 82 additions & 2 deletions addons/info/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,94 @@ storiesOf('Component', module)

## Usage as decorator

It is possible to add infos by default to all components by using a global or story decorator. The drawback is you won't be able to display a distinct info message per story.
It is possible to add infos by default to all components by using a global or story decorator.

It is important to declare this decorator as **the first decorator**, otherwise it won't work well.

```js
addDecorator((story, context) => withInfo('common info')(story)(context));
addDecorator(withInfo);
Copy link
Member

Choose a reason for hiding this comment

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

I think it is worth indicating this should be done in .storybook/config.js

```

Then, you can use the `info` parameter to pass certain options to your stories.
This can be done per book of stories:

```js
import { storiesOf } from '@storybook/react';

import Component from './Component';

storiesOf('Component', module)
.addParameters({
info: {
// Your settings
}
})
.add('with some emoji', () => <Component/>);
```

...or for each story individually:
```js
import { storiesOf } from '@storybook/react';

import Component from './Component';

storiesOf('Component', module)
.add(
'with some emoji',
() => <Component emoji/>,
{ info : { inline: false, header: false } } // Make your component render inline with the additional info
)
.add(
'with no emoji',
() => <Component/>,
{ info: '☹️ no emojis' } // Add additional info text
);
```

...or even together:

```js
import { storiesOf } from '@storybook/react';

import Component from './Component';

storiesOf('Component', module)
.addParameters({
info: { // Make a default for all stories in this book,
inline: true, // where the components are inlined
styles: {
header: {
h1: {
color: 'red' // and the headers of the sections are red.
}
}
},
}
})
.add(
'green version',
() => <Component green/>,
{
info: {
styles: {
headers: {
h1: {
color: 'green' // Still inlined but with green headers!
}
}
}
}
})
.add(
'something else',
() => <Component different/>,
{
info: "This story has additional text added to the info!" // Still inlined and with red headers!
}
);
```


## Global options
Copy link
Member

Choose a reason for hiding this comment

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

Let's deprecate this and the usage as a HoC ("Basic usage" and "Usage with options") -- so let's remove it from the docs.

Copy link
Member

Choose a reason for hiding this comment

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

@tmeasday on a related note, do you think we should have a Deprecated section in storybook-official so we still get story snapshots for deprecated usage, but they are separate from the recommended usage examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tmeasday Just so we're on the same line: you want to deprecate the setDefaults way of setting defaults (and shift that to the addDecorator in the config.js with either options or parameters) and remove all the docs involving this + the HoC?

Related to this, should I then rewrite all the existing info stories in official-storybook to use the parameterized way?

Copy link
Member

@tmeasday tmeasday Jun 4, 2018

Choose a reason for hiding this comment

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

Sorry guys, missed this!

should have a Deprecated section in storybook-official so we still get story snapshots for deprecated usage, but they are separate from the recommended usage examples?

Probably a good idea @shilman -- perhaps a small refactor of the stories after we get the last few PRs merged is in order.

Just so we're on the same line: you want to deprecate the setDefaults way of setting defaults (and shift that to the addDecorator in the config.js with either options or parameters) and remove all the docs involving this + the HoC?

Yeah that was what I was thinking, do you agree? Definitely get rid of the HoC; and setDefaults seems redundant now, right?

Related to this, should I then rewrite all the existing info stories in official-storybook to use the parameterized way?

What I've been doing is leaving the existing ones as "tests" for the deprecated functionality (we can remove them when actually delete that functionality). Then making sure there are stories to cover all the new ways of doing it! You don't need to re-test that parameter inheritance works though (although maybe adding a the story in core.stories.js to test your change to the way it works?), just add stories that make sense.

I hope I'm making sense here 😬


To configure default options for all usage of the info option, use `setDefaults` in `.storybook/config.js`:
Expand Down
1 change: 1 addition & 0 deletions addons/info/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"prepare": "node ../../scripts/prepare.js"
},
"dependencies": {
"@storybook/addons": "4.0.0-alpha.8",
"@storybook/client-logger": "4.0.0-alpha.8",
"@storybook/components": "4.0.0-alpha.8",
"babel-runtime": "^6.26.0",
Expand Down
16 changes: 12 additions & 4 deletions addons/info/src/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import nestedObjectAssign from 'nested-object-assign';
import { makeDecorator } from '@storybook/addons';
import { logger } from '@storybook/client-logger';
import Story from './components/Story';
import PropTable from './components/PropTable';
Expand Down Expand Up @@ -82,10 +83,17 @@ function addInfo(storyFn, context, infoOptions) {
return <Story {...props}>{storyFn(context)}</Story>;
}

export const withInfo = textOrOptions => {
const options = typeof textOrOptions === 'string' ? { text: textOrOptions } : textOrOptions;
return storyFn => context => addInfo(storyFn, context, options);
};
export const withInfo = makeDecorator({
name: 'withInfo',
parameterName: 'info',
wrapper: (getStory, context, { options, parameters }) => {
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to skip the addon if they don't set options or parameters? Either by passing skipIfNoOptionsOrParameters: true or manually doing it here..

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise (or maybe anyway) we should document setting { info: { disable: true } }

Copy link
Contributor Author

@Keraito Keraito Jun 2, 2018

Choose a reason for hiding this comment

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

Def agree with the disable: true part, will do. I'm not so sure about the skipIfNoOptionsOrParameters part though. I assume the both of them do the same thing, namely disable the addon entirely. But info is slightly different from the others addons imo that it does something even without configuration: display Proptypes, the component, etc.

Example: https://github.com/storybooks/storybook/pull/3697/files#diff-c915ecbefba3a2734ebd93eb258431dbR353 This is basically an parameters- and options-less story, besides some minor options (header styling) and parameters (inline and manual text). The configuration don't add a lot, but the addon itself adds a lot of information stand-alone already.

So I would say just only documenting the disable flag should be the only way

Copy link
Member

Choose a reason for hiding this comment

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

Ok, makes sense. Thanks for thinking about that.

const storyOptions = parameters || options;
const infoOptions = typeof storyOptions === 'string' ? { text: storyOptions } : storyOptions;
const mergedOptions =
typeof infoOptions === 'string' ? infoOptions : Object.assign(options, infoOptions);
return addInfo(getStory, context, mergedOptions);
},
});

export { Story };

Expand Down
36 changes: 36 additions & 0 deletions examples/official-storybook/stories/addon-info.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,39 @@ storiesOf('Addons|Info.Options.maxPropsIntoLine === 3', module).add(
/>
))
);

storiesOf('Addons|Info.Options.parameters', module)
.addDecorator(
withInfo({
styles: {
header: {
h1: {
color: 'green',
},
},
},
})
)
.addParameters({
info: {
text:
'This text should be displayed on every story and the component should be inlined between description and PropType table',
inline: true, // Displays info inline vs click button to view
},
})
.add('Using paramaters across all stories', () => <BaseButton label="Button" />)
.add(
'Overwriting and extending the parameters and options set stories-wise',
() => <BaseButton label="Button" />,
{
info: {
text: 'Label propType should be excluded',
excludedPropTypes: ['label'],
},
}
)
.add(
'Overwrite the parameters with markdown',
() => <BaseButton onClick={action('clicked')} label="Button" />,
{ info: markdownDescription }
);
26 changes: 20 additions & 6 deletions lib/core/src/client/preview/client_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,27 @@ export default class ClientApi {

const fileName = m ? m.id : null;

// Add the fully decorated getStory function.
this._storyStore.addStory(kind, storyName, this._decorateStory(getStory, decorators), {
...this._globalParameters,
...localParameters,
...parameters,
fileName,
const allParam = { fileName };

[this._globalParameters, localParameters, parameters].forEach(params => {
if (params) {
Object.keys(params).forEach(key => {
if (typeof params[key] === 'object') {
allParam[key] = Object.assign({ ...allParam[key] }, params[key]);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just { ...allParam[key], ...params[key] }? Kind of odd to use Object.assign and object spread ;)

} else {
allParam[key] = params[key];
}
});
}
});

// Add the fully decorated getStory function.
this._storyStore.addStory(
kind,
storyName,
this._decorateStory(getStory, decorators),
allParam
);
return api;
};

Expand Down