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

Addon-controls: configuring argTypes globally on preview.js #11697

Closed
guilhermewaess opened this issue Jul 27, 2020 · 32 comments
Closed

Addon-controls: configuring argTypes globally on preview.js #11697

guilhermewaess opened this issue Jul 27, 2020 · 32 comments

Comments

@guilhermewaess
Copy link
Contributor

Would be nice to have a way to configure some controls globally.

Example: imagine that all components contain one prop called ComponentId and we don't want this to be showing in the controls table.

How we do now:

On each component.stories.tsx you have to configure it manually within the default export:

export default {
  argTypes: { ComponentId: { table: { disabled: true } }
}

Proposal

Add this as configuration inside the preview.js

preview.js

export const argTypes: { ComponentId: { table: { disabled: true } }

And then it will be applied for all stories.

@ShaunaGordon
Copy link

To add another use-case:

I showcase different themes for my components (the third party themes addon doesn't work in my case, because of how the classes are applied vs how I need them, and I'd prefer users be able to tinker with components all in one place). The theming is done as a value passed in via a property, which I've set controls up to display the arg as an enum (because only certain values will work).

This arg is the same across all components and will contain the same selection options. The ideal way to set this up would be a global setting that provides that information without needing to add it to every single component.

@stale
Copy link

stale bot commented Sep 20, 2020

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!

@tmeasday
Copy link
Member

tmeasday commented Oct 1, 2020

I'm happy to help someone get a PR over the line adding this.

@shilman shilman modified the milestones: 6.1 args, 6.2 args Oct 13, 2020
@shilman shilman removed the tracked label Nov 4, 2020
@PupoSDC
Copy link

PupoSDC commented Dec 2, 2020

Hey! I'd love to try and tackle this.
If i understood the API correctly, we don't really need to change a lot, something like this should do:

--- a/addons/controls/src/components/ControlsPanel.tsx
+++ b/addons/controls/src/components/ControlsPanel.tsx
@@ -1,30 +1,45 @@
 import React, { FC } from 'react';
 import { ArgsTable, NoControlsWarning } from '@storybook/components';
-import { useArgs, useArgTypes, useParameter } from '@storybook/api';
+import { ArgTypes, useArgs, useArgTypes, useParameter } from '@storybook/api';

 import { PARAM_KEY } from '../constants';

 interface ControlsParameters {
   expanded?: boolean;
   hideNoControlsWarning?: boolean;
+  argTypes?: ArgTypes;
 }

 export const ControlsPanel: FC = () => {
   const [args, updateArgs, resetArgs] = useArgs();
   const rows = useArgTypes();
   const isArgsStory = useParameter<boolean>('__isArgsStory', false);
-  const { expanded, hideNoControlsWarning = false } = useParameter<ControlsParameters>(
+  const { expanded, hideNoControlsWarning = false, argTypes } = useParameter<ControlsParameters>(
     PARAM_KEY,
     {}
   );
   const hasControls = Object.values(rows).filter((argType) => !!argType?.control).length > 0;
+
+  const rowsWithDefaults = Object
+    .entries(rows)
+    .reduce((sum , [key, value]) => {
+      const defaultControls = argTypes[key] ?? {};
+      return {
+        ...sum,
+        [key]: {
+          ...defaultControls,
+          ...value
+        }
+      }
+    }, {} as ArgTypes);
+
   return (
     <>
       {(hasControls && isArgsStory) || hideNoControlsWarning ? null : <NoControlsWarning />}
       <ArgsTable
         {...{
           compact: !expanded && hasControls,
-          rows,
+          rows: rowsWithDefaults,
           args,
           updateArgs,
           resetArgs,

however, I'm a bit lost on how to test / experiment with this? Is there any guide i missed?

@tmeasday
Copy link
Member

tmeasday commented Dec 2, 2020

Hi @PupoSDC -- I'm not quite sure what that snippet does but that's not what I had in mind. I was thinking we'd add a concept of "global" args/argTypes, similar to global parameters and decorators. We would wire it up so it made its way from the .storybook/preview file into the StoryStore and then from there it should all just work automatically.

So basically we'd need to add:

  1. Support for a args and argTypes export key in preview (this is done via a virtual module entry (identically) in two places: https://github.com/storybookjs/storybook/blob/next/lib/builder-webpack4/src/preview/virtualModuleEntry.template.js / https://github.com/storybookjs/storybook/blob/next/lib/builder-webpack5/src/preview/virtualModuleEntry.template.js)

  2. Add a addArgs and addArgTypes export to @storybook/client-api, via ClientApi.js, which ultimately calls into story_store.js. All of this would be very similar to how parameters, decorators etc are handled.

  3. ClientApi would ultimately just put args and argTypes into parameters (this is what we do at the story level too:

    args: kindArgs,
    argTypes: kindArgTypes,
    /
    args,
    argTypes,
    )

  4. At this point it would all "just" work due to parameters inheritance. The one thing we'd need to think about would be how to do denormalized argtype enhancers (cc @shilman) but we haven't actually merged that PR yet: Added componentArgTypeEnhancers #11097

@PupoSDC
Copy link

PupoSDC commented Dec 2, 2020

Thank you for the extra information!

This is my first time contributing here, so im not fully aware of the ecosystem.

My idea, would have the argTypes as part of the parameters export`. But your strategy seems more sound.

Im just a bit lost, on how to test / develop this. is there any "development environment" i can quickly set up?

Also, is it worth it for me to pick this up? Or is it already a WIP?

@tmeasday
Copy link
Member

tmeasday commented Dec 3, 2020

My idea, would have the argTypes as part of the parameters export`. But your strategy seems more sound.

This would be a simple way to prototype it for sure! We wouldn't want to leave them there long term as we'd reserve the right to use a different mechanism to store arg[Types] but it would be a simple way to try it out.

I think right now we have a warning/error if you try and put them in there so a start could just be to drop the warning and try it out in some stories in official storybook.

The instructions are here but my TLDR is:

  1. Checkout the next branch of the monorepo

  2. Run yarn and yarn bootstrap --core

  3. Run yarn build --watch core client-api api [you might need more packages than this but that'll get you started]

  4. In another terminal inside examples/official-storybook run yarn storybook.

Then I would add some more stories to the official storybook that use global args/arg types and see what happens.

Also, is it worth it for me to pick this up? Or is it already a WIP?

Nope (not a WIP)!

@PupoSDC
Copy link

PupoSDC commented Dec 3, 2020

i was missing step 4. Thanks! I will try to work on this in the coming weekend!

@stale
Copy link

stale bot commented Dec 25, 2020

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 Dec 25, 2020
@midastouchprd
Copy link

This would be lovely....what else does it need?

@stale stale bot removed the inactive label Dec 28, 2020
@shilman shilman added this to the 6.3 controls milestone Apr 1, 2021
@mancristiana
Copy link

I am also interested in this feature, except I would like to configure global custom props, rather than disable children props. I had created an issue on Stackoverflow https://stackoverflow.com/questions/67074392/how-can-i-add-a-global-control-matcher-in-storybook expecting this was already possible.

Here was my attempt to edit preview.js to add a global configuration for props named icon. It would be amazing if it was possible to implement something that would be used like this:

export const parameters = {
    controls: {
        icon: {
            control: {
                type: 'select'
            },
            options: iconOptions,
            defaultValue: DefaultIcon,
        }
    }
};

Should I open a different feature request for this?
I would like to contribute and help to build this feature!

@tmeasday
Copy link
Member

@mancristiana I think you are looking for this feature (although it would be parameters.argTypes.icon... rather than parameters.controls.icon..., right?).

I am happy to help if you want to work on it. I think the comments I made above are still relevant.

@mancristiana
Copy link

@tmeasday thanks for clarifying! You are correct, I meant parameters.argTypes.icon....
I will attempt to implement this sometime this week following your comments! 😄

@mancristiana
Copy link

Hi @PupoSDC -- I'm not quite sure what that snippet does but that's not what I had in mind. I was thinking we'd add a concept of "global" args/argTypes, similar to global parameters and decorators. We would wire it up so it made its way from the .storybook/preview file into the StoryStore and then from there it should all just work automatically.

So basically we'd need to add:

  1. Support for a args and argTypes export key in preview (this is done via a virtual module entry here: https://github.com/storybookjs/storybook/blob/next/lib/core/src/server/preview/virtualModuleEntry.template.js)
  2. Add a addArgs and addArgTypes export to @storybook/client-api, via ClientApi.js, which ultimately calls into story_store.js. All of this would be very similar to how parameters, decorators etc are handled.
  3. ClientApi would ultimately just put args and argTypes into parameters (this is what we do at the story level too:
    args: kindArgs,
    argTypes: kindArgTypes,

    /
    args,
    argTypes,

    )
  4. At this point it would all "just" work due to parameters inheritance. The one thing we'd need to think about would be how to do denormalized argtype enhancers (cc @shilman) but we haven't actually merged that PR yet: Added componentArgTypeEnhancers #11097

@tmeasday could you update these steps since

  • the virtual module entry you are referencing in step 1 no longer exists. Also, I can see we are using TypeScript so I assume this step might be updating a type instead. Which file would this be?
  • addParameters and addDecorators seem to have been deprecated in favour of exporting parameters and decorators

I really appreciate your guidance, especially since this is my first time contributing to a popular open-source library ❤️ !

@tmeasday
Copy link
Member

tmeasday commented Apr 26, 2021

@mancristiana - good pickup! - I'll update the comment to be more accurate, but in short:

the virtual module entry you are referencing in step 1 no longer exists. Also, I can see we are using TypeScript so I assume this step might be updating a type instead. Which file would this be?

I'm not sure why it is a JS file, probably a webpack thing.

addParameters and addDecorators seem to have been deprecated in favour of exporting parameters and decorators

This file is actually where we transform the exported parameters/decorators into calls to addParameters/addDecorators in client_api.ts. Does that make sense?

@tmeasday
Copy link
Member

@mancristiana if you are still looking at this, this PR does a lot of similar things to the above and could be a useful reference: #14901

@mancristiana
Copy link

Thanks for the reference. I am still looking into this

@mancristiana
Copy link

I made a bit of progress on this issue and added a draft pull request here #14934.
Adding global args seems straightforward. However, I am not sure how to implement global argTypes since I expect them to be applied only for the stories where components use those props. For example, if I configure an icon option as part of global argTypes, I would expect only components exposing the icon prop would get the icon control.

How would you suggest implementing global argTypes?

@yannbf
Copy link
Member

yannbf commented May 14, 2021

I made a bit of progress on this issue and added a draft pull request here #14934.
Adding global args seems straightforward. However, I am not sure how to implement global argTypes since I expect them to be applied only for the stories where components use those props. For example, if I configure an icon option as part of global argTypes, I would expect only components exposing the icon prop would get the icon control.

How would you suggest implementing global argTypes?

The interesting thing about argTypes is that they don't need to relate to a property that exists in a component. If the argTypes are now only applied to components which expose the property, this would be a breaking change 🤔

@tmeasday
Copy link
Member

I made a bit of progress on this issue and added a draft pull request here #14934.

Looks good so far, nice job.

However, I am not sure how to implement global argTypes since I expect them to be applied only for the stories where components use those props. For example, if I configure an icon option as part of global argTypes, I would expect only components exposing the icon prop would get the icon control.

Hmm, I'm not sure I agree with the semantics here. I think if you have:

// in preview.js
export const argTypes = {
  icon: { control: 'icon' },
};

Then you are saying "the arg type icon always uses an icon control" which I feel like implies that the arg/prop always exists.

Maybe for the more subtle case of "if the arg type icon exists, then it should have an icon control", you should use an argTyp enhancer?

// in preview.js
export const argTypeEnhancers = [
  ({ argTypes }) => ({ ...argTypes, ...(argTypes.icon && { icon: { ...argTypes.icon, control: 'icon' } }),
];

WDYT @shilman? It's both harder to write and maybe more work for the store to process. Hmmm.

An alternative would be not to treat an argType as "set" unless it has an actual type field, rather than just other decorating metadata.

@shilman
Copy link
Member

shilman commented May 17, 2021

@tmeasday I agree with your proposal. The semantics of args ArgTypes should be consistent with the other metadata we support: parameters & decorators both cascade, and ArgTypes should too.

We have another mechanism for enriching the types of existing ArgTypes: matchers. I think this is a better user-facing API for augmenting ArgTypes, and we should reserve the "enhancer" API for addons and possibly not even expose this as a public API.

@shilman shilman added the small label Jun 8, 2021
@shilman shilman modified the milestones: 6.3 controls, 6.4 improvements Jun 8, 2021
@sauldeleon
Copy link

@shilman Does your last comment mean that this is not going to be done at the end? Do you recommend then using the matchers approach?

I have a similar use case, I have a theme property that I would like to control globally with a custom toolbar and a custom ThemeProvider, and then hide the theme property for all argTypes in my stories.

Do you think some global matcher could help? Like

if(arg.match(/theme/)){
  //hide the arg
}

Thanks!

@shilman
Copy link
Member

shilman commented Jul 9, 2021

@sauldeleon No, I agree with the basic premise of this issue. There's an internal API that @tmeasday was suggesting we might expose, and I was disagreeing with that.

If you want a global theme, I would use globals, which is exactly for that use case, and doesn't show up in your args at all: https://storybook.js.org/docs/react/essentials/toolbars-and-globals

@leepowelldev
Copy link

We have another mechanism for enriching the types of existing ArgTypes: matchers. I think this is a better user-facing API for augmenting ArgTypes, and we should reserve the "enhancer" API for addons and possibly not even expose this as a public API.

matchers are great, but is there a way of disabling controls using them?

export const parameters = {
  controls: {
    matchers: {
      'false': /^children$/i,
    },
  },
};

This would match the story defined version:

argTypes: {
  children: {
    control: false
  }
}

@tmeasday
Copy link
Member

FYI this was implemented in 6.4, currently only for storyStoreV7.

@shilman
Copy link
Member

shilman commented Jan 5, 2022

Olé!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.5.0-alpha.9 containing PR #17043 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Jan 5, 2022
@EliezerB123
Copy link

EliezerB123 commented Aug 17, 2022

..So in conclusion, if we want a conditional global argType (In order to hide if if there's no corresponding arg, as there's no reason to ever display a global argType if there's no arg for that component) we should do this?

(Matchers obviously won't work because we can't assign categories, subcategories, disable, or name to them.)

// in preview.js
export const argTypeEnhancers = [
  ({ argTypes }) => ({ ...argTypes, ...(argTypes.icon && { icon: { ...argTypes.icon, control: 'icon' } }),
];

Edit:
OKAY. This seems to be a reasonable workaround, if your @input has a default value:

// in preview.js
export const argTypes = {
  text: { table: { category: "Global"}, if:{ arg: "text", exists: true }},
  isDisabled: { table: { category: "Global"}, if:{ arg :"isDisabled", exists: true }},
};

Essentially, if your code has isDisabled or text with a default value in the code (In Angular, this would be e.g. @Input() isDisabled:boolean = false ), then Storybook will get an arg from Compudoc, allowing you only display the argType if it actually exists on your component.

(Of course, this won't help at all if you've also defined a default arg inside preview.js. Maybe someone else has an idea of what to do.)

@Devqon
Copy link

Devqon commented Jan 26, 2023

@EliezerB123 thank your for this example, this is exactly what I need too.
We have some components where we display images from our static assets. I wanted to create a custom control for an image selector with preview in the dropdown, but that does not seem possible (yet?)

Our current 'workaround' is to have a global options, but we only want to show the control if it really exists in the args. We now do it like this:

// .storybook/images.ts
type Image = {
  name: string;
  url: string;
}

const IMAGES = [
  { name: 'Awesome image', url: '/assets/img/awesome-image.jpg', },
  // ..
];

const getImageArgType = (controlName: string, images: Image[]) => {
  return {
    control: {
      type: 'select',
      labels: images.reduce((a, b) => {
        ...a,
        [b.url]: b.name,
      }, {}),
    },
    options: images.map(image => image.url),
    if: {
      arg: controlName,
      exists: true,
    },
  }
}
// .storybook/preview.ts
export const argTypes = {
  image: getImageArgType('image', IMAGES),
};

@YassienW
Copy link

YassienW commented Oct 6, 2023

I don't think it should be based on whether an arg is provided or not. There is no need for a globally defined argType to render if the component doesn't even have that prop in its definitions.

I want to display all the component's props without showing extra globally defined argTypes that don't apply to this specific component, currently there is no straight forward way to do it. Having to add a default prop for all props that i don't want to add to the args table isn't always reasonable. The other solution is to not use global arg types at all, which would be unfortunate because global argTypes are very convenient.

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