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

FIX #8504 - HTML elements get their classes dropped in MDX #8897

Merged
merged 5 commits into from Feb 12, 2020

Conversation

@fraincs
Copy link

@fraincs fraincs commented Nov 20, 2019

Issue: #8504

What I did

Added an example story where classes are stripped by Storybook - in cra-ts-kitchen-sink > classes stripped

How to test

  • Is this testable with Jest or Chromatic screenshots? No
  • Does this need a new example in the kitchen sink apps? No
  • Does this need an update to the documentation? No

If your answer is yes to any of these, please make sure to include it in your PR.

@vercel
Copy link

@vercel vercel bot commented Nov 20, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/storybook/monorepo/p5s7ahw22
🌍 Preview: https://monorepo-git-fork-fraincs-8504.storybook.now.sh

@fraincs
Copy link
Author

@fraincs fraincs commented Nov 20, 2019

See issue #8504

@ndelangen
Copy link
Member

@ndelangen ndelangen commented Nov 21, 2019

So this is a reproduction that needs a fix?

@fraincs
Copy link
Author

@fraincs fraincs commented Nov 21, 2019

So this is a reproduction that needs a fix?

This is a reproduction of the issue #8504 where straight HTML classes are stripped.

@stale stale bot added the inactive label Dec 12, 2019
@shilman shilman added this to the 6.0.0 milestone Dec 20, 2019
@stale stale bot removed the inactive label Dec 20, 2019
@stale stale bot added the inactive label Jan 10, 2020
@ndelangen
Copy link
Member

@ndelangen ndelangen commented Jan 11, 2020

@shilman do you think we should address this before 5.3 release?

@stale stale bot removed the inactive label Jan 11, 2020
@storybookjs storybookjs deleted a comment from stale bot Jan 21, 2020
@ndelangen ndelangen self-assigned this Jan 21, 2020
@ndelangen
Copy link
Member

@ndelangen ndelangen commented Jan 28, 2020

I'm adding this to my todo list

@storybookjs storybookjs deleted a comment from stale bot Jan 28, 2020
ndelangen added 3 commits Jan 29, 2020
…iterating over all exports

This makes the typings MUCH more strict (and thus correct)

ADD a props mapper function that changes `class` to `className` and merges and namespaces correctly

REMOVE the complex iterating over all exports in html.tsx
@ndelangen ndelangen changed the title #8504 HTML elements get their classes dropped because they are considered JSX elements in MDX Jan 29, 2020
@ndelangen ndelangen changed the title HTML elements get their classes dropped because they are considered JSX elements in MDX FIX #8504 - HTML elements get their classes dropped in MDX Jan 29, 2020
@ndelangen ndelangen added the mdx label Jan 29, 2020
@ndelangen ndelangen modified the milestones: 6.0.0, 5.3.x Jan 29, 2020
@ndelangen
Copy link
Member

@ndelangen ndelangen commented Jan 29, 2020

I think this is fixed now, and TS-typings for the html elements should be much improved as well!

@ndelangen ndelangen requested a review from shilman Jan 29, 2020
@patricklafrance
Copy link
Contributor

@patricklafrance patricklafrance commented Jan 31, 2020

@ndelangen pretty sure I fixed this when doing the MDX deep linking feature.

f0d1ee8#diff-0035b2eccdfae670442b799fe81b4ffe

@ndelangen
Copy link
Member

@ndelangen ndelangen commented Feb 3, 2020

Hey @patricklafrance I think the components are going to be much more strict after this PR.

storybookjs/vs-code-plugin#15

This just makes sure the className prop is accepted, but my change should make it so The <A> component actually only accepts props valid to a a tag PLUS the added class prop, that gets mapped to className (that's actually the fix for the issue).

@ndelangen ndelangen merged commit 3d6b1fc into storybookjs:next Feb 12, 2020
24 of 25 checks passed
24 of 25 checks passed
@github-actions
Automention Automention
Details
@github-actions
Danger JS
Details
@netlify
Header rules No header rules processed
Details
@netlify
Pages changed 132 new files uploaded
Details
@netlify
Redirect rules No redirect rules processed
Details
@packtracker
packtracker/images No images assets found.
Details
@packtracker
packtracker/javascript 8.55 MB — ▼ 0.03%
Details
DeepScan 0 new and 0 fixed issues
Details
@netlify
Mixed content No mixed content detected
Details
ci/chromatic 392 stories unchanged.
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: chromatic Your tests passed on CircleCI!
Details
ci/circleci: coverage Your tests passed on CircleCI!
Details
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: e2e Your tests passed on CircleCI!
Details
ci/circleci: examples Your tests passed on CircleCI!
Details
ci/circleci: frontpage Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: packtracker Your tests passed on CircleCI!
Details
ci/circleci: smoke-tests Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@circleci-checks
deploy Workflow: deploy
Details
@netlify
deploy/netlify Deploy preview ready!
Details
@circleci-checks
test Workflow: test
Details
@shilman shilman added the picked label Feb 25, 2020
shilman added a commit that referenced this pull request Feb 25, 2020
FIX #8504 - HTML elements get their classes dropped in MDX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants