Addon-Actions: Add preset and configure with parameters #9933

jonspalmer · 2020-02-22T17:25:00Z

What I did

Add a preset for Actions Addon
Changed Action Addon to be configured via parameters
Simplified the Server app to remove the special case actions code in favor of parameters.

examples/official-storybook/stories/addon-actions.stories.js

shilman

LGTM aside from disable not working. @tmeasday do you know the fix off the top of your head?

jonspalmer · 2020-02-23T23:32:53Z

addons/actions/src/preview/withActions.ts


+export const createDecorator = (actionsFn: any) => (...args: any[]) => (storyFn: () => any) => {
+  applyEventHandlers(actionsFn, ...args)
+
  return storyFn();
 };


createDecorator is used by the decorate options. I'm a bit fuzzy on that use case but we have examples of it in a few examples.

addons/actions/src/preview/withActions.ts

jonspalmer · 2020-02-23T23:35:00Z

app/server/src/lib/compiler/__testfixtures__/actions.snapshot

@@ -1,8 +1,7 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

 exports[`json-to-csf-compiler actions.json 1`] = `
-"import { withActions } from '@storybook/addon-actions';
-
+"


Making @tmeasday happy to see the imports go away!

tmeasday

Just a bunch of thoughts, I'm not quite sure of the exact way forward

MIGRATION.md

tmeasday · 2020-02-24T00:24:14Z

addons/actions/README.md


-You can define action handles in a declarative way using `withActions` decorators. It accepts the same arguments as [`actions`](#multiple-actions)


I don't really understand this line, it doesn't seem correct ("It accepts the same arguments as [actions]"). I realise this was here before but it doesn't seem right at all. The arguments are very much different.

addons/actions/src/preview/withActions.ts

tmeasday · 2020-02-24T00:39:22Z

addons/actions/src/preview/withActions.ts


+export const createDecorator = (actionsFn: any) => (...args: any[]) => (storyFn: () => any) => {


How is this used? I think we should drop it and just allow deprecated usage below..

It ends up getting used like this example:

https://github.com/storybookjs/storybook/blob/next/examples/html-kitchen-sink/stories/addon-actions.stories.js#L40

Feels weird to me but maybe I'm not understanding the use case.

Oh, wow that's pretty weird. I suspect this is a super old usage pattern from before we had support for story-level decorators, this was the only way to apply a decorator to a single story.

I'm not sure we need to keep supporting it. WDYT @shilman?

Yeah I think we should drop support for this too.

There isn't a great way to remove this without removing the ability to manipulate the event data for frameworks like html and web components.

The problem is if you use parameters then the presetDecorator will apply but it would have the customer actionFunctions that decorate otherwise adds. We can't use parameters to supply the actionFunctions as they have to be serializable (so args v1 won't help here either).

Is there some other mechanism where we can 'configure' the decorator to use a different set of action functions? @tmeasday @shilman ?

@jonspalmer I don't really understand what that use case is doing.

@tmeasday I'm not too sure of it either. It looks like we went out of our way to allow people to manipulate the event arguments. I'm not clear why you want that.

In React land (or some other JS component framework) if you wanted to manipulate the args of the event handler you could do this without any special support from the actions addon:

import { action } from '@storybook/addon-actions'; import Button from './button'; export default { title: 'Button', component: Button, }; const clickAction = action('click'); export const first = () => ( <Button onClick={(...args) => clickAction(args[0])}>Hello World!</Button> );

However in html or web components we can't do it in the story function. So the equivalent is:

import { decorate } from '@storybook/addon-actions'; const firstArg = decorate([args => [args[0]]]); export const first = () => firstArg.withActions('click')( () => `<button type="button">Hello World!</button>`; );

If its important to support the manipulate of event args I don't think we have a great way to maintain support other than this approach. However, I could see us dropping it entirely.

After a conversation with @shilman we agreed the decorate API is YAGNI. I've removed it and updated the docs.

Thanks for explaining it to me anyway @jonspalmer, I get it now ;)

jonspalmer · 2020-02-24T03:07:22Z

addons/actions/src/preview/__tests__/actions.test.js

@@ -0,0 +1,90 @@
+import addons from '@storybook/addons';
+import { actions } from '../..';


I feel a lot better having a test of some of the new logic. Not sure if this is the best factoring. Some prior art from the action.test. Could perhaps make it a little less verbose but...

Co-Authored-By: Tom Coleman <tom@thesnail.org>

jonspalmer · 2020-03-24T02:50:01Z

@tmeasday @shilman I've rebased from next. I think this is good to go. Looks like CircleCI is borked - Do we have to pay them money to get more credits?

shilman · 2020-03-24T06:49:18Z

Awesome @jonspalmer!! 💯 Re: CircleCI, I think @ndelangen 's running point on this. Hopefully we can get it sorted out quickly 🤞

shilman

🥇

ndelangen · 2020-03-25T08:05:16Z

We're sponsored by CircleCI, but something is mis-configured. They are working on it to resolve this.

jonspalmer requested a review from shilman February 22, 2020 17:25

jonspalmer requested a review from usulpro as a code owner February 22, 2020 17:25

jonspalmer commented Feb 22, 2020

View reviewed changes

examples/official-storybook/stories/addon-actions.stories.js Show resolved Hide resolved

jonspalmer added addon: actions feature request labels Feb 22, 2020

jonspalmer mentioned this pull request Feb 22, 2020

Server: Simplify server addons #9931

Merged

shilman reviewed Feb 23, 2020

View reviewed changes

jonspalmer changed the title ~~Add Preset for Actions Addon~~ Actions Addon - Add Preset for and configure with Parameters Feb 23, 2020

jonspalmer commented Feb 23, 2020

View reviewed changes

addons/actions/src/preview/withActions.ts Show resolved Hide resolved

jonspalmer commented Feb 23, 2020

View reviewed changes

tmeasday reviewed Feb 24, 2020

View reviewed changes

jonspalmer requested review from ndelangen and theinterned as code owners February 24, 2020 03:03

jonspalmer commented Feb 24, 2020

View reviewed changes

jonspalmer force-pushed the actions_addon_preset branch 6 times, most recently from ed3c994 to 6b4f35f Compare February 25, 2020 18:39

jonspalmer added the BREAKING CHANGE label Feb 25, 2020

jonspalmer force-pushed the actions_addon_preset branch from 6b4f35f to 2fcc21d Compare February 25, 2020 19:06

jonspalmer added this to the 6.0.0 milestone Feb 25, 2020

shilman added the ci: do not merge label Feb 26, 2020

jonspalmer force-pushed the actions_addon_preset branch from 2fcc21d to 1e2d8ea Compare February 29, 2020 12:48

jonspalmer added 4 commits March 23, 2020 21:56

Add Preset for Actions Addon

7c25d25

fix preset

3ed9601

Configure actions with parameters

e3a0c22

linting and test fixes

7c9d186

jonspalmer and others added 6 commits March 23, 2020 22:08

support deprecated withActions syntax

b40cffb

Update MIGRATION.md

fc999d0

Co-Authored-By: Tom Coleman <tom@thesnail.org>

allowDeprecatedUsage

487bd06

cleanup

c7acdae

remove the decorate APIs

4931395

fix snapshot

195d26b

jonspalmer force-pushed the actions_addon_preset branch from 1e2d8ea to 195d26b Compare March 24, 2020 02:29

Cleanup docs

fab01ec

shilman approved these changes Mar 24, 2020

View reviewed changes

Merge branch 'next' into actions_addon_preset

e7f3617

ndelangen removed the ci: do not merge label Mar 25, 2020

shilman modified the milestones: 6.0, 6.0 server Mar 26, 2020

shilman changed the title ~~Actions Addon - Add Preset for and configure with Parameters~~ Addon-Actions: Add preset and configure with parameters Mar 27, 2020

shilman merged commit 4007098 into storybookjs:next Mar 27, 2020

shilman removed the feature request label Mar 31, 2020

shilman mentioned this pull request Mar 31, 2020

Remove special code for a11y, actions and links in @storybook/server #9741

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addon-Actions: Add preset and configure with parameters #9933

Addon-Actions: Add preset and configure with parameters #9933

jonspalmer commented Feb 22, 2020 •

edited

Loading

shilman left a comment

jonspalmer Feb 23, 2020

jonspalmer Feb 23, 2020

tmeasday left a comment

tmeasday Feb 24, 2020

tmeasday Feb 24, 2020

jonspalmer Feb 24, 2020

tmeasday Feb 24, 2020

ndelangen Feb 24, 2020

jonspalmer Feb 24, 2020

tmeasday Feb 25, 2020

jonspalmer Feb 25, 2020

jonspalmer Feb 25, 2020

tmeasday Feb 26, 2020

jonspalmer Feb 24, 2020

jonspalmer commented Mar 24, 2020

shilman commented Mar 24, 2020 •

edited

Loading

shilman left a comment

ndelangen commented Mar 25, 2020


		You can define action handles in a declarative way using `withActions` decorators. It accepts the same arguments as [`actions`](#multiple-actions)


		export const createDecorator = (actionsFn: any) => (...args: any[]) => (storyFn: () => any) => {

		@@ -0,0 +1,90 @@
		import addons from '@storybook/addons';
		import { actions } from '../..';

Addon-Actions: Add preset and configure with parameters #9933

Addon-Actions: Add preset and configure with parameters #9933

Conversation

jonspalmer commented Feb 22, 2020 • edited Loading

What I did

shilman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmeasday left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonspalmer commented Mar 24, 2020

shilman commented Mar 24, 2020 • edited Loading

shilman left a comment

Choose a reason for hiding this comment

ndelangen commented Mar 25, 2020

jonspalmer commented Feb 22, 2020 •

edited

Loading

shilman commented Mar 24, 2020 •

edited

Loading