Viewport Addon #1753

shilman · 2017-08-28T17:46:11Z

Issue: #1624

What @saponifi3d did 😉

This is a fairly early PR of a viewport plugin which allows you to resize the iframe of storybook.

Remaining Tasks

Fix lint issues
Add unit tests
Add documentation and screenshots to readme.

Possible Features

Responsive hooks, basically add the ability to drag the ends of the iframe to change the size
Improve interaction with the iframe rather than just grabbing the iframe from the DOM add listeners to the iframe to allow us to interact with it

Screen Cap

How to test

FIXME

Is this testable with jest or storyshots?

Does this need a new example in the kitchen sink apps?

Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

… for storybook

…ngle button to switch.. unclear if that's good or not though.

Viewport Addon

…port addon

codecov · 2017-08-28T19:49:31Z

Codecov Report

Merging #1753 into release/3.3 will increase coverage by 0.51%.
The diff coverage is 63.09%.

@@               Coverage Diff               @@
##           release/3.3    #1753      +/-   ##
===============================================
+ Coverage         22.8%   23.31%   +0.51%     
===============================================
  Files              269      277       +8     
  Lines             5934     6031      +97     
  Branches           696      709      +13     
===============================================
+ Hits              1353     1406      +53     
- Misses            4080     4107      +27     
- Partials           501      518      +17

Impacted Files	Coverage Δ
addons/viewport/register.js	`0% <0%> (ø)`
addons/viewport/src/index.js	`0% <0%> (ø)`
addons/viewport/manager.js	`0% <0%> (ø)`
addons/viewport/src/components/Panel.js	`100% <100%> (ø)`
addons/viewport/src/components/styles.js	`100% <100%> (ø)`
addons/viewport/src/components/SelectViewport.js	`15% <23.07%> (ø)`
addons/viewport/src/components/RotateViewport.js	`22.72% <35.71%> (ø)`
addons/viewport/src/components/viewportInfo.js	`36.36% <36.36%> (ø)`
addons/viewport/src/manager.js	`87.5% <87.5%> (ø)`
lib/ui/src/modules/ui/configs/handle_routing.js	`26.74% <0%> (ø)`	⬆️
... and 47 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1ff05a...d51d0c7. Read the comment docs.

… into 1624-viewport-addon

saponifi3d · 2017-08-29T03:05:44Z

@shilman, i was able to fix-up the lint errors, add a readme, and wrote a bunch of unit tests (it's not 100% coverage - didn't think we needed tests for simple exports yet).

i also noticed the code coverage reports also include the dist directories from addons, want me to fix that in this PR, a separate one, or is someone already fixing that?

ndelangen

If you need any help with making these changes, I'm very much willing to jump in and help! Just let me know. This feature is fantastic, a lot of people are going to be super happy with this!

ndelangen · 2017-08-30T19:00:36Z

addons/viewport/.gitignore

@@ -0,0 +1,5 @@
+*.sw*


Please move any necessary gitignore to the root

ndelangen · 2017-08-30T19:00:48Z

addons/viewport/.storybook/addons.js

@@ -0,0 +1 @@
+import '@storybook/addon-viewport/register';


Please remove the dev setup for this addon, and use the cra-kitchen-sink to develop on.

ndelangen · 2017-08-30T19:01:27Z

addons/viewport/README.md

+
+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).


Awesome, great work!

ndelangen · 2017-08-30T19:01:52Z

addons/viewport/README.md

+
+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).
+
+![Screenshot](docs/viewport.png)


Can we make this is full URL? so it shows up in places like npm?

ndelangen · 2017-08-30T19:02:45Z

addons/viewport/README.md

+
+Install the following npm module:
+
+    npm i -D @storybook/addon-viewport


Use fenced code-blocks please like so:

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

ndelangen · 2017-08-30T19:13:12Z

addons/viewport/src/components/tests/Panel.spec.jsx

@@ -0,0 +1,249 @@
+/* global document */


Please import any global from the global package.

ndelangen · 2017-08-30T19:13:53Z

addons/viewport/src/components/tests/Panel.spec.jsx

@@ -0,0 +1,249 @@
+/* global document */
+import React from 'react';
+import { shallow } from 'enzyme'; // eslint-disable-line import/no-extraneous-dependencies


We use a <name>.test.js naming convention for tests.

ndelangen · 2017-08-30T19:21:12Z

addons/viewport/src/components/tests/Panel.spec.jsx

+  describe('componentDidMount', () => {
+    let previousGet;
+
+    beforeEach(() => {


low priority:

I prefer to use beforeEach and afterEach functions as last resort only.
The reason being it becomes harder and hard to know/understand all the steps executed to setup a test.

Not directly necessary to get this PR merged, but I'd like to see a setup function being used to get a shallow render in a specific state.

ndelangen · 2017-08-30T19:21:39Z

addons/viewport/src/components/tests/RotateViewport.spec.jsx

@@ -0,0 +1,91 @@
+import React from 'react';
+import { shallow } from 'enzyme'; // eslint-disable-line import/no-extraneous-dependencies


Please use the .test.js naming convention here too

No use of .jsx file extention

Should I change all .jsx files to be .js?

Yes please!

If you can rename all .spec. to .test. and all .jsx to .js I think this is good to go!

Let me know if you need any help! 👍

ndelangen · 2017-08-30T19:23:48Z

addons/viewport/src/manager.jsx

+const ADDON_ID = 'storybook-addon-viewport';
+const PANEL_ID = `${ADDON_ID}/addon-panel`;
+
+function addChannel(api) {


Looks like these could be arrow functions? any reason to make them function declarations?

not a huge thing - but from what i understand chrome specifically will do a better job at optimizing named functions like this over arrow functions. happy to update to be an arrow function instead :D

…the component in the vue example

ndelangen · 2017-09-04T20:16:15Z

Looking good @saponifi3d, looks like we could merge this soon.

What do you think @usulpro @Hypnosphi ?

Hypnosphi

LGTM

saponifi3d · 2017-09-05T17:22:06Z

@ndelangen just updated all the file names, let me know if there's anything else you wanted done (a lot of the comments just go smushed from the filename change).

thanks so much for the review / feedback!!!

shilman · 2017-09-06T15:59:48Z

addons/viewport/register.js

@@ -0,0 +1,3 @@
+// NOTE: loading addons using this file is deprecated!
+// please use manager.js and preview.js files instead
+require('./manager');


@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?

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

shilman · 2017-09-06T16:01:53Z

addons/viewport/src/components/Panel.js

+    this.props.channel.removeListener('addon:viewport:update', this.changeViewport);
+  }
+
+  iframe = undefined;


@saponifi3d define in constructor?

shilman · 2017-09-06T16:07:49Z

@saponifi3d made a minor suggestion but I think it looks great to release for alpha. I'd like to generalize it in a way that also supports something like #1394 but I think we can do that in a later revision and it's better for us to get this out as part of the alpha. Looking forward to merging and putting out our first 3.3 alpha this week!

avaly · 2017-10-19T15:00:07Z

What's the differences between this new addon and storybook-addon-scissors?

Hypnosphi · 2017-12-27T23:41:23Z

addons/viewport/src/index.js

@@ -0,0 +1 @@
+export { register } from './manager';


manager.js don't have this export

Hypnosphi · 2017-12-27T23:42:17Z

addons/viewport/register.js

@@ -0,0 +1,3 @@
+// NOTE: loading addons using this file is deprecated!
+// please use manager.js and preview.js files instead
+require('./manager');


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

Hypnosphi · 2017-12-27T23:43:23Z

addons/viewport/package.json

+  "name": "@storybook/addon-viewport",
+  "version": "3.2.0",
+  "description": "Storybook addon to change the viewport size to mobile",
+  "main": "dist/index.js",


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

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

Josh Callender and others added 9 commits August 17, 2017 15:26

initial port

a0cc1c9

improve the viewport package.json for integration into the lerna repo…

b47bc5b

… for storybook

fix most lint errors

3eb9cc5

Add some features (respnosive, veritcal / horizontal swap), change si…

c25a42f

…ngle button to switch.. unclear if that's good or not though.

Update the panels to be a bit more stylish

0a8cf2a

Merge branch 'release/3.3' into add-viewport-plugin

82f413d

Merge pull request #1740 from saponifi3d/add-viewport-plugin

34f3136

Viewport Addon

add addon viewport to kitchen sink, also sets correct version to view…

c6efb68

…port addon

Fix viewport to register correctly in cra-kitchen-sink example

7b6e1af

marchdoe and others added 5 commits August 28, 2017 15:53

Merge branch '1624-viewport-addon' of github.com:storybooks/storybook…

f0121f5

… into 1624-viewport-addon

Add basic documentation, cleanup story

9de1008

Fix lint errors for the viewport addon

5233d5b

Add tests for manager and panel component

ea4a605

Add tests for rotate and select components

2366e9b

Josh Callender and others added 2 commits August 28, 2017 20:15

add tests for viewport info constants.. helping the codecov report

d4feca3

Merge branch 'release/3.3' into 1624-viewport-addon

5b910c7

ndelangen requested changes Aug 30, 2017

View reviewed changes

ndelangen assigned saponifi3d Aug 30, 2017

ndelangen added addon: viewport feature request labels Aug 30, 2017

Josh Callender added 8 commits August 31, 2017 14:53

Update gitignore to be happier for vim'ers

8852d63

remove the local storybook and use cra-kitchen-sink for dev

df6a566

Remove dev deps and use the storybook deps

c6f68ba

Update the README with PR feedback

420a5ac

Import document from global

640491a

use the font themes from storybook/components

e8e6384

fix the examples, add the addon to the cra package.json and register …

d622f82

…the component in the vue example

remove scripts except for prepublish

9695408

Josh Callender and others added 3 commits September 1, 2017 17:00

Move to arrow functions instead of named function declrations

ea0b055

Fix lint errors

48723a8

Merge branch 'release/3.3' into 1624-viewport-addon

9464b0d

Hypnosphi approved these changes Sep 4, 2017

View reviewed changes

Hypnosphi and others added 2 commits September 5, 2017 03:22

Merge branch 'release/3.3' into 1624-viewport-addon

badc57f

move all files from .jsx to .js and .spec.jsx to .test.js

ea3d9f2

Merge branch 'release/3.3' into 1624-viewport-addon

220d9a9

ndelangen approved these changes Sep 6, 2017

View reviewed changes

Merge branch 'release/3.3' into 1624-viewport-addon

2d878da

danielduan added this to the v3.3.0 milestone Sep 6, 2017

danielduan approved these changes Sep 6, 2017

View reviewed changes

shilman commented Sep 6, 2017

View reviewed changes

ndelangen added 2 commits September 6, 2017 20:00

Merge branch 'release/3.3' into 1624-viewport-addon

bfdc4a3

FIX unit test

d51d0c7

ndelangen merged commit 3fda202 into release/3.3 Sep 6, 2017

ndelangen deleted the 1624-viewport-addon branch September 6, 2017 18:43

Hypnosphi reviewed Dec 27, 2017

View reviewed changes

This was referenced Jan 12, 2018

puppeteer versions available: 1.0.0 #2731

Closed

puppeteer versions available: 1.0.0 #2732

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Viewport Addon #1753

Viewport Addon #1753

shilman commented Aug 28, 2017 •

edited by saponifi3d

Loading

codecov bot commented Aug 28, 2017 •

edited

Loading

saponifi3d commented Aug 29, 2017 •

edited

Loading

ndelangen left a comment

ndelangen Aug 30, 2017

ndelangen Aug 30, 2017

ndelangen Aug 30, 2017

ndelangen Aug 30, 2017

ndelangen Aug 30, 2017

ndelangen Aug 30, 2017

ndelangen Aug 30, 2017

ndelangen Aug 30, 2017

ndelangen Aug 30, 2017

ndelangen Aug 30, 2017

saponifi3d Sep 1, 2017

ndelangen Sep 4, 2017

ndelangen Sep 5, 2017

ndelangen Aug 30, 2017

saponifi3d Sep 1, 2017

ndelangen commented Sep 4, 2017

Hypnosphi left a comment

saponifi3d commented Sep 5, 2017

shilman Sep 6, 2017

Hypnosphi Dec 27, 2017

shilman Sep 6, 2017

shilman commented Sep 6, 2017

avaly commented Oct 19, 2017

Hypnosphi Dec 27, 2017

Hypnosphi Dec 27, 2017

Hypnosphi Dec 27, 2017 •

edited

Loading

Hypnosphi Dec 28, 2017


		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).


		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).

		![Screenshot](docs/viewport.png)


		Install the following npm module:

		npm i -D @storybook/addon-viewport

		@@ -0,0 +1,91 @@
		import React from 'react';
		import { shallow } from 'enzyme'; // eslint-disable-line import/no-extraneous-dependencies

Viewport Addon #1753

Viewport Addon #1753

Conversation

shilman commented Aug 28, 2017 • edited by saponifi3d Loading

What @saponifi3d did 😉

Remaining Tasks

Possible Features

Screen Cap

How to test

codecov bot commented Aug 28, 2017 • edited Loading

Codecov Report

saponifi3d commented Aug 29, 2017 • edited Loading

ndelangen 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

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

ndelangen commented Sep 4, 2017

Hypnosphi left a comment

Choose a reason for hiding this comment

saponifi3d commented Sep 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shilman commented Sep 6, 2017

avaly commented Oct 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hypnosphi Dec 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shilman commented Aug 28, 2017 •

edited by saponifi3d

Loading

codecov bot commented Aug 28, 2017 •

edited

Loading

saponifi3d commented Aug 29, 2017 •

edited

Loading

Hypnosphi Dec 27, 2017 •

edited

Loading