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

Viewport Addon #1753

Merged
merged 33 commits into from
Sep 6, 2017
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
a0cc1c9
initial port
Aug 17, 2017
b47bc5b
improve the viewport package.json for integration into the lerna repo…
Aug 17, 2017
3eb9cc5
fix most lint errors
Aug 17, 2017
c25a42f
Add some features (respnosive, veritcal / horizontal swap), change si…
Aug 18, 2017
0a8cf2a
Update the panels to be a bit more stylish
Aug 25, 2017
82f413d
Merge branch 'release/3.3' into add-viewport-plugin
ndelangen Aug 26, 2017
34f3136
Merge pull request #1740 from saponifi3d/add-viewport-plugin
shilman Aug 28, 2017
c6efb68
add addon viewport to kitchen sink, also sets correct version to view…
marchdoe Aug 28, 2017
7b6e1af
Fix viewport to register correctly in cra-kitchen-sink example
Aug 28, 2017
f0121f5
Merge branch '1624-viewport-addon' of github.com:storybooks/storybook…
marchdoe Aug 28, 2017
9de1008
Add basic documentation, cleanup story
Aug 28, 2017
5233d5b
Fix lint errors for the viewport addon
Aug 28, 2017
ea4a605
Add tests for manager and panel component
Aug 29, 2017
2366e9b
Add tests for rotate and select components
Aug 29, 2017
d4feca3
add tests for viewport info constants.. helping the codecov report
Aug 29, 2017
5b910c7
Merge branch 'release/3.3' into 1624-viewport-addon
Hypnosphi Aug 30, 2017
8852d63
Update gitignore to be happier for vim'ers
Aug 31, 2017
df6a566
remove the local storybook and use cra-kitchen-sink for dev
Aug 31, 2017
c6f68ba
Remove dev deps and use the storybook deps
Aug 31, 2017
420a5ac
Update the README with PR feedback
Aug 31, 2017
640491a
Import document from global
Sep 1, 2017
e8e6384
use the font themes from storybook/components
Sep 1, 2017
d622f82
fix the examples, add the addon to the cra package.json and register …
Sep 1, 2017
9695408
remove scripts except for prepublish
Sep 1, 2017
ea0b055
Move to arrow functions instead of named function declrations
Sep 2, 2017
48723a8
Fix lint errors
Sep 2, 2017
9464b0d
Merge branch 'release/3.3' into 1624-viewport-addon
ndelangen Sep 4, 2017
badc57f
Merge branch 'release/3.3' into 1624-viewport-addon
Hypnosphi Sep 5, 2017
ea3d9f2
move all files from .jsx to .js and .spec.jsx to .test.js
Sep 5, 2017
220d9a9
Merge branch 'release/3.3' into 1624-viewport-addon
ndelangen Sep 6, 2017
2d878da
Merge branch 'release/3.3' into 1624-viewport-addon
ndelangen Sep 6, 2017
bfdc4a3
Merge branch 'release/3.3' into 1624-viewport-addon
ndelangen Sep 6, 2017
d51d0c7
FIX unit test
ndelangen Sep 6, 2017
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ node_modules
*.log
.idea
.vscode
*.sw*
npm-shrinkwrap.json
dist
.tern-port
Expand Down
37 changes: 37 additions & 0 deletions addons/viewport/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Storybook Viewport Addon

Storybook Viewport Addon allows your stories to be displayed in different sizes and layouts in [Storybook](https://storybookjs.org). This helps build responsive components inside of Storybook.

This addon works with Storybook for: [React](https://github.com/storybooks/storybook/tree/master/app/react) and [Vue](https://github.com/storybooks/storybook/tree/master/app/vue).
Copy link
Member

Choose a reason for hiding this comment

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

Awesome, great work!


![Screenshot](https://github.com/storybooks/storybook/blob/master/docs/viewport.png)

## Installation

Install the following npm module:

npm i --save-dev @storybook/addon-viewport

or with yarn:

yarn add -D @storybook/addon-viewport

## Basic Usage

Simply import the Storybook Viewport Addon in the `addon.js` file in your `.storybook` directory.

```js
import '@storybook/addon-viewport/register'
```

This will register the Viewport Addon to Storybook and will show up in the action area.

## FAQ

#### How do I add a new device?

Unfortunately, this is currently not supported.

#### How can I make a custom screen size?

Unfortunately, this is currently not supported.
Binary file added addons/viewport/docs/viewport.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions addons/viewport/manager.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const manager = require('./dist/manager');

manager.init();
22 changes: 22 additions & 0 deletions addons/viewport/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "@storybook/addon-viewport",
"version": "3.2.0",
"description": "Storybook addon to change the viewport size to mobile",
"main": "dist/index.js",
Copy link
Member

@Hypnosphi Hypnosphi Dec 27, 2017

Choose a reason for hiding this comment

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

This file is broken (see #1753 (comment))

Copy link
Member

Choose a reason for hiding this comment

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

#2589 fixes that by removing the index.js file and using register.js (or should it be manager.js?) as main

"keywords": [
"storybook"
],
"scripts": {
"prepublish": "node ../../scripts/prepublish.js"
},
"license": "MIT",
"dependencies": {
"@storybook/components": "^3.2.7",
"global": "^4.3.2",
"prop-types": "^15.5.10"
},
"peerDependencies": {
"@storybook/addons": "^3.2.0",
"react": "*"
}
}
3 changes: 3 additions & 0 deletions addons/viewport/register.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// NOTE: loading addons using this file is deprecated!
// please use manager.js and preview.js files instead
require('./manager');
Copy link
Member Author

Choose a reason for hiding this comment

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

@ndelangen @mnmtanish tangential to this PR but do you know when register was deprecated? This is the first time I've seen it. And isn't it strange that all of our existing addons use a deprecated format (AFAIK)? Seems like something we should figure out?

Copy link
Member

Choose a reason for hiding this comment

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

It's also weird that we actually recommend importing this file in addon's documentation

119 changes: 119 additions & 0 deletions addons/viewport/src/components/Panel.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { baseFonts } from '@storybook/components';
import { document } from 'global';

import { viewports, defaultViewport, resetViewport } from './viewportInfo';
import { SelectViewport } from './SelectViewport';
import { RotateViewport } from './RotateViewport';

import * as styles from './styles';

const storybookIframe = 'storybook-preview-iframe';
const containerStyles = {
padding: 15,
width: '100%',
boxSizing: 'border-box',
...baseFonts,
};

export class Panel extends Component {
static propTypes = {
channel: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
};

constructor(props, context) {
super(props, context);
this.state = {
viewport: defaultViewport,
isLandscape: false,
};

this.props.channel.on('addon:viewport:update', this.changeViewport);
}

componentDidMount() {
this.iframe = document.getElementById(storybookIframe);
}

componentWillUnmount() {
this.props.channel.removeListener('addon:viewport:update', this.changeViewport);
}

iframe = undefined;
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to set this in the constructor

Copy link
Member

@Hypnosphi Hypnosphi Aug 31, 2017

Choose a reason for hiding this comment

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

Is it a general suggestion not to use property initializers? Why? Maybe we should setup a eslint rule then?

Note that it's already stage 3: https://github.com/tc39/proposal-class-fields


changeViewport = viewport => {
const { viewport: previousViewport } = this.state;

if (previousViewport !== viewport) {
this.setState(
{
viewport,
isLandscape: false,
},
this.updateIframe
);
}
};

toggleLandscape = () => {
const { isLandscape } = this.state;

this.setState({ isLandscape: !isLandscape }, this.updateIframe);
};

updateIframe = () => {
Copy link
Member

Choose a reason for hiding this comment

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

arrow-functions cannot have a this binding, why are they object methods then?

If they are utility functions that are unrelated to the instance, we should hoist them to the top of the module. Or possibly even be moved into their own module.

Copy link
Member

Choose a reason for hiding this comment

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

The function obviously uses this. Making it arrow function is just one more way to make it suitable as a callback: https://facebook.github.io/react/docs/handling-events.html ("If calling bind annoys you" section)

Choose a reason for hiding this comment

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

@ndelangen are you saying to instead have this be a normal class function than in the constructor do this.updateIframe = this.updateIframe.bind(this);? Happy to move to that pattern instead if you want, just want to make sure i do it correctly :)

fwiw, this is a fairly common pattern as @Hypnosphi pointed out, but I think it's more important to be consistent through the codebase.

const { viewport: viewportKey, isLandscape } = this.state;
const viewport = viewports[viewportKey] || resetViewport;

if (!this.iframe) {
throw new Error('Cannot find Storybook iframe');
}

Object.keys(viewport.styles).forEach(prop => {
this.iframe.style[prop] = viewport.styles[prop];
});

if (isLandscape) {
this.iframe.style.height = viewport.styles.width;
this.iframe.style.width = viewport.styles.height;
}
};

render() {
const { isLandscape, viewport } = this.state;

const disableDefault = viewport === defaultViewport;
const disabledStyles = disableDefault ? styles.disabled : {};

const buttonStyles = {
...styles.button,
...disabledStyles,
marginTop: 30,
padding: 20,
};

return (
<div style={containerStyles}>
<SelectViewport
activeViewport={viewport}
onChange={e => this.changeViewport(e.target.value)}
/>

<RotateViewport
onClick={this.toggleLandscape}
disabled={disableDefault}
active={isLandscape}
/>

<button
style={buttonStyles}
onClick={() => this.changeViewport(defaultViewport)}
disabled={disableDefault}
>
Reset Viewport
</button>
</div>
);
}
}
30 changes: 30 additions & 0 deletions addons/viewport/src/components/RotateViewport.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import React from 'react';
import PropTypes from 'prop-types';
import * as styles from './styles';

export function RotateViewport({ active, ...props }) {
const disabledStyles = props.disabled ? styles.disabled : {};
const actionStyles = {
...styles.action,
...disabledStyles,
};
return (
<div style={styles.row}>
<label style={styles.label}>Rotate</label>
<button {...props} style={actionStyles}>
{active ? 'Vertical' : 'Landscape'}
</button>
</div>
);
}

RotateViewport.propTypes = {
disabled: PropTypes.bool,
onClick: PropTypes.func.isRequired,
active: PropTypes.bool,
};

RotateViewport.defaultProps = {
disabled: true,
active: false,
};
26 changes: 26 additions & 0 deletions addons/viewport/src/components/SelectViewport.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import React from 'react';
import PropTypes from 'prop-types';

import { viewports, defaultViewport } from './viewportInfo';
import * as styles from './styles';

export function SelectViewport({ activeViewport, onChange }) {
return (
<div style={styles.row}>
<label style={styles.label}>Device</label>
<select style={styles.action} value={activeViewport} onChange={onChange}>
<option value={defaultViewport}>Default</option>
{Object.keys(viewports).map(key =>
<option value={key} key={key}>
{viewports[key].name}
</option>
)}
</select>
</div>
);
}

SelectViewport.propTypes = {
onChange: PropTypes.func.isRequired,
activeViewport: PropTypes.string.isRequired,
};
30 changes: 30 additions & 0 deletions addons/viewport/src/components/styles.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
export const row = {
width: '100%',
display: 'flex',
marginBottom: 15,
};

export const label = {
width: 80,
marginRight: 15,
};

const actionColor = 'rgb(247, 247, 247)';

export const button = {
color: 'rgb(85, 85, 85)',
width: '100%',
border: `1px solid ${actionColor}`,
backgroundColor: actionColor,
borderRadius: 3,
};

export const disabled = {
opacity: '0.5',
cursor: 'not-allowed',
};

export const action = {
...button,
height: 30,
};
Loading