-
-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
feat: add Vue support to @addons/background #3488
Changes from 7 commits
ade4495
f27e8b4
b3a703c
1997ec5
86ca14a
48b27e7
460aa29
8f5755c
a24fc42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import Vue from 'vue'; | ||
import { vueHandler } from '../vue'; | ||
|
||
describe('Vue handler', () => { | ||
it('Returns a component with a created function', () => { | ||
const testChannel = { emit: jest.fn() }; | ||
const testStory = () => ({ template: '<div> testStory </div>' }); | ||
const testContext = { | ||
kind: 'Foo', | ||
story: 'bar baz', | ||
}; | ||
const testBackground = [ | ||
{ name: 'twitter', value: '#00aced' }, | ||
{ name: 'facebook', value: '#3b5998', default: true }, | ||
]; | ||
const component = vueHandler(testChannel, testBackground)(testStory, testContext); | ||
|
||
expect(component).toMatchObject({ | ||
created: expect.any(Function), | ||
beforeDestroy: expect.any(Function), | ||
render: expect.any(Function), | ||
}); | ||
}); | ||
|
||
it('Subscribes to the channel on creation', () => { | ||
const testChannel = { emit: jest.fn() }; | ||
const testStory = () => ({ render: h => h('div', ['testStory']) }); | ||
const testContext = { | ||
kind: 'Foo', | ||
story: 'bar baz', | ||
}; | ||
const testBackground = [ | ||
{ name: 'twitter', value: '#00aced' }, | ||
{ name: 'facebook', value: '#3b5998', default: true }, | ||
]; | ||
|
||
new Vue(vueHandler(testChannel, testBackground)(testStory, testContext)).$mount(); | ||
|
||
expect(testChannel.emit).toHaveBeenCalledTimes(1); | ||
expect(testChannel.emit).toHaveBeenCalledWith('background-set', expect.any(Array)); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,66 +1,7 @@ | ||
import React from 'react'; | ||
import { polyfill } from 'react-lifecycles-compat'; | ||
import PropTypes from 'prop-types'; | ||
import { window } from 'global'; | ||
import ReactBackground from './react'; | ||
import VueBackground from './vue'; | ||
|
||
import addons from '@storybook/addons'; | ||
const Background = window.STORYBOOK_ENV === 'vue' ? VueBackground : ReactBackground; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem with this approach is that now you bring Vue to react storybook bundle when it's not actually needed. In knobs, we decided to have separate entry points for each framework:
see #1832 for reference There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But actually, there's a chance we come up with a generic framework-agnostic solution for addons like this, which only add subscriptions but don't render anything inside preview iframe: See #3475 (comment) In this case, there will be no need in having different decorators in different frameworks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. I will follow this up with a PR to update generic framework agnostic addons would awesome! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I wait for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can add a temporary |
||
|
||
export class BackgroundDecorator extends React.Component { | ||
constructor(props) { | ||
super(props); | ||
|
||
const { channel } = props; | ||
|
||
// A channel is explicitly passed in for testing | ||
if (channel) { | ||
this.channel = channel; | ||
} else { | ||
this.channel = addons.getChannel(); | ||
} | ||
|
||
this.state = {}; | ||
} | ||
|
||
componentDidMount() { | ||
this.channel.emit('background-set', this.props.backgrounds); | ||
} | ||
|
||
componentWillUnmount() { | ||
this.channel.emit('background-unset'); | ||
} | ||
|
||
render() { | ||
return this.state.story; | ||
} | ||
} | ||
|
||
BackgroundDecorator.getDerivedStateFromProps = ({ story }, { prevStory }) => { | ||
if (story !== prevStory) { | ||
return { | ||
story: story(), | ||
prevStory: story, | ||
}; | ||
} | ||
return null; | ||
}; | ||
|
||
BackgroundDecorator.propTypes = { | ||
backgrounds: PropTypes.arrayOf(PropTypes.object), | ||
channel: PropTypes.shape({ | ||
emit: PropTypes.func, | ||
on: PropTypes.func, | ||
removeListener: PropTypes.func, | ||
}), | ||
// eslint-disable-next-line react/no-unused-prop-types | ||
story: PropTypes.func.isRequired, | ||
}; | ||
|
||
BackgroundDecorator.defaultProps = { | ||
backgrounds: [], | ||
channel: undefined, | ||
}; | ||
|
||
polyfill(BackgroundDecorator); | ||
|
||
export default backgrounds => story => ( | ||
<BackgroundDecorator story={story} backgrounds={backgrounds} /> | ||
); | ||
export default Background; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
import React from 'react'; | ||
import { polyfill } from 'react-lifecycles-compat'; | ||
import PropTypes from 'prop-types'; | ||
|
||
import addons from '@storybook/addons'; | ||
|
||
export class BackgroundDecorator extends React.Component { | ||
constructor(props) { | ||
super(props); | ||
|
||
const { channel } = props; | ||
|
||
// A channel is explicitly passed in for testing | ||
if (channel) { | ||
this.channel = channel; | ||
} else { | ||
this.channel = addons.getChannel(); | ||
} | ||
|
||
this.state = {}; | ||
} | ||
|
||
componentDidMount() { | ||
this.channel.emit('background-set', this.props.backgrounds); | ||
} | ||
|
||
componentWillUnmount() { | ||
this.channel.emit('background-unset'); | ||
} | ||
|
||
render() { | ||
return this.state.story; | ||
} | ||
} | ||
|
||
BackgroundDecorator.getDerivedStateFromProps = ({ story }, { prevStory }) => { | ||
if (story !== prevStory) { | ||
return { | ||
story: story(), | ||
prevStory: story, | ||
}; | ||
} | ||
return null; | ||
}; | ||
|
||
BackgroundDecorator.propTypes = { | ||
backgrounds: PropTypes.arrayOf(PropTypes.object), | ||
channel: PropTypes.shape({ | ||
emit: PropTypes.func, | ||
on: PropTypes.func, | ||
removeListener: PropTypes.func, | ||
}), | ||
// eslint-disable-next-line react/no-unused-prop-types | ||
story: PropTypes.func.isRequired, | ||
}; | ||
|
||
BackgroundDecorator.defaultProps = { | ||
backgrounds: [], | ||
channel: undefined, | ||
}; | ||
|
||
polyfill(BackgroundDecorator); | ||
|
||
export default backgrounds => story => ( | ||
<BackgroundDecorator story={story} backgrounds={backgrounds} /> | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import addons from '@storybook/addons'; | ||
|
||
export const vueHandler = (channel, backgrounds) => (getStory, context) => ({ | ||
data() { | ||
return { | ||
context, | ||
getStory, | ||
story: getStory(context), | ||
}; | ||
}, | ||
|
||
render(h) { | ||
return h(this.story); | ||
}, | ||
|
||
created() { | ||
channel.emit('background-set', backgrounds); | ||
}, | ||
|
||
beforeDestroy() { | ||
channel.emit('background-unset'); | ||
}, | ||
}); | ||
|
||
export default function makeDecorator(backgrounds) { | ||
const channel = addons.getChannel(); | ||
return vueHandler(channel, backgrounds); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`Storyshots Addon|Backgrounds story 1 1`] = ` | ||
<button> | ||
You should be able to switch backgrounds for this story | ||
</button> | ||
`; | ||
|
||
exports[`Storyshots Addon|Backgrounds story 2 1`] = ` | ||
<button> | ||
This one too! | ||
</button> | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import { storiesOf } from '@storybook/vue'; | ||
import backgrounds from '@storybook/addon-backgrounds'; | ||
|
||
storiesOf('Addon|Backgrounds', module) | ||
.addDecorator( | ||
backgrounds([ | ||
{ name: 'twitter', value: '#00aced' }, | ||
{ name: 'facebook', value: '#3b5998', default: true }, | ||
]) | ||
) | ||
.add('story 1', () => { | ||
const content = 'You should be able to switch backgrounds for this story'; | ||
|
||
return { | ||
template: `<button>${content}</button>`, | ||
}; | ||
}) | ||
.add('story 2', () => { | ||
const content = 'This one too!'; | ||
|
||
return { | ||
template: `<button>${content}</button>`, | ||
}; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove this peerDependency now (and suppress corresponding ESLint warnings)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean the
react
or thevue
dependency? or should I just remove both?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or should I move the
react
dep intodevDependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean react. In devDependencies, you can have anything that's used in tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! Thanks for clarifying