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.

Sign up for GitHub

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

Jump to bottom

Fix for issue 5932, adding default options to action(s) #6438

Merged
merged 3 commits into from Jun 7, 2019
Merged

Fix for issue 5932, adding default options to action(s) #6438

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e287bd
fix for issue 5932, adding default options to action(s)
LFDanLu Apr 5, 2019
16670df
Merging user options with global options instead of replace
LFDanLu Apr 5, 2019
ad73162
Merge branch 'next' into pr/LFDanLu/6438
ndelangen Apr 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
130 changes: 64 additions & 66 deletions addons/actions/src/preview/__tests__/action.test.js
View file
Open in desktop
@@ -1,6 +1,5 @@
import addons from '@storybook/addons';
import { action } from '../..';
// import { configureActions } from '../..';
import { action, configureActions } from '../..';

jest.mock('@storybook/addons');

Expand Down Expand Up @@ -29,67 +28,66 @@ describe('Action', () => {
});
});

// TODO: This functionality is removed, unsure if to add back or remove
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw that this test was commented out and covers the exact functionality that I was re-adding via this pull. Couldn't find the old implementation nor the reasoning for it's removal so if this was removed for a specific reason I would appreciate a pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Scratch that found the pull (#4086), but it is quite a large one so I'm still unable to glean the reason for the initial removal. Gonna change my implementation to mirror how user provided config options were merged with the config supplied by configureActions)

// describe('Depth config', () => {
// it('with global depth configuration', () => {
// const channel = createChannel();

// const depth = 1;

// configureActions({
// depth,
// });

// action('test-action')({
// root: {
// one: {
// two: 'foo',
// },
// },
// });

// expect(getChannelData(channel)[0]).toEqual({
// root: {
// one: {
// two: 'foo',
// },
// },
// });
// });

// it('per action depth option overrides global config', () => {
// const channel = createChannel();

// configureActions({
// depth: 1,
// });

// action('test-action', { depth: 3 })({
// root: {
// one: {
// two: {
// three: {
// four: {
// five: 'foo',
// },
// },
// },
// },
// },
// });

// expect(getChannelData(channel)[0]).toEqual({
// root: {
// one: {
// two: {
// three: {
// four: {
// five: 'foo',
// },
// },
// },
// },
// },
// });
// });
// });
describe('Depth config', () => {
it('with global depth configuration', () => {
const channel = createChannel();

const depth = 1;

configureActions({
depth,
});

action('test-action')({
root: {
one: {
two: 'foo',
},
},
});

expect(getChannelData(channel)[0]).toEqual({
root: {
one: {
two: 'foo',
},
},
});
});

it('per action depth option overrides global config', () => {
const channel = createChannel();

configureActions({
depth: 1,
});

action('test-action', { depth: 3 })({
root: {
one: {
two: {
three: {
four: {
five: 'foo',
},
},
},
},
},
});

expect(getChannelData(channel)[0]).toEqual({
root: {
one: {
two: {
three: {
four: {
five: 'foo',
},
},
},
},
},
});
});
});
3 changes: 2 additions & 1 deletion addons/actions/src/preview/action.ts
View file
Open in desktop
Expand Up @@ -2,8 +2,9 @@ import uuid from 'uuid/v1';
import { addons } from '@storybook/addons';
import { EVENT_ID } from '../constants';
import { ActionDisplay, ActionOptions, HandlerFunction } from '../models';
import { config } from './configureActions';

export function action(name: string, options: ActionOptions = {}): HandlerFunction {
export function action(name: string, options: ActionOptions = config): HandlerFunction {
const actionOptions = {
...options,
};
Expand Down
3 changes: 2 additions & 1 deletion addons/actions/src/preview/actions.ts
View file
Open in desktop
@@ -1,8 +1,9 @@
import { action } from './action';
import { ActionOptions, ActionsMap } from '../models';
import { config } from './configureActions';

export function actions(...args: any[]): ActionsMap {
let options: ActionOptions = {};
let options: ActionOptions = config;
const names = args;
// last argument can be options
if (names.length !== 1 && typeof args[args.length - 1] !== 'string') {
Expand Down