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

Using raw HTML in MDX files strips classes #8504

Closed
fraincs opened this issue Oct 21, 2019 · 14 comments
Closed

Using raw HTML in MDX files strips classes #8504

fraincs opened this issue Oct 21, 2019 · 14 comments
Assignees
Milestone

Comments

@fraincs
Copy link

fraincs commented Oct 21, 2019

Describe the bug
When using mdx as a source of doc, not wrapping the html in a story component strips all classes.

To Reproduce
Steps to reproduce the behavior:

  1. Create a mdx file
  2. Paste code from code snippet
  3. No classes are applied to the DOM elements

Expected behavior
Using raw HTML in mdx would behave as straight HTML

Screenshots

image

Code snippets

import { Meta } from "@storybook/addon-docs/blocks";

<Meta title="Materials|Colours" />

<div>
    <div class="pa4 bg-desktop-500">
        <div className="flex justify-between">
            <span>500</span>
            <span>#F47321</span>
        </div>
    </div>
    <div className="pa4 bg-sunray-500">
        <div className="flex justify-between">
            <span>500</span>
            <span>#FCB736</span>
        </div>
    </div>
</div>

Note that nor class or ClassName are working

System:
System:
OS: macOS Mojave 10.14.6
CPU: (4) x64 Intel(R) Core(TM) i5-7360U CPU @ 2.30GHz
Binaries:
Node: 12.7.0 - ~/.nvm/versions/node/v12.7.0/bin/node
Yarn: 1.19.1 - ~/Repos/sg-orbit/node_modules/.bin/yarn
npm: 6.10.0 - ~/.nvm/versions/node/v12.7.0/bin/npm
Browsers:
Chrome: 77.0.3865.120
Firefox: 69.0.1
Safari: 13.0.2
npmPackages:
@storybook/addon-actions: 5.2.4 => 5.2.4
@storybook/addon-console: 1.2.1 => 1.2.1
@storybook/addon-docs: 5.2.4 => 5.2.4
@storybook/addon-knobs: 5.2.4 => 5.2.4

@shilman
Copy link
Member

shilman commented Oct 21, 2019

@fraincs This was introduced in #7771, which tries to isolate "document theming" vs. "user story theming", and is a little tricky.

  • Right now AFAIK there is no way to distinguish between an h1 generated by markdown (e.g. # Heading) and a user-added h1 (e.g. <h1>Heading</h1>). We clearly want to style the former, and may or may not want to style the latter. Thoughts?
  • If we could preserve the user's classes, and ADD classes that match the document styling, would that be sufficient?

@shilman shilman added this to the 5.3.0 milestone Oct 21, 2019
@fraincs
Copy link
Author

fraincs commented Oct 22, 2019

  • Right now AFAIK there is no way to distinguish between an h1 generated by markdown (e.g. # Heading) and a user-added h1 (e.g. <h1>Heading</h1>). We clearly want to style the former, and may or may not want to style the latter. Thoughts?

Exactly, I currently documenting how H1s are affected by some atomic CSS classes, and wouldn't want Storybook styling to bleed. e.g. <h1 className="f6"... should only have f6 styling applied to it.

  • If we could preserve the user's classes, and ADD classes that match the document styling, would that be sufficient?

Yes as long as user's classes are applied after the classes that match the document styling

@vidarc
Copy link

vidarc commented Nov 6, 2019

Don't know if this will help, but with version 5.3.0-alpha.38, if I import a Component into my MDX file, that component's classes do not get stripped out, but also the storybook classes do not get added.

@ndelangen
Copy link
Member

@fraincs could you open a PR with a reproduction perhaps? That would make it easier to start work.

@fraincs
Copy link
Author

fraincs commented Nov 20, 2019

@ndelangen see #8897

@lostPixels
Copy link

I just encountered this issue when trying to add our company logo to the introduction of our styleguide. Not a huge problem, but it would be nice to be able to selective style elements in our MDX file with a CSS file.

@atanasster
Copy link
Member

atanasster commented Dec 16, 2019

@fraincs & everyone else , I just tried the reproduction sample on the latest 5.3 beta and can not reproduce either issue:
The class names are not stripped and also the sb-docs classes are kept in there.

Can you guys please try to upgrade to the latest 5.3 beta and post here if you can still reproduce the issue?

grab30

@shilman
Copy link
Member

shilman commented Dec 17, 2019

Thanks @atanasster. Gonna close this!!! Please scream if it's still broken for you 😱

@shilman shilman closed this as completed Dec 17, 2019
@patricklafrance
Copy link
Member

patricklafrance commented Dec 18, 2019

@atanasster @shilman

It's not strip anymore, but CSS classes added by MDX have precedence over the custom classes added by the user.

You can see it in action in the following screenshot. Same code rendered twiced. The first box is in a story, the second is not.

It would be nice if we could import a block like <ResetStyles> and paste inside code that shouldn't be affected by default MDX styles.

What do you think?

image

@atanasster
Copy link
Member

@patricklafrance - probably this should be in a new , separate issue and more of a feature request. What was initially reported works fine now.

As of now, your use case has a workaround too with !important modifier, right?

@patricklafrance
Copy link
Member

@atanasster you're right. Sorry I re-opened the issue since in the current state, even if the classes are not strip, it still doesn't do it for us. I will create a separate issue.

For !important, yes and no...

Yes it would work.

No since our docs use the same atomic CSS codebase that we use for our components and our apps. Adding !important to the selectors might cause a lot of regressions.

We could write new selectors for the docs but I would prefer not doing it.

@patricklafrance
Copy link
Member

#9190

ndelangen added a commit that referenced this issue Feb 12, 2020
FIX #8504 - HTML elements get their classes dropped in MDX
@shilman
Copy link
Member

shilman commented Feb 13, 2020

Jeepers creepers!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-alpha.11 containing PR #8897 that references this issue. Upgrade today to try it out!

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

shilman pushed a commit that referenced this issue Feb 25, 2020
FIX #8504 - HTML elements get their classes dropped in MDX
@shilman
Copy link
Member

shilman commented Feb 25, 2020

Gadzooks!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.14 containing PR #8897 that references this issue. Upgrade today to try it out!

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