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-docs: Show source block with generated knob values #8342

Closed
JamesIves opened this issue Oct 8, 2019 · 47 comments
Closed

Addon-docs: Show source block with generated knob values #8342

JamesIves opened this issue Oct 8, 2019 · 47 comments

Comments

@JamesIves
Copy link

JamesIves commented Oct 8, 2019

Describe the bug

I'm using addon-knobs along with addon-docs, and I'm having a hard time getting the two to work well together.

Within my Component.stories.mdx file I have the following:

export const defaultProps = (selected, icon) => ({
  selected: boolean('selected', false),
  label: text('label', 'Essentials'),
  description: text(
    'description',
    'This s a description.'
  ),
});

<Preview>
  <Story name="default">
    <Box
      {...defaultProps(false)}
    />
  </Story>
</Preview>

I'm using defaultProps as a helper function for all of my stories in the mdx file as they all share the same props. The problem I have is that when this renders in the Storybook docs page it shows the function call instead of all of the props.

Screen Shot 2019-10-08 at 10 47 56 AM

If i don't use a function to generate these, I still see the addon-knobs function call instead of the actual prop value.

Screen Shot 2019-10-08 at 10 48 55 AM

To Reproduce

  1. Create a mdx story with addon-knobs.
  2. Observe that any created story ends up having the addon-knobs function call instead of the value the functions return.

Expected behavior

I'd expect it to show the actual prop values. For example:

<Box selected={true} label="The label" />

Is there a correct way to do this? With the way it works currently it's hard to define stories inside of the mdx file exclusively.

@JamesIves JamesIves changed the title addon-knobs doesn't work correctly with addon-knobs addon-knobs doesn't work well with addon-knobs Oct 8, 2019
@JamesIves JamesIves changed the title addon-knobs doesn't work well with addon-knobs addon-knobs doesn't work well with addon-docs Oct 8, 2019
@JamesIves
Copy link
Author

JamesIves commented Oct 10, 2019

I've also experienced some inconsistencies with getting the default knob values to apply to stories that are created through the mdx format.

For instance the following code should apply the selected prop as false In the first occurrence, and in the second one as true.

export const defaultProps = (selected) => ({
  selected: boolean('selected', selected),
  label: text('label', 'Essentials'),
  description: text(
    'description',
    'These are the basics: paying the bills and putting food on the table. They include your non-discretionary spending.'
  ),
});


<Preview>
  <Story name="default">
    <Box {...defaultProps(false)}/>
  </Story>
</Preview>

<Preview>
  <Story name="selected">
    <Box {...defaultProps(true)}  />
  </Story>
</Preview>

In this scenario, whatever the first defaultProps() function call returns is what gets applied to all stories. This makes creating several staged stories difficult when there's a lot of shared props.

Doing the following pattern does appear to work correctly, so I'm wondering if it's somewhere in the addon-knobs addon.

export const myName = (selected) => ({selected})

<Preview>
  <Story name="default">
    <Box {...myName(false)}/>
  </Story>
</Preview>

<Preview>
  <Story name="selected">
    <Box {...myName(true)}  />
  </Story>
</Preview>

If there's a workaround, or a potential starting point to work on a fix I'd be happy to take a look.

@JamesIves
Copy link
Author

Let me know if I should make a separate issue for the problem above @shilman ☝️

@shilman
Copy link
Member

shilman commented Oct 10, 2019

Thanks, we can keep them lumped together for now!

@kkorach
Copy link

kkorach commented Oct 11, 2019

I'm seeing the same thing with CSF stories. If the prop is set by a knob then it looks like its using the component's default value instead of what the knob says.

@mrmckeb
Copy link
Member

mrmckeb commented Oct 15, 2019

Adding a groupId (third parameter) to knobs solved this for me.

Each should be unique.

text('Text', basic, 'basic')

@JamesIves
Copy link
Author

Adding a groupId (third parameter) to knobs solved this for me.

Each should be unique.

text('Text', basic, 'basic')

Which version of knobs are you using? This workaround doesn't seem to work for me. In the Canvas view it looks correct, but not in the Docs view.

@mrmckeb
Copy link
Member

mrmckeb commented Oct 16, 2019

Everything is at latest stable (5.2.4). This could be related to something else though in your case.

What version are you on?

@JamesIves
Copy link
Author

The same version that you're on. I can try and get a working example soon, we've scraped the use of addon-docs until this can get figured out.

@joelstransky
Copy link

Just include withKnobs in the Meta decorators and knobs work same as they do in CSF.
<Meta title="Desktop|Image Gallery" component={ImageGallery} decorators={[withKnobs]} />

@stale
Copy link

stale bot commented Nov 20, 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 Nov 20, 2019
@daneah
Copy link

daneah commented Dec 19, 2019

I was inquiring about roughly this issue in Discord the other night—it appears that knobs with the same label act something like singletons in Docs mode, and the first default value that's defined is used for all components in the Docs tab. Adding a groupId is a nice idea and worked for me just now on 5.3.0-beta.31.

@stale stale bot removed the inactive label Dec 19, 2019
@stale
Copy link

stale bot commented Jan 9, 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 Jan 9, 2020
@daneah
Copy link

daneah commented Jan 9, 2020

This is still valuable to fix.

@stale stale bot removed the inactive label Jan 9, 2020
@joelstransky
Copy link

@daneah This might be better as a new ticket.

@liamross
Copy link

liamross commented Jan 9, 2020

Yeah this is still broken on version 5.3.0-rc.12, even after adding groupId. I have a button component with some simple props and this is what I get in the docs "Show code" expandable:

() => (
<Button
  disabled={boolean('disabled', false, '_')}
  onClick={action('onClick')}
>
  {text('children', 'Button content', '_')}
</Button>
)

For reference, here's my Button.stories.tsx file:

import { action } from '@storybook/addon-actions';
import { boolean, text } from '@storybook/addon-knobs';
import React from 'react';
import Button from '../Button';
import docs from './README.md';

export default { title: 'Button', component: Button, parameters: { info: docs } };

export const Basic = () => (
  <Button
    disabled={boolean('disabled', false, '_')}
    onClick={action('onClick')}
  >
    {text('children', 'Button content', '_')}
  </Button>
);

Also, while I'm here, I am wondering if there are any plans to allow you to set knobs within docs without having to go back to canvas? Between this issue and the knobs not being within docs, It is making me strongly consider removing story previews from docs entirely since they provide minimal value (non configurable and with broken code previews)

@monospaced
Copy link

monospaced commented Jan 9, 2020

Can also confirm what @liamross is saying. My approach has been to write new stories, without knobs, that speicfically target the docs page code previews. Then include a knobs story to be access by developers, in the canvas tab, that I exclude from the docs:

export const basic = () => (
  <Image alt="Kitten" src="http://placekitten.com/800/432" />
);

export const knobs = () => (
  <Image
    alt={text('alt', 'Kitten')}
    src={text('src', 'http://placekitten.com/800/432')}
  />
);

knobs.story = { parameters: { docs: { disable: true } } };

This works pretty well, but would not be necessary if the code previews showed the default values specifed by the knobs functions.

@arfa
Copy link

arfa commented Jan 17, 2020

@monospaced

also this work

export default {
  title: 'Button/pros',
  component: Button,
  parameters: { docs: { disable: true } },
};
export const button: React.FC<ButtonProps> = () => <Button>{text('children', 'Button content', '_')}</Button>;

@shilman shilman changed the title addon-knobs doesn't work well with addon-docs Addon-docs: Show source block with generated knob values Jan 17, 2020
@shilman
Copy link
Member

shilman commented Jan 21, 2020

@liamross #6639 is coming, probably in a 6.x release.

@joelstransky
Copy link

I still don't see the point of having Knobs UI in Docs. Would it still be an add-on tab? Would it be one set of knobs that controls all the stories with Knob params or would a Knob UI need to be appended to every story? Why not include a few more stories showing the important variations? I'm not saying it's pointless, just doesn't seem like a high priority.

@JamesIves
Copy link
Author

JamesIves commented Jan 22, 2020

I still don't see the point of having Knobs UI in Docs. Would it still be an add-on tab? Would it be one set of knobs that controls all the stories with Knob params or would a Knob UI need to be appended to every story? Why not include a few more stories showing the important variations? I'm not saying it's pointless, just doesn't seem like a high priority.

For me at least It's not so much about making the knobs visible in the docs so much as it's making the previews that are generated as an outcome useful. Otherwise you end up with a bunch of component previews with business logic in them.

Originally we wanted to define all of our stories in our docs (still using Canvas for addons) but we had to move away from that approach in favor of the storiesOf api to make this work correctly. Now we define our stories in the story.js file, and we setup repeated examples in the .mdx file. Admittedly this works well, but it's not entirely future proofed I feel.

https://github.com/UnitedIncome/components/tree/develop/components/molecules/Cabinet

@joelstransky
Copy link

joelstransky commented Jan 22, 2020

Otherwise you end up with a bunch of component previews with business logic in them.

Can you elaborate on this?

Originally we wanted to define all of our stories in our docs (still using Canvas for addons) but we had to move away from that approach in favor of the storiesOf api to make this work correctly. Now we define our stories in the story.js file, and we setup repeated examples in the .mdx file. Admittedly this works well, but it's not entirely future proofed I feel.
https://github.com/UnitedIncome/components/tree/develop/components/molecules/Cabinet

Great looking code there btw. Maybe it's my level of experience with Storybook but I don't understand this. I define everything in a single MDX and get both a Canvas tab where I can play with add-ons and a Docs tab that renders my MDX in full. Is it that docs contaminate the canvas or visa versa? I can definitely understand wanting more control over content, just hasn't been a problem for me yet.

@JamesIves
Copy link
Author

The biggest reason we have to separate the two out is mostly due to how the preview code gets generated. As knobs require a function the value gets rendered in the preview as that, instead of what the result of that function produces.

<Dropdown label={text('label', 'Default')} />

In our markdown file we generate document specific versions that have more context around why they are being displayed that don't use knobs to prevent this problem. In the example above I'd expect label to look like this:

<Dropdown label="Default" />

When you combine that with a helper function that generates a series of default props the code examples end up looking really messy. The same thing happens with the InVision integration:

<DollarCircleIllustration {...defaultProps(false, false, false)} />

I'd love for there to be an easy way to generate clean code previews as that will prevent a bunch of repetition on our end. I'd also love for there to be an easy way to generate stories that are defined in the .mdx file, but hidden from the Docs tab so they only appear in the canvas.

@joelstransky
Copy link

Thanks for elaborating!
Yes, I hate seeing add-on code in the sample. Maybe storybook needs a babel extension.
And yah, seems like a simple prop could flag a story as "Docs only". Similar to the OP's point of needing to be able to dictate if knobs should render controls next to MDX stories.

@shilman shilman added the todo label Jan 23, 2020
@atanasster
Copy link
Member

atanasster commented Jan 23, 2020

@JamesIves and other participants - you make great points. We are adding to the csf story format, and with sb6+ the properties(knobs) will be created in a declarative way and without any addon code in the story, so this should address your main concerns.
here's how you would write your knob'ed stories in 6.0:

export const selectPropStory = ({ label, color, backgroundColor }) => (
  <Button color={color} backgroundColor ={backgroundColor}>{label}</Button >
);

Additionally, we can add a second tab to the preview component to display the raw html code, maybe even a dom-type explorer for the story - what you think?

@joelstransky
Copy link

I certainly wouldn't mind a literal component. something like:

const knobs = {
  disabled: boolean("Disabled", false),
  label: text("Label", "Hello Storybook")
};
<Preview>
  <Story name="all checkboxes">
    <Knobs {...knobs}>
      {({ disabled, label }) => <button disabled={disabled}>{label}</button>}
    </Knobs>
  </Story>
</Preview>;

@atanasster
Copy link
Member

Yes, that would be the goal, sth


<Preview>
  <Story name="all checkboxes" properties={knobs}>
      {({ disabled, label }) => <button disabled={disabled}>{label}</button>}
  </Story>
</Preview>;

@shilman
Copy link
Member

shilman commented Jan 23, 2020

Nothin' but ❤️ 's for this work @atanasster. Heroic! 🥇

@JamesIves
Copy link
Author

Amazing! This will be a game changer for us!

@atanasster
Copy link
Member

atanasster commented Jan 23, 2020

To whet your appetite:

csf stories with properties (aka knobs): https://github.com/atanasster/storybook/blob/properties-2/examples/official-storybook/stories/addon-controls/props-editors.stories.js

mdx stories with properties (aka knobs):
https://github.com/atanasster/storybook/blob/properties-2/examples/official-storybook/stories/addon-controls/propeditors.stories.mdx

It should make it into the 6.0 alphas by mid-february and we would love to get testing and feedback for it, especially you guys have been at it for some time already.

and a screenshot of the source code preview for this story:

export const textDefaultProp = ({ text }) => text;
textDefaultProp.story = {
  properties: {
    text: 'Hello',
  },
};

grab42

@dvnrsn
Copy link

dvnrsn commented Feb 14, 2020

These pages are not found ^ but I'm trying to figure out how to get source code from knobs right now. A major roadblock in the DX for us. Happy to test and provide feedback

@alexhhn
Copy link

alexhhn commented Mar 25, 2020

Is this supported in current latest release, or only in alpha version?

@snyderhaus
Copy link

Still no update on this? All those pages are not found, unfortunately.

@goldhand
Copy link
Contributor

goldhand commented Apr 2, 2020

Looks like the links @atanasster are to an experimental "addon-controls" addon that was removed. See the commit history in properties-2 branch.

The removed code is here: https://github.com/atanasster/storybook/tree/4a62e69ecb82fbfc19c65c834309a93e50de0fe7/examples/official-storybook/stories/addon-controls

I don't know what the plans are for this but it sounds nice :)

Side note: It would be amazing if the knobs / controls could be generated from prop types, similar to how the props table is.

@shilman
Copy link
Member

shilman commented Apr 2, 2020

@goldhand It's work in progress. More info here: https://docs.google.com/document/d/1Mhp1UFRCKCsN8pjlfPdz8ZdisgjNXeMXpXvGoALjxYM/edit?usp=sharing

@f1yn
Copy link

f1yn commented May 2, 2020

Contrarian here, I think this new prop injection is a terrible idea, namely since it basically obliterates the whole point of copy on source display components.

If a potential client's developer tried to copy and paste source code, they'd be under the impression that these props exist on our platform. For now I've been devising a workaround for this, but this feature might detract from the whole copy-paste formula some devs rely on for prototyping, especially for props that aren't easy digestible from a props table.

@shilman
Copy link
Member

shilman commented May 2, 2020

@flynnham you can still use no-arg functions in 6.0. in a future iteration of the source block we will show the live values in the code per this issue so you can copy and paste it.

Story:

const Basic = ({ label }) => <Button label={label} />

Raw display:

({ label }) => <Button label={label} />

Values display:

({ label }) => <Button label="actual label" />

@f1yn
Copy link

f1yn commented May 2, 2020

Because the storybook environment we're building atm, we've already been using a custom Source block in the documentation (which I've cleaned up to share). As a last resort to not confuse clients, I've opted for a very cheap static analysis version that's getting the job done. We're not using weird knob configurations so this is doing the job just fine, but for more advanced usages (like higher order functions and values dependant on closures), you'd need a full VM setup, or to setup this call within the same context as a call (which could be a callback hook).

Alternatively, you can detect variables also using static analysis and then add placeholders when evaluating (as shown below).

Edit: added Object serialization

import React, { useContext } from 'react';
import { DocsContext } from '@storybook/addon-docs/blocks';
import { Source } from '@storybook/components';

/**
 * Adapted from https://github.com/storybookjs/storybook/blob/master/addons/docs/src/blocks/Source.tsx
 */
const extractLines = (source, location) => {
	const { startBody: start, endBody: end } = location;
	const lines = source.split('\n');
	if (start.line === end.line) {
		return lines[start.line - 1].substring(start.col, end.col);
	}

	// NOTE: storysource locations are 1-based not 0-based!
	const startLine = lines[start.line - 1];
	const endLine = lines[end.line - 1];

	return [
		startLine.substring(start.col),
		...lines.slice(start.line, end.line - 1),
		endLine.substring(0, end.col),
	];
};

/**
 * Joins the original source for various
 * @param originalLines
 * @return {*}
 */
function applyKnobDefaults(originalLines) {
	const joinedSource = originalLines.join('\n');

	// Special cases for knobs for resolving index. Most knob calls use [1] as the defaultValue index
	const specialTypeToParamIndex = {
		select: 2,
		radios: 2,
		files: 2,
	};

	// safer and slightly eval for converting object into statement
	// see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval#Do_not_ever_use_eval!
	const evaluateObject = (obj) => Function('"use strict";return (' + obj + ')')();

	// Special sauce regular expression, will extract the block group representing a common knob helper
	// This also works for multiline knobs
	return joinedSource
		.replace(/(color|boolean|text|array|select|object|radios|files)\(((\s|\S)*?)\)/g, (fullMatch, knobType, baseRawParams) => {
			// Super hack: Evaluate the parameter list as a native object. Note this won't respect the closure of
			// the original source. That would require a full VM OR to evaluate the whole script in isolation
			try {
				// Anything that we can't resolve (that's not a function call), attempt to replace with dummy value
				const rawParams = baseRawParams.replace(/([a-zA-Z0-9()]+?),/g, 'null,');
				const parsedParams = evaluateObject(`[${rawParams}]`);
				// Determine which index to use
				const indexForExtraction = specialTypeToParamIndex[knobType] || 1;
				const result = parsedParams[indexForExtraction];

				if (result && typeof result === 'object') {
					// Adapted from https://stackoverflow.com/a/11233515
					return JSON.stringify(result).replace(/"([^"]+)":/g, ' $1: ');
				}

				return JSON.stringify(result);
			} catch (error) {
				console.error('Failed to automatically resolve knob value', error);
				return fullMatch;
			}
		})
		.split('\n');
}

export function Example({
	id: customId,
}) {
	const context = useContext(DocsContext);

	const data = customId ?
		context.storyStore.fromId(customId) :
		context;

	const { source: rawSource } = data.parameters.storySource;
	const location = data.parameters.storySource.locationsMap[data.id];

	// Extract the source code for the contextual (or provided) story
	let lines = applyKnobDefaults(extractLines(rawSource, location));

	return (
		<Source
			code={lines.join('\n')}
			language="jsx"
			format={false}
		/>
	)
}

It's not the prettiest solution, but just wanted to highlight that it is possible with static analysis too.

@shilman
Copy link
Member

shilman commented May 28, 2020

Hi gang, We’ve just released addon-controls in 6.0-beta!

Controls are portable, auto-generated knobs that are intended to replace addon-knobs long term.

Please upgrade and try them out today. Thanks for your help and support getting this stable for release!

@shilman
Copy link
Member

shilman commented Jun 1, 2020

For anybody who is interested in Controls but don't know where to start, I've created a quick & dirty step-by-step walkthrough to go from a fresh CRA project to a working demo. Check it out:

=> Storybook Controls w/ CRA & TypeScript

There are also some "knobs to controls" migration docs in the Controls README:

=> How do I migrate from addon-knobs?

@AnnyCaroline
Copy link

Loved the idea of use controls inside Docs page, but the code is still strange. From the Storybook "Controls w/ CRA & TypeScript" example:

image

@shilman
Copy link
Member

shilman commented Jun 6, 2020

@AnnyCaroline Yup. I'll probably implement the improved source rendering in 6.1 based on the controls work (NOT the knobs package, which will be deprecated at some point in the future), so I felt it was relevant to share the progress.

@shilman
Copy link
Member

shilman commented Jun 30, 2020

Great Caesar's ghost!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-beta.38 containing PR #11332 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

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

@shilman shilman closed this as completed Jun 30, 2020
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