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

Generic addon decorators #3555

Merged
merged 22 commits into from
May 15, 2018
Merged

Generic addon decorators #3555

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f330011
Generic addon-a11y decorator
Hypnosphi May 7, 2018
3861711
Merge remote-tracking branch 'origin/master' into generic-addon-decor…
Hypnosphi May 7, 2018
6f78c12
Generic addon-backgrounds decorator
Hypnosphi May 7, 2018
0ecf66e
Update examples
Hypnosphi May 7, 2018
f9e946e
Update readme
Hypnosphi May 7, 2018
09225c2
Generic addon-events decorator
Hypnosphi May 7, 2018
7cbadb1
Merge remote-tracking branch 'origin/master' into generic-addon-decor…
Hypnosphi May 8, 2018
bb5c62f
Introduce "story rendered" event
Hypnosphi May 8, 2018
2e30aeb
Generic addon-knobs decorator
Hypnosphi May 9, 2018
f779598
Support supscriptions & force render in RN
Hypnosphi May 12, 2018
b94155e
Fix tests
Hypnosphi May 12, 2018
c15b9da
Merge branch 'master' into generic-addon-decorators
Hypnosphi May 12, 2018
2889076
RN: Sync selected story between manager and device
Hypnosphi May 12, 2018
56f30e7
RN: Sync selected story between manager and device
Hypnosphi May 12, 2018
24fe256
Backgrounds: add overriding example
Hypnosphi May 12, 2018
34f0c53
Events support deprecated usage
Hypnosphi May 12, 2018
2557bc6
Events: fix switching stories
Hypnosphi May 12, 2018
06101ef
Generic addon-viewport decorator
Hypnosphi May 12, 2018
7b19079
Merge remote-tracking branch 'origin/master' into generic-addon-decor…
Hypnosphi May 12, 2018
a36f35d
Merge remote-tracking branch 'origin/master' into generic-addon-decor…
Hypnosphi May 15, 2018
ef03d22
Remove unneeded file
Hypnosphi May 15, 2018
da9990e
Update storyshot
Hypnosphi May 15, 2018
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
6 changes: 3 additions & 3 deletions ADDONS_SUPPORT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

| |[React](app/react)|[React Native](app/react-native)|[Vue](app/vue)|[Angular](app/angular)| [Polymer](app/polymer)| [Mithril](app/mithril)| [HTML](app/html)| [Marko](app/marko)|
| ----------- |:-------:|:-------:|:-------:|:-------:|:-------:|:-------:|:-------:|
|[a11y](addons/a11y) |+| | | | | |+| |
|[a11y](addons/a11y) |+| |+|+|+|+|+|+|
|[actions](addons/actions) |+|+|+|+|+|+|+|+|
|[backgrounds](addons/backgrounds) |+| | | | |+|+| |
|[backgrounds](addons/backgrounds) |+| |+|+|+|+|+|+|
|[centered](addons/centered) |+| |+| | |+|+| |
|[events](addons/events) |+| | | | | |+| |
|[events](addons/events) |+| |+|+|+|+|+|+|
|[graphql](addons/graphql) |+| | | | | | | |
|[info](addons/info) |+| | | | | | | |
|[jest](addons/jest) |+| | |+| | |+| |
Expand Down
1 change: 0 additions & 1 deletion addons/a11y/html.js
View file
Open in desktop

This file was deleted.

14 changes: 0 additions & 14 deletions addons/a11y/src/A11yManager.js
View file
Open in desktop

This file was deleted.

58 changes: 0 additions & 58 deletions addons/a11y/src/components/WrapStory.js
View file
Open in desktop

This file was deleted.

35 changes: 0 additions & 35 deletions addons/a11y/src/html.js
View file
Open in desktop

This file was deleted.

39 changes: 29 additions & 10 deletions addons/a11y/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,37 @@
import { document } from 'global';
import axe from 'axe-core';
import addons from '@storybook/addons';
import Events from '@storybook/core-events';
import { logger } from '@storybook/client-logger';

import A11yManager from './A11yManager';
import * as shared from './shared';
import { CHECK_EVENT_ID, RERUN_EVENT_ID } from './shared';

const manager = new A11yManager();
let axeOptions = {};

function checkA11y(storyFn, context) {
export const configureA11y = (options = {}) => {
axeOptions = options;
};

const runA11yCheck = () => {
const channel = addons.getChannel();
return manager.wrapStory(channel, storyFn, context, axeOptions);
}
const wrapper = document.getElementById('root');
Copy link
Member

Choose a reason for hiding this comment

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

Is the pattern we want to encourage?

I feel like it is more inflexible than we might like, i.e it means stories can only ever be rendered in #root, whereas there are other conceivable ways you might want to render a story.

Could we instead do something like:

import { afterStory } from '@storybook/core';

export const checkA11y = story => {
  addons.getChannel().emit(Events.REGISTER_SUBSCRIPTION, a11ySubscription);

  // The idea is that `afterStory` wraps a story in a callback that triggers with the story's 
  // DOM element after the story has rendered to the screen.
  const wrappedStory = afterStory(story, (html) => {
    runA11yCheck(html);
  });
 
 return wrappedStory();
};

Not sure if that's the best API, but does the idea make sense?

Copy link
Member Author

@Hypnosphi Hypnosphi May 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Makes sense, will try something like STORY_RENDERED event

Not sure that I get what you mean by other ways to render a story though. The current setup with showMain etc expects that everything you render is inside #root

Copy link
Member

Choose a reason for hiding this comment

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

I guess what I mean is that at the moment you can call const story = getStory() on the story store and then execute that function to get something renderable (how you render that depends on the view layer of course). At the moment that function is also wrapped in the relevant decorators and those decorators will work however the story is rendered.

At the moment the only place a story is rendered is to the #root via the renderMain function, but other ways you might want to render a story is e.g. inside some <Story/> element in a documentation site.

Maybe I'm overthinking it, we would need to think about how decorators/addons make sense in that context a bit anyway, but it's something I think we will want to do in the future.

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 think it's a pattern we could support in SBNext

CC @ndelangen

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've added STORY_RENDERED event to avoid setTimeout(fn, 0), but I decided to keep #root part for now. Most of the addons, including a11y, have manager part, so they are unusable outside of storybook context. So I think that getting rootEl dynamically would bring more complexity than value right now

Copy link
Member

Choose a reason for hiding this comment

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

WFM, thanks for talking through it with me.


function configureA11y(options = {}) {
axeOptions = options;
}
axe.reset();
axe.configure(axeOptions);
axe.run(wrapper).then(results => channel.emit(CHECK_EVENT_ID, results), logger.error);
};

const a11ySubscription = () => {
const channel = addons.getChannel();
channel.on(Events.STORY_RENDERED, runA11yCheck);
channel.on(RERUN_EVENT_ID, runA11yCheck);
return () => {
channel.removeListener(Events.STORY_RENDERED, runA11yCheck);
channel.removeListener(RERUN_EVENT_ID, runA11yCheck);
};
};

export { checkA11y, shared, configureA11y };
export const checkA11y = story => {
addons.getChannel().emit(Events.REGISTER_SUBSCRIPTION, a11ySubscription);
return story();
};
14 changes: 0 additions & 14 deletions addons/backgrounds/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,3 @@ storiesOf("Button", module)
.addDecorator(backgrounds)
.add("with text", () => <button>Click me</button>);
```

> In the case of Mithril, use these imports:
>
> ```js
> import { storiesOf } from '@storybook/mithril';
> import backgrounds from "@storybook/addon-backgrounds/mithril";
> ```

> In the case of Vue, use these imports:
>
> ```js
> import { storiesOf } from '@storybook/vue';
> import backgrounds from "@storybook/addon-backgrounds/vue";
> ```
1 change: 0 additions & 1 deletion addons/backgrounds/html.js
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion addons/backgrounds/mithril.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
module.exports = require('./dist/mithril');
module.exports = require('./dist/deprecated');
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 think we can keep those files for one alpha release

2 changes: 1 addition & 1 deletion addons/backgrounds/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"babel-runtime": "^6.26.0",
"global": "^4.3.2",
"prop-types": "^15.6.1",
"react-lifecycles-compat": "^3.0.2"
"util-deprecate": "^1.0.2"
},
"devDependencies": {
"vue": "^2.5.16"
Expand Down
64 changes: 0 additions & 64 deletions addons/backgrounds/src/__tests__/index.js
View file
Open in desktop

This file was deleted.

44 changes: 0 additions & 44 deletions addons/backgrounds/src/__tests__/vue.js
View file
Open in desktop

This file was deleted.

8 changes: 8 additions & 0 deletions addons/backgrounds/src/deprecated.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import deprecate from 'util-deprecate';

import backgrounds from '.';

export default deprecate(
backgrounds,
"addon-backgrounds: framework-specific imports are deprecated, just use `import backgrounds from '@storybook/addon-backgrounds`"
);
17 changes: 0 additions & 17 deletions addons/backgrounds/src/html.js
View file
Open in desktop

This file was deleted.

Loading