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-backgrounds MDX support #14322

Open
shilman opened this issue Mar 23, 2021 · 11 comments
Open

Addon-backgrounds MDX support #14322

shilman opened this issue Mar 23, 2021 · 11 comments

Comments

@shilman
Copy link
Member

shilman commented Mar 23, 2021

Hi @yannbf ,

Your PR fixes the problem for DocsPage generated documentation, but not for manually written MDX stories in the style described here.

The code in the PR tries to set styles on the CSS selector here

This works for DocsPage since it generates Anchor components for every story.
However this pattern is not documented in the MDX guide above, and actually seems to conflict with the behaviour of the Meta tag here, which generates a single Anchor component at the top of the page for the first Story encountered (which is a bit strange tbh).

Would it be possible to make the background selection work on any story regardless of whether they are contained in an anchor?

Originally posted by @angryzor in #7978 (comment)

@yannbf
Copy link
Member

yannbf commented Mar 24, 2021

@patricklafrance we spoke about this back then. Did you have any ideas on how to best approach this? The addon must know what target to set a custom background for.

@havercake
Copy link

havercake commented Mar 26, 2021

Hi everyone, for Stories that are wrapped in a Canvas element there is a an easy target class of:

sbdocs sbdocs-preview

You could set the background on that. (Note that this is div that the background colour is being set on by default already)

@havercake
Copy link

havercake commented Mar 26, 2021

As @angryzor pointed out in their investigation (thanks for that), the change in the PR here.

If I understand correctly this line:

context.viewMode === 'docs' ? '#anchor--${context.id} .docs-story' : '.sb-show-main';

Could be changed to:

context.viewMode === 'docs' ? '.sbdocs-preview .docs-story' : '.sb-show-main';

This should work for both DocsPage generated stories and custom MDX stories (as long as they are inside a canvas)

@yannbf
Copy link
Member

yannbf commented Mar 26, 2021

Hey @havercake thanks for your insights! The problem with that approach is that we lose the specificity of the story. That will indeed make the backgrounds work, but it will apply the same background to every story, even if each story has a backgrounds override.

Essentially this can only be solved if we change the structure of a docs block. I will see what I can do about it!

@yannbf
Copy link
Member

yannbf commented Mar 26, 2021

Context

Hey @shilman, if all stories from docs used DocsStory instead of Story block, the addon would work just fine (as well as any other addon that needs specific story targeting). The reason is because the addons need a wrapper containing an identifier for that story, and when users write stories like this, it won't have the id in the wrapper:

<Canvas>
  <Story id="some-story"/>
</Canvas>

whereas the automatically generated docs contain the following structure:

<Anchor storyId={id}> // <-- Necessary piece!
  {subheading && <Subheading>{subheading}</Subheading>}
  {description && <Description markdown={description} />}
  <Canvas withToolbar={withToolbar}>
    <Story id={id} />
  </Canvas>
</Anchor>

Small POC

Here's a hack to make the addon work with custom MDX. In docs/src/blocks/Canvas.tsx:

image

And the result:
2021-03-26 at 18 31 17 - Lavender Wildebeest

This requires no change in the backgrounds addon, and it will work fine with automatically generated docs pages as well.. but I'm not happy with that. I just wanted to show a mechanism we need in order to make addons like backgrounds work in docs. Ideally both auto generated docs and mdx docs stories should have a similar structure, so that addons could work consistently across all supported formats.

As reference, here's the MDX used in that test

import {
  Story,
  Canvas,
  Meta,
} from '@storybook/addon-docs/blocks';
import BaseButton from '../components/BaseButton';

<Meta
  title="Addons/Backgrounds/mdx"
  id="addon-backgrounds-mdx"
  component={BaseButton}
  parameters={{ 
    backgrounds: {
      default: 'dark',
      values: [
        { name: 'white', value: '#ffffff' },
        { name: 'light', value: '#eeeeee' },
        { name: 'gray', value: '#cccccc' },
        { name: 'dark', value: '#222222' },
        { name: 'black', value: '#000000' },
      ]
    }
  }}
/>

# Story from id

This will use the story id as target for backgrounds 

<Canvas>
  <Story id="addons-backgrounds-canvas--overridden" />
</Canvas>

# Multiple stories from id inside canvas

This will use the first story id as target for backgrounds 

<Canvas>
  <Story id="addons-backgrounds-canvas--with-gradient" />
  <Story id="addons-backgrounds-canvas--story-1" />
</Canvas>

# Story defined in mdx

This will use the meta id as target for backgrounds, thus using default backgrounds from meta parameters

<Canvas>
  <Story name="simple story">
    <button>non story</button>
  </Story>
</Canvas>

@rbardini
Copy link
Contributor

Any updates on this? The different structures between regular and MDX stories in docs mode make it difficult to maintain certain addons that should work across formats—I've even noticed the anchor disappearing while switching between canvas and docs tabs 😕

@shilman shilman removed the P2 label Oct 18, 2022
@shilman shilman removed the todo label Jun 20, 2023
@lazenyuk-dmitry
Copy link

It is strange why this problem still does not have a solution.

@yannbf said everything correctly. The problem is the absence of a wrapper around <Canvas> when using it separately from <Storis> in MDX.

The way to wrap <Canvas> in <Anchor> is very inconvenient. Since you have to manually prescribe storyId every time. If <Anchor> was able to put his storyId himself)

<Anchor storyId={id}>
    <Canvas />
</Anchor>

I can only share a small temporary fix.

Add the next code to your preview.js:

decorators: [
		(story, context) => {
			const storyAnchor = `anchor--${context.id}`;
			const existAnchor = context.canvasElement.closest(`#${storyAnchor}`);
			const storyContainer = context.canvasElement.closest(".sbdocs");

			if (!existAnchor && storyContainer) {
				storyContainer.id = storyAnchor;
			}

			return {
				components: { story },
				template: `<story />`,
			}
		},
	],

I use Vue, in your case return will be different.

@yannbf
Copy link
Member

yannbf commented Oct 3, 2023

Can anyone confirm whether this is still applicable in Storybook 7? Both the bug and the workaround mentioned in this discussion? Thank you!

@ClementChaumel
Copy link

The bug is still a thing for me on 7.1.0 I haven't been able to get the workaround to work though.

@pimenovoleg
Copy link

Yes it still doesn't work with 7.6
Repo: https://github.com/radix-ng/primitives/tree/main/packages/primitives/.storybook

https://661288c24c1e1cf776635f22-kusmqcjumr.chromatic.com/?path=/docs/primitives-separator--docs

Do you have any plans to fix this? Or is it already fixed in version 8?

@pimenovoleg
Copy link

for Angular, you can work around this as:

decorators: [
        (Story, context) => {
            const storyAnchor = `anchor--${context.id}`;
            const existAnchor = context.canvasElement.closest(`#${storyAnchor}`);
            const storyContainer = context.canvasElement.closest('.sbdocs');

            if (!existAnchor && storyContainer) {
                storyContainer.id = storyAnchor;
            }

            return Story(context);
        }
    ],
    

Angular 17.x
Sb 7.6.x

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

7 participants