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

Refactor transitional decorator from addon-notes #3559

Merged
merged 8 commits into from
May 15, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion addons/notes/src/__tests__/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import addons from '@storybook/addons';
import { withNotes } from '..';

jest.mock('@storybook/addons');
addons.getChannel = jest.fn();

describe('Storybook Addon Notes', () => {
it('should inject text from `notes` parameter', () => {
Expand Down
34 changes: 11 additions & 23 deletions addons/notes/src/index.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
import addons from '@storybook/addons';
import addons, { makeTransitionalDecorator } from '@storybook/addons';
import marked from 'marked';

function renderMarkdown(text, options) {
marked.setOptions({ ...marked.defaults, options });
return marked(text);
}

const decorator = options => {
const channel = addons.getChannel();
return (getStory, context) => {
const {
parameters: { notes },
} = context;
const storyOptions = notes || options;
export const withNotes = makeTransitionalDecorator({
name: 'withNotes',
parameterName: 'notes',
wrapper: (getStory, context, { options, parameters }) => {
const channel = addons.getChannel();

const storyOptions = parameters || options;

if (storyOptions) {
const { text, markdown, markdownOptions } =
Expand All @@ -26,23 +26,11 @@ const decorator = options => {
}

return getStory(context);
};
};

const hoc = options => story => context => decorator(options)(story, context);
},
});

export const withMarkdownNotes = (text, options) =>
hoc({
withNotes({
markdown: text,
markdownOptions: options,
});

export const withNotes = (...args) => {
// Used without options as .addDecorator(withNotes)
if (typeof args[0] === 'function') {
return decorator()(...args);
}

// Input are options, ala .add('name', withNotes('note')(() => <Story/>))
return hoc(args[0]);
};
3 changes: 2 additions & 1 deletion lib/addons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
},
"dependencies": {
"@storybook/channels": "4.0.0-alpha.4",
"global": "^4.3.2"
"global": "^4.3.2",
"util-deprecate": "^1.0.2"
}
}
1 change: 1 addition & 0 deletions lib/addons/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import global from 'global';

export mockChannel from './storybook-channel-mock';
export { makeTransitionalDecorator } from './transitional-decorator';

export class AddonStore {
constructor() {
Expand Down
40 changes: 40 additions & 0 deletions lib/addons/src/transitional-decorator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import deprecate from 'util-deprecate';

// Create a decorator that can be used both in the (deprecated) old "hoc" style:
// .add('story', decorator(options)(() => <Story />));
//
// And in the new, "parameterized" style:
// .addDecorator(decorator)
// .add('story', () => <Story />, { name: { parameters } });

export const makeTransitionalDecorator = ({ name, parameterName, wrapper }) => {
Copy link
Member

Choose a reason for hiding this comment

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

Should we rename it to something like makePolymorphicDecorator?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure! Or just makeDecorator()?

Copy link
Member

Choose a reason for hiding this comment

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

just makeDecorator sounds good

Copy link
Member

Choose a reason for hiding this comment

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

I like the makeDecorator name too

const decorator = options => (getStory, context) => {
const parameters = context.parameters && context.parameters[parameterName];

return wrapper(getStory, context, {
options,
parameters,
});
};

return (...args) => {
// Used without options as .addDecorator(decorator)
if (typeof args[0] === 'function') {
return decorator()(...args);
}

return (...innerArgs) => {
// Used as [.]addDecorator(decorator(options))
if (typeof innerArgs.length > 1) {
Copy link
Member

Choose a reason for hiding this comment

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

typeof?

Copy link
Member Author

Choose a reason for hiding this comment

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

wow :0

Copy link
Member Author

Choose a reason for hiding this comment

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

I need to add a test for this modality, it just doesn't really make sense for notes. I'll do another PR for backgrounds and we'll definitely have one.

return decorator(...args)(...innerArgs);
}

// Used to wrap a story directly .add('story', decorator(options)(() => <Story />))
// This is now deprecated:
return deprecate(
context => decorator(...args)(innerArgs[0], context),
`Passing stories directly into ${name}() is deprecated, instead use addDecorator(${name}) and pass options with the '${parameterName}' parameter`
);
};
};
};