Storybook: [Backgrounds addon] Set a different default background for a specific story

Created on 16 Dec 2019 · 14Comments · Source: storybookjs/storybook

Is your feature request related to a problem? Please describe.
I have a component prop that controls whether the component is visible on a dark background or a light background. In the dark-background story, I'd like to default to the dark background by name, rather than overriding all the backgrounds for that component.

Describe the solution you'd like
Something like:

storiesOf("Logo", module)
  .add("horizontal", () => <Logo layout="horizontal" />)
  .add("vertical", () => <Logo layout="vertical" />)
  .add("horizontal - inverted", () => <Logo layout="horizontal" inverted/>, {defaultBackground: "primary"})
  .add("vertical - inverted", () => <Logo layout="vertical" inverted/>, {defaultBackground: "primary"});

Describe alternatives you've considered
I saw on the documentation page that you can override the background for a component, but I don't see a way to leave the other options and just change which one is the default. It may be possible, but I don't see how in the documentation.

Are you able to assist bring the feature to reality?
Maybe; I'm not sure if I can.

BREAKING CHANGE backgrounds maintenance question / support todo

Source

iansan5653

Most helpful comment

Turns out that #9219 doesn't really apply. Here's what I'm thinking for a better backgrounds API. We can deprecate the old API in 6.0 and remove it in 7.0:

// preview.js
addParameters({
  backgrounds: {
    default: 'white',
    values: [
      { name: 'white', value: '#ffffff' }, 
      { name: 'black', value: '#000000' },
    ]
  }
}

// Foo.stories.js
export const Foo = ...
Foo.story = {
  parameters: {
    backgrounds: { default: 'black' },
  }
}

shilman on 23 Dec 2019

👍2

All 14 comments

@kevin940726 this seems related to your change in #9137, but for backgrounds instead of viewports. any chance you can take a quick look?

shilman on 18 Dec 2019

Sure, I'll take a look. Thx for asking!

kevin940726 on 18 Dec 2019

❤1

We can support this by accepting a new field defaultBackground in the background parameter config.

Overridden.story = {
  parameters: {
    backgrounds: { defaultBackground: "primary" }
  }
};

The API is intended for the users to override the backgrounds, but I agree that it's somewhat annoying to do as a regular basis.

IMHO the current config API is pretty simple, and thus not so powerful and error-prone. If we want to provide more control and flexibility, we might want to consider a different data structure? Maybe a normalized object with an ordered list. (In addition, maybe the order doesn't really matter? Or we can pre-sort the order for the users? based on brightness or something like that.)

Needless to say, there would probably be breaking changes. I can help make this (defaultBackground) happen first though, please let me know what you think about this :)

kevin940726 on 19 Dec 2019

The defaultBackground method looks perfect! It's exactly what I was thinking of.

iansan5653 on 19 Dec 2019

👍2

Here's some updates after digging into the source a little bit more. Turns out that the parameters value is merged upon creation, so we cannot get the parents' parameters if they've been overridden. It's a fundamental design choice, so it's not easy to fix. (Correct me if I was wrong though!)

For now the best way to work around this issue is to define a global variable and a marker function to mark the default.

const BACKGROUNDS = [
  { name: "white", value: "#ffffff" },
  { name: "light", value: "#eeeeee" },
  { name: "gray", value: "#cccccc" },
  { name: "dark", value: "#222222" },
  { name: "black", value: "#000000" }
];

function markDefault(backgrounds, defaultBackground) {
  // make a copy
  const newBackgrounds = backgrounds.map(bg => ({ ...bg }));

  const defaultBg = newBackgrounds.find(bg => bg.name === defaultBackground);
  defaultBg.default = true;

  return newBackgrounds;
}

export default {
  parameters: {
    backgrounds: markDefault(BACKGROUNDS, "dark")
  }
};

Story1.story = {
  parameters: {
    backgrounds: markDefault(BACKGROUNDS, "light")
  }
};

It's not ideal, but it's the only way I can think of with current limitations.

We could provide a different parameter name like defaultBackground, but I doubt that it would be a better idea?

Story1.story = {
  parameters: {
    defaultBackground: "light"
    // instead of backgrounds: {  defaultBackground: 'light'  }
  }
};

kevin940726 on 22 Dec 2019

Thanks @kevin940726. The way we handle parameters is actually up for grabs. Just filed #9219. It doesn't solve this problem directly, but you can see how it would be much easier to solve if the parameters were shallow-merged.

cc @ndelangen @tmeasday

shilman on 22 Dec 2019

👍1

Turns out that #9219 doesn't really apply. Here's what I'm thinking for a better backgrounds API. We can deprecate the old API in 6.0 and remove it in 7.0:

// preview.js
addParameters({
  backgrounds: {
    default: 'white',
    values: [
      { name: 'white', value: '#ffffff' }, 
      { name: 'black', value: '#000000' },
    ]
  }
}

// Foo.stories.js
export const Foo = ...
Foo.story = {
  parameters: {
    backgrounds: { default: 'black' },
  }
}

shilman on 23 Dec 2019

👍2

Yeah perhaps this is a better solution in the 6.0 timeframe than my plan :)

tmeasday on 23 Dec 2019

Although we are going to to run into the problem that @kylesuss was debugging in chromaui/chromatic-cli#80, and see backgrounds.values get concat-ed if you try and overwrite it :/

I feel like we should drop that behaviour, I wonder why we added it?

Update: it looks like @ndelangen added it in order to fix some tests: https://github.com/storybookjs/storybook/commit/deade9dd75859d70ac1ec48b88b46ab873a74120

I guess if we remove it, we can see which tests break to understand why ;)

tmeasday on 23 Dec 2019

👍1

Yeah I think concatenating the values is not a desirable behavior. @ndelangen what's the use case? I propose we revert this in 6.0.

shilman on 23 Dec 2019

I agree we have a clearer picture of the best practices now, and should form an opinion on the dataformat inside parameters.

addParameters({
  backgrounds: {
    default: 'white',
    values: [
      { name: 'white', value: '#ffffff' }, 
      { name: 'black', value: '#000000' },
    ]
  }
}

Looks like an improvement to me!

ndelangen on 23 Dec 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!