-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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] How to migrate dynamic knobs to controls? #11984
Comments
Unfortunately controls does not support anything equivalent to dynamic knobs. What's your use case? |
Theme UI (built on top of Styled System) allows to define specific variants for components: I would like to illustrate that, based on the theme, you can select different variants. Would you recommend to stick to knobs then for that? |
Thanks for sharing that. I think the workaround would be to have different stories for different themes, but if you want it fully dynamic, knobs is your best bet for now. |
Is it possible to load different stories based on a global arg? |
Not really, I don't think. It's easy to make a story render differently based on a global arg. But actually loading different stories I don't think so. @tmeasday might have ideas |
Hmm, yeah sorry @flo-sch, this is not something we considered in creating There are quite a few use cases that revolve around "implied" changes when you change theme (for instance wanting to change another set of globals -- e.g. backgrounds). It's pretty tricky to imagine how we could do this in a way that wouldn't be really complex. I wonder to what degree this behaviour is unique to theme or if its something people would want for a variety of globals. |
One thought bubble: rather than changing the sets, we could instead disallow certain combinations of globals or globals + args. You could imagine something like: Default.argTypes = {
variant: {
disallow: (value, { globals: { theme } }) => theme === 'light' : !lightVariants.include(value) ? !darkVariants.include(value))
}
} |
Don't be, this is a question about an edge-case of this great OSS project :) Thanks for the tips, that could be enough, and I will also try to re-think how to document those variants, maybe juste some MDX about it will cover it in the end. Regarding dynamic use of globals, that is the only use-case I can think about right now, but others may have different ones? Not sure if you want to keep this thread open for discussions, otherwise we can close it :) Keep up the good work with SB and Chromatic, it's awesome! |
Maybe let's leave it open until the bot closes it and see what comes up. As with any feature we probably just need to let it permeate through the ecosystem a bit and see what comes back before thinking about what's next ;) |
In a somewhat similar vein with @flo-sch , I've got a set up where one knob ( const secretAgentKnob = (
num: number,
label: string = 'Secret Agent',
groupName: string = 'SecretAgents',
): AgentContact[] => {
const secretAgentsArr: SecretAgentContact[] = [];
for (let i = 0; i < num; i += 1) {
const secretAgentNum = i + 1;
secretAgentsArr.push({
name: text(`${label} ${secretAgentNum} name`, `${label} ${secretAgentNum}`, groupName),
company: text(`${label} ${secretAgentNum} company`, 'CIA', groupName),
role: text(`${label} ${secretAgentNum} role`, 'Agent', groupName),
});
}
return secretAgentsArr;
};
export const InquirySecretAgentDropdown = () => {
const numberOfSecretAgents = number(
'Number of Secret Agents',
5,
{
range: true,
min: 2,
max: 10,
step: 1,
},
'Number of Secret Agents',
);
return <SecretAgentDropdown secretAgents={secretAgentKnob(numberOfSecretAgents)} />;
}; Is there a way to do this with argTypes? I didn't see any documentation about it 😬
|
Nope, not possible currently @frimmy |
What is considered a dynamic knob? |
@SajPrad by dynamic I meant something that depends on the runtime: here, on the context of the given story. Knobs are defined by a function, you can either define them statically before your story, or dynamically within your story (and as @frimmy mentioned, have dynamic options based on other knobs state for instance) Controls however are static, defined at the moment the story is invoked, so for me at the moment controls are not always a replacement, it depends on your use-case |
@flo-sch I see, thanks for the clarification. Right now I want to see how much of previous knobs could be implemented using controls. From what it seems like, it depends on what the knob does and the data it has. |
I am looking for the exact same functionality, have been searching all over for it since exploring the move to v6 to no avail. |
@agrohs it is still possible to use (Not sure if they can be mixed together with controls though) |
Yes, I know we can still use knobs but this seems like such a big design flaw for controls. Playing with some things now to use a custom |
This article (and 3rd party add-on) got my mind thinking to try the Page route: https://medium.com/@atanasstoyanov/custom-documentation-pages-for-storybookjs-13eb9637d6ab |
I love that Controls syncs with your Types, but I am struggling to get a boolean toggle working for one of my props. It's a simple button component and one of it's props is 'icon' which is used to pass in an icon as a React Component (JSX). When I used Knobs I simply use this to toggle the icon on and off:
But when I try to do this in Controls I seem to only be able to pass in the component like this:
Which when I run and view it in Storebook results in a text input showing some random code: '{"key":null,"ref":null,"props":{},"_owner":null,"_store":{}}' Is there a way to toggle this prop on and off? |
I may be missing something obvious, but it looks like a static control use-case to me? export const CalendarButtonStory = ({ showIcon, ...args }) => (
<Button
{...args}
icon={showIcon ? <IconTimeCalendar /> : undefined}
/>
);
CalendarButtonStory.argTypes = {
showIcon: {
control: 'boolean'
}
};
CalendarButtonStory.args = {
showIcon: true
}; Or is the problem that controls also show a strange input for your In that case you could either:
|
@flo-sch Ah awesome, this did what I needed. The only issue now is that the docs page is showing this additional showIcon prop, but this isn't an actual prop used by the component. Is there a way to show the control for showIcon, but hide from the docs table (and vice versa for the icon which I want to show on the docs page, but not in the controls)? |
Not that I know of, but perhaps @tmeasday could confirm that? What I could think of, is instead of using a "storybook-specific argument" in such a case, is to use the (Disclaimer: non tested code) export const CalendarButtonStory = ({ icon, args }) => (
<Button
{...args}
icon={{
'TimeCalendar': <IconTimeCalendar />,
'SomeOtherIconName': <OtherIconComponent />,
// unknown names would result in icon being undefined, which should be fine for the Button?
}[icon]}
/>
);
CalendarButtonStory.argTypes = {
icon: {
control: 'select',
options: [
// blank (or unknown) option that would act as "no icon"
'',
'TimeCalendar',
'SomeOtherIconName',
//...
]
}
};
CalendarButtonStory.args = {
icon: 'TimeCalendar'
}; The caveat is what would be shown in the "source" part with such a conditional rendering, but there are probably ways to make that look better using React displayName or such? |
I think @shilman is the expert here. If it isn't currently possible it definitely falls into the area of things we are looking at. We think "synthetic" args like PS // If you have this
CalendarButtonStory.args = {
showIcon: true
};
// We'll infer this automatically
CalendarButtonStory.argTypes = {
showIcon: {
control: 'boolean'
}
}; |
+1 here too for this |
I've got a very similar use case to the original question (wanting to show users the options available based on the currently selected theme stored in globals). I feel like most if not all cases here could be covered by just allowing
This way there should be no need to hard-code any implementation of anyone's particular use case, we could cover them all pretty simply by just applying the appropriate properties, for example: {
argTypes: (args, { globals }) => ({
// Conditional show / hide of dependent controls: show "iconOptions" only if there's an icon
iconOptions: { table: { disable: !args.icon } },
// Pre-populate with options specific to a context: show variants available in the current theme
variant: { control: { options: myVariantsByTheme[globals.themeName] } }
})
} |
+1 |
Adds a `destroy` method to o-banner. This is used in the o-banner Story to clean up the close button when re-rendered, which o-banner dynamically creates on `init`. This is required to support the `suppressCloseButton` Story control. It appears banner headings (headingLong/headingShort) should only be used with the small/compact layout variants. Not with the default banner style which is full bleed. However it is possible to use Story controls to create that view. `@storybook/addon-knobs` (deprecated) allowed dynamic knobs, so we could hide the heading controls if the default layout was selected. That's not possible with `@storybook/addon-controls`: storybookjs/storybook#11984 I think that's probably a good thing. Support for dynamic controls was worked on but not merge. It's a poor experience when controls shift around. storybookjs/storybook#13890 For now this commit hides the layout control on layout demos, and hides the heading controls from the default layout demo, to avoid showing the discouraged heading + layout combination. However it is still possible to select the base layout with heading on the theme specific demos, so that the small/compact layout can also be selected which is allowed. This could be resolved by exporting 2 templates, one for each kind of banner / usecase. This could make components easier to reason with and maintain. For now this commit sticks to one banner template as coming up with a name without history / useage guidelines is difficult, and we don't know that users aren't already using headings with the base layout - though we never intended it as far as I can tell. The custom theme demo has not been migrated to a Story yet. https://registry.origami.ft.com/components/[email protected]#demo-custom-theme I'm not sure there is value, and maybe harm, in showing made up customised styles alongside those with brand/design approval. Plus it's not clear how to re-create the style without understanding Sass and inspecting demo code. We probably want to have stories for customised components at a later date, with improved guidelines around them and demo Sass/JS pane. See related issue: #370 The description of the demo starts "Although not recommended for design consistency..." Let's not recomend it with a demo.
Destroy Method: Adds a `destroy` method to o-banner. This is used in the o-banner Story to clean up the close button when re-rendered, which o-banner dynamically creates on `init`. This is required to support the `suppressCloseButton` Story control. Hidden Controls: It appears banner headings (headingLong/headingShort) should only be used with the small/compact layout variants. Not with the default banner style which is full bleed. However it is possible to use Story controls to create that view. `@storybook/addon-knobs` (deprecated) allowed dynamic knobs, so we could hide the heading controls if the default layout was selected. That's not possible with `@storybook/addon-controls`: storybookjs/storybook#11984 I think that's probably a good thing. Support for dynamic controls was worked on but not merge. It's a poor experience when controls shift around. storybookjs/storybook#13890 For now this commit hides the layout control on layout demos, and hides the heading controls from the default layout demo, to avoid showing the discouraged heading + layout combination. However it is still possible to select the base layout with heading on the theme specific demos, so that the small/compact layout can also be selected which is allowed. This could be resolved by exporting 2 templates, one for each kind of banner / usecase. This could make components easier to reason with and maintain. For now this commit sticks to one banner template as coming up with a name without history / useage guidelines is difficult, and we don't know that users aren't already using headings with the base layout - though we never intended it as far as I can tell. No Custom Theme Demo: The custom theme demo has not been migrated to a Story yet. https://registry.origami.ft.com/components/[email protected]#demo-custom-theme I'm not sure there is value, and maybe harm, in showing made up customised styles alongside those with brand/design approval. Plus it's not clear how to re-create the style without understanding Sass and inspecting demo code. We probably want to have stories for customised components at a later date, with improved guidelines around them and demo Sass/JS pane. See related issue: #370 No Custom Call To Action Demo: https://registry.origami.ft.com/components/[email protected]#demo-custom-call-to-action-layout The description of the demo starts "Although not recommended for design consistency..." Let's not recomend it with a demo.
Destroy Method: Adds a `destroy` method to o-banner. This is used in the o-banner Story to clean up the close button when re-rendered, which o-banner dynamically creates on `init`. This is required to support the `suppressCloseButton` Story control. Hidden Controls: It appears banner headings (headingLong/headingShort) should only be used with the small/compact layout variants. Not with the default banner style which is full bleed. However it is possible to use Story controls to create that view. `@storybook/addon-knobs` (deprecated) allowed dynamic knobs, so we could hide the heading controls if the default layout was selected. That's not possible with `@storybook/addon-controls`: storybookjs/storybook#11984 I think that's probably a good thing. Support for dynamic controls was worked on but not merge. It's a poor experience when controls shift around. storybookjs/storybook#13890 For now this commit hides the layout control on layout demos, and hides the heading controls from the default layout demo, to avoid showing the discouraged heading + layout combination. However it is still possible to select the base layout with heading on the theme specific demos, so that the small/compact layout can also be selected which is allowed. This could be resolved by exporting 2 templates, one for each kind of banner / usecase. This could make components easier to reason with and maintain. For now this commit sticks to one banner template as coming up with a name without history / useage guidelines is difficult, and we don't know that users aren't already using headings with the base layout - though we never intended it as far as I can tell. No Custom Theme Demo: The custom theme demo has not been migrated to a Story yet. https://registry.origami.ft.com/components/[email protected]#demo-custom-theme I'm not sure there is value, and maybe harm, in showing made up customised styles alongside those with brand/design approval. Plus it's not clear how to re-create the style without understanding Sass and inspecting demo code. We probably want to have stories for customised components at a later date, with improved guidelines around them and demo Sass/JS pane. See related issue: #370 No Custom Call To Action Demo: https://registry.origami.ft.com/components/[email protected]#demo-custom-call-to-action-layout The description of the demo starts "Although not recommended for design consistency..." Let's not recomend it with a demo.
+1 I have used dynamic knobs a lot - for example I had a button component that could use I came across an example recently where I wanted dynamic controls. I have a color prop that can be "primary" "secondary" "gray" or any custom color. Ideally I would make this a select control with the options |
+1 at my company we have the exact same use case as @flo-sch. Enabling selection of variants from the currently active theme. |
Son of a gun!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.5.0-alpha.50 containing PR #17536 that references this issue. Upgrade today to the
Closing this issue. Please re-open if you think there's still more to do. |
I've released the first pass at dynamic controls for dynamic knobs users. Updated documentation available in the PR: #17536 and on the docs site here https://storybook.js.org/docs/6.5/react/essentials/controls#conditional-controls Feedback welcome! |
Nice! Will give that a spin on a feature-branch!
This thought was dropped for the first iteration, right? Just making sure... I guess the current comparison is a simple check for truthiness? Where a property having any value at all evaluates to Unfortunately that's really limiting, still. I think one of the main use-cases (at least that's the case for us) with this was reacting to the values of an |
Looks like a nice feature for toggles, thanks. Checking I've understood correctly, it looks like it has these two limitations:
Is that correct? If it is, I'm sorry but it seems too narrow for, I think, almost all the use cases discussed in this thread which discuss matching particular options and/or matching globals (like theme). Is it not possible to do something like: // There's no control called theme, so it checks for a global called 'theme' and adds if its value === 'dark'
{ ..., addIf: ['theme', 'dark'] } |
Awesome to see some progress with this and a way to add some conditional controls! However, I agree with the comments above that this doesn't address a lot of use cases. I think we either need to open a new issue or leave this issue open. One example of where this falls short would be a color prop that could be And also a nice feature would be to be able for a control to be dependent on multiple controls. This is a bit of a contrived example, but if we only show a border if |
That's a good idea @kmurgic, using objects instead of tuples would be more extensible in future without breaking changes, and would make it really clean to detect if an array of conditions was passed in then recurse over them. |
@shilman Looks good! The only thing that caught my eye was the lack of operators for numbers. Wouldn't it be a good idea to add "greater than" and "less than" operators? |
Good golly!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.5.0-alpha.56 containing PR #17883 that references this issue. Upgrade today to the
NOTE: I might make some additions based on feedback here (e.g. |
@shilman
|
Looks great, thanks! I really like the use of If you wanted to expand on the condition logic later to add more types of test, without inventing a potentially ever-expanding homespun syntax for serialising every comparison someone might use, you could use an existing schema-defining tool for setting validation criteria in a serialisable object, like jsonschema or AJV. For example you could add one new key, In theory you could get an established, documented and tried-and-tested system for all sorts of tests, with minimal new code and minimal new conventions to invent and document. |
Hello. I'm hoping to pass a value into a story argTypes - is this the right chat for that? It might look something like
And it would be created in preview.js? |
Hello! I have a complex storybook using conditional knobs to migrate to controls. Some knobs are visible only in specific case with complex condition. like let extraKnob;
if (firstKnob && secondKnob) {
extraKnob = boolean('optional knob only available when 2 others are specified', false);
} It's currently not possible with controls: #18542 |
Just want to make sure I'm not missing something (@shilman, @tmeasday), but I haven't found mention of a similar use case to below and wondering what the best suggestion might be in the new world of controls. Previous "dependent knob" code below:
"Real example", but specifically how to get one select knob such as |
Question summary
After migration to Storybook v6, I am trying to replace dynamic knobs by the new controls addon.
However I had some dynamic knobs providing options from e.g. a global parameter:
I have a global decorator wrapping stories with a
The theme comes from a global (selection from the toolbar)
Since controls seem to be statically provided, is it possible to migrate those, and how?
I was thinking, maybe if argTypes options could be defined as a function (as an opt-in advanced feature) receiving the context as arguments, it might do the trick?
Please specify which version of Storybook and optionally any affected addons that you're running
The text was updated successfully, but these errors were encountered: