From 3db6b121035da205ea5fb44bfb8f35f0e2ef881c Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 4 Mar 2019 12:22:13 +1100 Subject: [PATCH 1/7] FIX restore default viewport behaviour Also restore using the `viewport` key, and enforce the viewport parameter to be a object. --- addons/viewport/src/Tool.js | 15 +++++- addons/viewport/src/constants.js | 2 +- examples/official-storybook/config.js | 6 --- .../official-storybook/extra-viewports.json | 18 ------- .../stories/addon-viewport.stories.js | 54 ++++++------------- 5 files changed, 30 insertions(+), 65 deletions(-) delete mode 100644 examples/official-storybook/extra-viewports.json diff --git a/addons/viewport/src/Tool.js b/addons/viewport/src/Tool.js index 3e986d455d1e..8fce477f1597 100644 --- a/addons/viewport/src/Tool.js +++ b/addons/viewport/src/Tool.js @@ -8,6 +8,7 @@ import { Icons, IconButton, WithTooltip, TooltipLinkList } from '@storybook/comp import { SET_STORIES } from '@storybook/core-events'; import { PARAM_KEY } from './constants'; +import { INITIAL_VIEWPORTS, DEFAULT_VIEWPORT } from './defaults'; const toList = memoize(50)(items => items ? Object.entries(items).map(([id, value]) => ({ ...value, id })) : [] @@ -28,12 +29,22 @@ const flip = ({ width, height }) => ({ height: width, width: height }); const getState = memoize(10)((props, state, change) => { const data = props.api.getCurrentStoryData(); - const list = toList(data && data.parameters && data.parameters[PARAM_KEY]); + const parameters = data && data.parameters && data.parameters[PARAM_KEY]; + + if (typeof parameters !== 'object') { + console.warn( + 'The viewport parameter must be an object with keys `viewports` and `defaultViewport`' + ); + } + + const { viewports, defaultViewport } = parameters || {}; + + const list = toList(viewports || INITIAL_VIEWPORTS); const selected = state.selected === 'responsive' || list.find(i => i.id === state.selected) ? state.selected - : list.find(i => i.default) || 'responsive'; + : list.find(i => i.default) || defaultViewport || DEFAULT_VIEWPORT; const resets = selected !== 'responsive' diff --git a/addons/viewport/src/constants.js b/addons/viewport/src/constants.js index 90beefe690de..1a9e8f7376b2 100644 --- a/addons/viewport/src/constants.js +++ b/addons/viewport/src/constants.js @@ -1,5 +1,5 @@ export const ADDON_ID = 'storybook/viewport'; -export const PARAM_KEY = 'viewports'; +export const PARAM_KEY = 'viewport'; export default { UPDATE: `${ADDON_ID}/update`, diff --git a/examples/official-storybook/config.js b/examples/official-storybook/config.js index f394a8dab04b..157855965e1a 100644 --- a/examples/official-storybook/config.js +++ b/examples/official-storybook/config.js @@ -2,7 +2,6 @@ import React from 'react'; import { storiesOf, configure, addDecorator, addParameters } from '@storybook/react'; import { Global, ThemeProvider, themes, createReset } from '@storybook/theming'; -import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport'; import { withCssResources } from '@storybook/addon-cssresources'; import { withA11Y } from '@storybook/addon-a11y'; import { withNotes } from '@storybook/addon-notes'; @@ -10,7 +9,6 @@ import { withNotes } from '@storybook/addon-notes'; import 'storybook-chromatic'; import addHeadWarning from './head-warning'; -import extraViewports from './extra-viewports.json'; if (process.env.NODE_ENV === 'development') { if (!process.env.DOTENV_DEVELOPMENT_DISPLAY_WARNING) { @@ -52,10 +50,6 @@ addParameters({ hierarchySeparator: /\/|\./, hierarchyRootSeparator: '|', }, - viewports: { - ...INITIAL_VIEWPORTS, - ...extraViewports, - }, backgrounds: [ { name: 'storybook app', value: themes.normal.background.app, default: true }, { name: 'light', value: '#eeeeee' }, diff --git a/examples/official-storybook/extra-viewports.json b/examples/official-storybook/extra-viewports.json deleted file mode 100644 index 4d688518385c..000000000000 --- a/examples/official-storybook/extra-viewports.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "kindleFire2": { - "name": "Kindle Fire 2", - "styles": { - "width": "600px", - "height": "963px" - }, - "type": "tablet" - }, - "kindleFireHD": { - "name": "Kindle Fire HD", - "styles": { - "width": "533px", - "height": "801px" - }, - "type": "tablet" - } -} diff --git a/examples/official-storybook/stories/addon-viewport.stories.js b/examples/official-storybook/stories/addon-viewport.stories.js index 7276ee4eaa84..b9d4018df044 100644 --- a/examples/official-storybook/stories/addon-viewport.stories.js +++ b/examples/official-storybook/stories/addon-viewport.stories.js @@ -3,10 +3,7 @@ import { storiesOf } from '@storybook/react'; import { styled } from '@storybook/theming'; -// import { Viewport, withViewport } from '@storybook/addon-viewport'; -// import EventEmitter from 'eventemitter3'; - -// import Logger from './Logger'; +import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport'; const Panel = styled.div(); @@ -15,6 +12,20 @@ storiesOf('Addons|Viewport', module).add('default', () => ( )); storiesOf('Addons|Viewport.Custom Default (Kindle Fire 2)', module) + .addParameters({ + viewport: { + viewports: { + ...INITIAL_VIEWPORTS, + kindleFire2: { + name: 'Kindle Fire 2', + styles: { + width: '600px', + height: '963px', + }, + }, + }, + }, + }) .add('Inherited', () => ( I've inherited Kindle Fire 2 viewport from my parent. @@ -27,38 +38,5 @@ storiesOf('Addons|Viewport.Custom Default (Kindle Fire 2)', module) I respect my parents but I should be looking good on iPad. ), - { viewports: [] } + { viewport: { defaultViewport: 'ipad' } } ); - -// const emitter = new EventEmitter(); - -// storiesOf('Addons|Viewport.withViewport', module) -// .addDecorator( -// withViewport({ -// onViewportChange({ viewport }) { -// emitter.emit(Logger.LOG_EVENT, { -// name: 'Viewport Changed', -// payload: `${viewport.name} (${viewport.type})`, -// }); -// }, -// }) -// ) -// .add('onViewportChange', () => ); - -// storiesOf('Addons|Viewport.deprecated', module) -// .addDecorator(withViewport('kindleFire2')) -// .add( -// 'Overridden via "withViewport" decorator', -// withViewport('iphone6')(() => ( -// -// I respect my parents but I should be looking good on iPhone 6. -// -// )) -// ) -// .add('Overridden via "Viewport" component', () => ( -// -// -// I respect my parents but I should be looking good on iPhone 6 Plus. -// -// -// )); From c2d55a466cfac425e0d3f373f4e8328230efc7f1 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 4 Mar 2019 12:22:32 +1100 Subject: [PATCH 2/7] ADD documenation of new viewport API and migration notes --- MIGRATION.md | 22 ++++++ addons/viewport/README.md | 152 +++++++++++--------------------------- 2 files changed, 66 insertions(+), 108 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index a1d107791255..c683b0b95342 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -163,6 +163,28 @@ storiesOf('Stories', module) .add('centered', () => 'Hello', { decorators: [centered] }); ``` +## Addon viewport uses parameters + +Similarly, `@storybook/addon-viewport` uses parameters to pass viewport options. If you previously had: + +```js +import { configureViewport } from `@storybook/addon-viewport`; + +configureViewport(options); +``` + +You can replace it with: + +```js +import { addParameters } from '@storybook/react'; // or others + +addParameters({ viewport: options }); +``` + +The `withViewport` decorator is also no longer supported and should be replaced with a parameter based API as above. Also the `onViewportChange` callback is no longer supported. + +See the README for the viewport addon for more information: https://github.com/storybooks/storybook/blob/master/addons/viewport/README.md + ## From version 4.0.x to 4.1.x There are are a few migrations you should be aware of in 4.1, including one unintentionally breaking change for advanced addon usage. diff --git a/addons/viewport/README.md b/addons/viewport/README.md index 5356c91bcff4..5e9273e9b486 100644 --- a/addons/viewport/README.md +++ b/addons/viewport/README.md @@ -1,6 +1,6 @@ # Storybook Viewport Addon -Storybook Viewport Addon allows your stories to be displayed in different sizes and layouts in [Storybook](https://storybook.js.org). This helps build responsive components inside of Storybook. +Storybook Viewport Addon allows your stories to be displayed in different sizes and layouts in [Storybook](https://storybook.js.org). This helps build responsive components inside of Storybook. [Framework Support](https://github.com/storybooks/storybook/blob/master/ADDONS_SUPPORT.md) @@ -28,21 +28,30 @@ import '@storybook/addon-viewport/register'; ## Configuration -Import and use the `configureViewport` function in your `config.js` file. +The viewport addon is configured by story parameters with the `viewport` key. To configure globally, import `addParameters` from your app layer in your `config.js` file. ```js -import { configureViewport } from '@storybook/addon-viewport'; +import { addParameters } from '@storybook/react'; + +addParameters({ viewport: options }); ``` +Options can take a object with the following keys: + ### defaultViewport : String ----- + +--- + Setting this property to, let say `iphone6`, will make `iPhone 6` the default device/viewport for all stories. Default is `'responsive'` which fills 100% of the preview area. ### viewports : Object ----- + +--- + A key-value pair of viewport's key and properties (see `Viewport` definition below) for all viewports to be displayed. Default is [`INITIAL_VIEWPORTS`](src/shared/index.js) #### Viewport Model + ```js { /** @@ -70,154 +79,81 @@ A key-value pair of viewport's key and properties (see `Viewport` definition bel } ``` -## Decorators +## Configuring per component or story -Sometimes you want to show collection of mobile stories, and you know those stories look horible on desktop (`responsive`), so you think you need to change the default viewport only for those? +Parameters can be configured for a whole set of stories or a single story via the standard parameter API: -Here is the answer, with `withViewport` decorator, you can change the default viewport of single, multiple, or all stories. - -`withViewport` accepts either -* A `String`, which represents the default viewport, or -* An `Object`, which looks like ```js -{ - name: 'iphone6', // default viewport - onViewportChange({ viewport }) { // called whenever different viewport is selected from the dropdown +import addStories from '@storybook/react'; - } -} +addStories('Stories', module) + // To set a default viewport for all the stories for this component + .addParameters({ viewport: { defaultViewport: 'iphone6' }}) + .add('story', () => , { viewport: 'iphonex' }); ``` ## Examples -### Basic Usage - -Simply import the Storybook Viewport Addon in the `addons.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. - - ### Use Custom Set of Devices -This will replace all previous devices with `Kindle Fire 2` and `Kindle Fire HD` by simply calling `configureViewport` with the two devices as `viewports` in `config.js` file in your `.storybook` directory. +This will replace all previous devices with `Kindle Fire 2` and `Kindle Fire HD` by simply calling `addParameters` with the two devices as `viewports` in `config.js` file in your `.storybook` directory. ```js -import { configureViewport } from '@storybook/addon-viewport'; +import { addParameters } from '@storybook/react'; const newViewports = { kindleFire2: { name: 'Kindle Fire 2', styles: { width: '600px', - height: '963px' - } + height: '963px', + }, }, kindleFireHD: { name: 'Kindle Fire HD', styles: { width: '533px', - height: '801px' - } - } + height: '801px', + }, + }, }; -configureViewport({ - viewports: newViewports +addParameters({ + viewport: { viewports: newViewports }, }); ``` - ### Add New Device This will add both `Kindle Fire 2` and `Kindle Fire HD` to the list of devices. This is acheived by making use of the exported [`INITIAL_VIEWPORTS`](src/shared/index.js) property, by merging it with the new viewports and pass the result as `viewports` to `configureViewport` function ```js -import { configureViewport, INITIAL_VIEWPORTS } from '@storybook/addon-viewport'; +import { addParameters } from '@storybook/react'; +import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport'; const newViewports = { kindleFire2: { name: 'Kindle Fire 2', styles: { width: '600px', - height: '963px' - } + height: '963px', + }, }, kindleFireHD: { name: 'Kindle Fire HD', styles: { width: '533px', - height: '801px' - } - } + height: '801px', + }, + }, }; -configureViewport({ - viewports: { - ...INITIAL_VIEWPORTS, - ...newViewports - } -}); -``` - - -### Change The Default Viewport - -This will make `iPhone 6` the default viewport for all stories. - -```js -import { configureViewport } from '@storybook/addon-viewport'; - -configureViewport({ - defaultViewport: 'iphone6' +addParameters({ + viewport: { + viewports: { + ...INITIAL_VIEWPORTS, + ...newViewports, + }, + }, }); ``` - -## withViewport Decorator - -Change the default viewport for single/multiple/global stories, or listen to viewport selection changes - -```js -import React from 'react'; -import { storiesOf, addDecorator } from '@storybook/react'; -import { withViewport } from '@storybook/addon-viewport'; - -// Globablly -addDecorator(withViewport('iphone5')); - -// Collection -storiesOf('Decorator with string', module) - .addDecorator(withViewport('iphone6')) - .add('iPhone 6', () => ( -

- Do I look good on iPhone 6? -

- )); - -// Single -storiesOf('Parameterized story', module) - .addDecorator(withViewport()) - .add( - 'iPad', - () => ( -

- Do I look good on iPad? -

- ), - { viewport: 'ipad' } - ); - -storiesOf('Decorator with object', module) - .addDecorator( - withViewport({ - onViewportChange({ viewport }) { - console.log(`Viewport changed: ${viewport.name} (${viewport.type})`); // e.g. Viewport changed: iphone6 (mobile) - }, - }) - ) - .add('onViewportChange', () => ); - -``` From 6dfa51a6ef605d93bc8a38a7cac41539d34aed01 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 4 Mar 2019 12:30:44 +1100 Subject: [PATCH 3/7] ADD some better upgrade messages --- addons/viewport/preview.js | 1 - addons/viewport/src/Tool.js | 1 + addons/viewport/src/legacy_preview/index.js | 4 ++-- addons/viewport/src/legacy_preview/withViewport.js | 10 ++-------- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/addons/viewport/preview.js b/addons/viewport/preview.js index 0c0f6c625393..aea18c083543 100644 --- a/addons/viewport/preview.js +++ b/addons/viewport/preview.js @@ -4,4 +4,3 @@ exports.configureViewport = preview.configureViewport; exports.DEFAULT_VIEWPORT = preview.DEFAULT_VIEWPORT; exports.INITIAL_VIEWPORTS = preview.INITIAL_VIEWPORTS; exports.withViewport = preview.withViewport; -exports.Viewport = preview.Viewport; diff --git a/addons/viewport/src/Tool.js b/addons/viewport/src/Tool.js index 8fce477f1597..6c6c4093edc4 100644 --- a/addons/viewport/src/Tool.js +++ b/addons/viewport/src/Tool.js @@ -32,6 +32,7 @@ const getState = memoize(10)((props, state, change) => { const parameters = data && data.parameters && data.parameters[PARAM_KEY]; if (typeof parameters !== 'object') { + // eslint-disable-next-line no-console console.warn( 'The viewport parameter must be an object with keys `viewports` and `defaultViewport`' ); diff --git a/addons/viewport/src/legacy_preview/index.js b/addons/viewport/src/legacy_preview/index.js index 9a4af3066d53..7bb7c3ef5fc5 100644 --- a/addons/viewport/src/legacy_preview/index.js +++ b/addons/viewport/src/legacy_preview/index.js @@ -1,7 +1,7 @@ import deprecate from 'util-deprecate'; export { INITIAL_VIEWPORTS, DEFAULT_VIEWPORT } from '../defaults'; -export { default as withViewport, Viewport } from './withViewport'; +export { default as withViewport } from './withViewport'; export const configureViewport = deprecate(() => {}, -'usage is deprecated, use .addParameters({ viewport }) instead'); +'configureViewport is no longer supported, use .addParameters({ viewport }) instead'); diff --git a/addons/viewport/src/legacy_preview/withViewport.js b/addons/viewport/src/legacy_preview/withViewport.js index 5cdb169ca311..f1f6ca2a6f9e 100644 --- a/addons/viewport/src/legacy_preview/withViewport.js +++ b/addons/viewport/src/legacy_preview/withViewport.js @@ -3,17 +3,11 @@ import deprecate from 'util-deprecate'; const withViewport = makeDecorator({ name: 'withViewport', - parameterName: 'viewports', - allowDeprecatedUsage: true, + parameterName: 'viewport', wrapper: deprecate( (getStory, context) => getStory(context), - 'usage is deprecated, use .addParameters({ viewport }) instead' + 'withViewport is no longer supported, use .addParameters({ viewport }) instead' ), }); export default withViewport; - -export const Viewport = deprecate( - ({ children }) => children, - ` usage is deprecated, use .addParameters({ viewport }) instead` -); From a5dd722190ed66d11ace1823f927a9c74add0921 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 4 Mar 2019 12:34:36 +1100 Subject: [PATCH 4/7] ADD more warnings to the viewport tool --- addons/viewport/src/Tool.js | 23 ++++++++++++++----- .../stories/addon-viewport.stories.js | 2 +- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/addons/viewport/src/Tool.js b/addons/viewport/src/Tool.js index 6c6c4093edc4..a86c1b54091b 100644 --- a/addons/viewport/src/Tool.js +++ b/addons/viewport/src/Tool.js @@ -1,6 +1,7 @@ import React, { Component, Fragment } from 'react'; import PropTypes from 'prop-types'; import memoize from 'memoizerific'; +import deprecate from 'util-deprecate'; import { Global } from '@storybook/theming'; @@ -27,18 +28,28 @@ const createItem = memoize(1000)((id, name, value, change) => ({ const flip = ({ width, height }) => ({ height: width, width: height }); +const deprecatedViewportString = deprecate( + () => 0, + 'The viewport parameter must be an object with keys `viewports` and `defaultViewport`' +); +const deprecateOnViewportChange = deprecate( + () => 0, + 'The viewport parameter `onViewportChange` is no longer supported' +); + const getState = memoize(10)((props, state, change) => { const data = props.api.getCurrentStoryData(); const parameters = data && data.parameters && data.parameters[PARAM_KEY]; - if (typeof parameters !== 'object') { - // eslint-disable-next-line no-console - console.warn( - 'The viewport parameter must be an object with keys `viewports` and `defaultViewport`' - ); + if (parameters && typeof parameters !== 'object') { + deprecatedViewportString(); } - const { viewports, defaultViewport } = parameters || {}; + const { viewports, defaultViewport, onViewportChange } = parameters || {}; + + if (onViewportChange) { + deprecateOnViewportChange(); + } const list = toList(viewports || INITIAL_VIEWPORTS); diff --git a/examples/official-storybook/stories/addon-viewport.stories.js b/examples/official-storybook/stories/addon-viewport.stories.js index b9d4018df044..89b4a4def401 100644 --- a/examples/official-storybook/stories/addon-viewport.stories.js +++ b/examples/official-storybook/stories/addon-viewport.stories.js @@ -38,5 +38,5 @@ storiesOf('Addons|Viewport.Custom Default (Kindle Fire 2)', module) I respect my parents but I should be looking good on iPad. ), - { viewport: { defaultViewport: 'ipad' } } + { viewport: { defaultViewport: 'ipad', onViewportChange: () => 0 } } ); From cfdffd1b7e72fbd83ddf2fbae89f876b5a4c7097 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 4 Mar 2019 12:36:32 +1100 Subject: [PATCH 5/7] ADD Allow disabling viewport per-story Not sure why you'd want this, but it makes sense to support it. --- addons/viewport/src/Tool.js | 4 ++-- .../official-storybook/stories/addon-viewport.stories.js | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/addons/viewport/src/Tool.js b/addons/viewport/src/Tool.js index a86c1b54091b..2c4390838f19 100644 --- a/addons/viewport/src/Tool.js +++ b/addons/viewport/src/Tool.js @@ -45,13 +45,13 @@ const getState = memoize(10)((props, state, change) => { deprecatedViewportString(); } - const { viewports, defaultViewport, onViewportChange } = parameters || {}; + const { disable, viewports, defaultViewport, onViewportChange } = parameters || {}; if (onViewportChange) { deprecateOnViewportChange(); } - const list = toList(viewports || INITIAL_VIEWPORTS); + const list = disable ? [] : toList(viewports || INITIAL_VIEWPORTS); const selected = state.selected === 'responsive' || list.find(i => i.id === state.selected) diff --git a/examples/official-storybook/stories/addon-viewport.stories.js b/examples/official-storybook/stories/addon-viewport.stories.js index 89b4a4def401..987764632389 100644 --- a/examples/official-storybook/stories/addon-viewport.stories.js +++ b/examples/official-storybook/stories/addon-viewport.stories.js @@ -38,5 +38,8 @@ storiesOf('Addons|Viewport.Custom Default (Kindle Fire 2)', module) I respect my parents but I should be looking good on iPad. ), - { viewport: { defaultViewport: 'ipad', onViewportChange: () => 0 } } - ); + { viewport: { defaultViewport: 'ipad' } } + ) + .add('Disabled', () => There should be no viewport selector in the toolbar, { + viewport: { disable: true }, + }); From 809ccf99b67dc29ea7e330034a5327340e0704cf Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 4 Mar 2019 12:43:11 +1100 Subject: [PATCH 6/7] UPDATE snapshots --- .../addon-viewport.stories.storyshot | 8 + .../__snapshots__/storyshots.test.js.snap | 146 ------------------ 2 files changed, 8 insertions(+), 146 deletions(-) diff --git a/examples/official-storybook/stories/__snapshots__/addon-viewport.stories.storyshot b/examples/official-storybook/stories/__snapshots__/addon-viewport.stories.storyshot index a9645fbf6151..9fcc118a9b63 100644 --- a/examples/official-storybook/stories/__snapshots__/addon-viewport.stories.storyshot +++ b/examples/official-storybook/stories/__snapshots__/addon-viewport.stories.storyshot @@ -8,6 +8,14 @@ exports[`Storyshots Addons|Viewport default 1`] = ` `; +exports[`Storyshots Addons|Viewport.Custom Default (Kindle Fire 2) Disabled 1`] = ` +
+ There should be no viewport selector in the toolbar +
+`; + exports[`Storyshots Addons|Viewport.Custom Default (Kindle Fire 2) Inherited 1`] = `
Date: Mon, 4 Mar 2019 13:20:43 +1100 Subject: [PATCH 7/7] CHANGE link to link --- MIGRATION.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MIGRATION.md b/MIGRATION.md index c683b0b95342..e498b9d97e48 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -183,7 +183,7 @@ addParameters({ viewport: options }); The `withViewport` decorator is also no longer supported and should be replaced with a parameter based API as above. Also the `onViewportChange` callback is no longer supported. -See the README for the viewport addon for more information: https://github.com/storybooks/storybook/blob/master/addons/viewport/README.md +See the [README](https://github.com/storybooks/storybook/blob/master/addons/viewport/README.md) for the viewport addon for more information. ## From version 4.0.x to 4.1.x