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: Missleading example in README #5968

Closed
no23reason opened this issue Mar 8, 2019 · 5 comments
Closed

Viewport addon: Missleading example in README #5968

no23reason opened this issue Mar 8, 2019 · 5 comments

Comments

@no23reason
Copy link

no23reason commented Mar 8, 2019

Describe the bug
Following the example of setting defaultViewport per story in @storybook/addon-viewport README does not work and breaks the viewport configuration.

To Reproduce
Steps to reproduce the behavior:

  1. Create a custom viewport configuration:
import { configure, addParameters } from '@storybook/react';
import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport';

const myViewports = {
    myOne: {
        name: 'MyOne',
        styles: {
            width: '1200px',
            height: '900px'
        }
    }
};

addParameters({
    viewport: {
        viewports: {
            ...INITIAL_VIEWPORTS,
            ...myViewports
        }
    }
});

const req = require.context('../stories', true, /\.tsx?$/);

function loadStories() {
    req.keys().forEach(filename => req(filename));
}

configure(loadStories, module);
  1. Create a story and try to set its default viewport following the example.
addStories('Stories', module)
    .add('story', () => </>, { viewport: 'myOne' });
  1. The viewport list does not contain MyOne and the default viewport is not set.

Expected behavior
The viewport list contains MyOne and it is set as a default viewport.

System:

  • OS: MacOS
  • Device: Macbook Pro 2018
  • Browser: Chrome, Safari
  • Framework: react
  • Addons: viewport
  • Version: 5.0.0
@shilman
Copy link
Member

shilman commented Mar 8, 2019

@tmeasday Didn't #5829 try to address this?

@tmeasday
Copy link
Member

tmeasday commented Mar 8, 2019

You can set viewport: { defaultViewport: 'myOne' }, but cannot set the parameters object to a string (as this breaks auto-merging of parameters).

Did I not end up documenting this properly?

@tmeasday
Copy link
Member

tmeasday commented Mar 8, 2019

Hmm I see I made a typo, I'll fix it.

@tmeasday
Copy link
Member

tmeasday commented Mar 8, 2019

@stale
Copy link

stale bot commented Mar 30, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Mar 30, 2019
@shilman shilman closed this as completed Mar 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants