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

Switch to using Storybook's new story parameters (Storybook v5) #64

Merged
merged 5 commits into from
Apr 4, 2019

Conversation

timhaines
Copy link
Contributor

@timhaines timhaines commented Feb 9, 2019

Important: This requires a major version bump when releasing and is dependent on Storybook v5.

Breaking Change: This PR removes addWithPercyOptions and replaces it with Storybook's own story parameters for providing options to Percy.

Options are now provided in a percy object in story parameters, either as the 3rd argument to the add function, or with a addParameters function. i.e.:

// add Percy parameters to a single story via the 3rd argument to the `add` function.
storiesOf('Circle Stories', module)
  .add(
    'with multiple widths',
    () => <span>This span renders in widths of 222px and 333px</span>,
    { percy: { widths: [222, 333], rtl: true } }
  );

// OR

// add parameters to all Square Stories with addParameters.
storiesOf('Square Stories', module)
  .addParameters({ percy: { widths: [222, 333], rtl: true } });
  .add(
    'with multiple widths',
    () => <span>This span renders in widths of 222px and 333px</span>,
  );

// OR

// add global parameters to all stories with addParameters in your storybook's config.js.
import { configure, setAddon, addParameters } from '@storybook/react';
addParameters({ percy: { widths: [320, 800] } });

Storybook's behavior is that adding parameters with the add function will be merged into and overwrite clashing parameters provided by addParameters.

This PR also significantly simplifies installation, removing the need for the long step 3.

Closes #50.

Todo:

  • Docs update for on each Percy Storybook docs page for this release, simplifying setup, and replacing options section
  • Maintain docs for v2 that supports Storybook < v5.
  • Ship as pre-release, and have some folk test it.

@timhaines timhaines changed the title [WIP] Switch to using Storybook's new story parameters Switch to using Storybook's new story parameters Feb 11, 2019
Copy link
Contributor

@djones djones left a comment

Choose a reason for hiding this comment

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

🍍 LGTM

@BPScott
Copy link

BPScott commented Feb 14, 2019

Nice! I got very excited about that −19,422 diff before realising it's mostly deleting a stray package.lock, oh well :D

I can't see any tests for global parameters, it'd be nice to make sure those work as you'd expect.

e.g.

//global parameters, applied to all stories
addParameters({ percy: { widths: [222, 333, 444], rtl: true } });

storiesOf('My Story', module)
  .add(
    'with default widths',
    () => <span>This span renders in widths of 222px, 333px and 444px</span>,
  )

Now that global parameters are possible within Percy, is it worth removing the --widths, --rtl and --rtl_regex options from the CLI, as they can be easily expressed with a global addParameters() call? That might help simplify the docs too as you can talk about CLI options, and then parameters that affect stories that can be configured within Percy.

@timhaines
Copy link
Contributor Author

@BPScott In my one attempt, supplying global params with addParameters did not work. I'll follow up on that. If they're supposed to work, I'll figure out why they're not.

@timhaines
Copy link
Contributor Author

@BPScott Have confirmed that adding global parameters does work. My previous test must have been flawed.

@timhaines timhaines force-pushed the update-option-setting branch 3 times, most recently from 434bf19 to 678d829 Compare February 16, 2019 22:52
@Robdel12 Robdel12 mentioned this pull request Mar 6, 2019
@Robdel12
Copy link
Contributor

It'd be nice to also switch this package to use the @percy namespace on NPM since we're doing a major version bump

CC: @djones

@timhaines timhaines merged commit 2a55396 into master Apr 4, 2019
@timhaines timhaines deleted the update-option-setting branch April 4, 2019 22:30
@Robdel12 Robdel12 changed the title Switch to using Storybook's new story parameters Switch to using Storybook's new story parameters (Storybook v5) Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants