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

Escaped Pipe creates new table cell #6869

Closed
de-dan opened this issue May 23, 2019 · 7 comments · Fixed by #7694
Closed

Escaped Pipe creates new table cell #6869

de-dan opened this issue May 23, 2019 · 7 comments · Fixed by #7694

Comments

@de-dan
Copy link

de-dan commented May 23, 2019

Describe the bug
In Storybook-Addon-Notes: I'm using markdown-files with tables. Inside a table-cell I use escaped pipes. These pipes are creating new cells

To Reproduce
Steps to reproduce the behavior:

  1. Create a story
  2. Use addon notes
  3. Create a markdown file with this table:
| Attribute    | Type                  |
| ------------ | --------------------- |
| `position`   | `"left" \| "right"`   |

Which should render this:

Attribute Type
position "left" | "right"

But it actually renders like this:

Attribute Type
position "left" "right"

Expected behavior
Do not ignore escaped pipes. Do not create new table cells.

Screenshots
table

System:

  • OS: Windows7
  • Browser: Chrome
  • Addons: Notes
  • Version: 5.0.11
@shilman shilman added this to the 5.1.x milestone Jun 10, 2019
@stale stale bot added the inactive label Jul 1, 2019
@robaxelsen
Copy link
Contributor

Same issue here, unfortunately.

I wanted to try and find the root cause, and checked the markdown-to-jsx library for clues, but no similar issue reported there. Anyone have any insights into what part of the code or library is parsing this wrong?

@stale stale bot removed the inactive label Jul 3, 2019
@storybookjs storybookjs deleted a comment from stale bot Jul 4, 2019
@stale
Copy link

stale bot commented Jul 25, 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 Jul 25, 2019
@robaxelsen
Copy link
Contributor

I wanted to try and find the root cause, and checked the markdown-to-jsx library for clues, but no similar issue reported there. Anyone have any insights into what part of the code or library is parsing this wrong?

I could investigate this further, and maybe attempt a fix, but would be helpful with some pointers on where in the code to look - or if it's entirely a markdown-to-jsx issue. Pinging some recently active contributors @Gongreg @tmeasday @ndelangen @danielduan.

@stale stale bot removed the inactive label Jul 25, 2019
@Gongreg
Copy link
Member

Gongreg commented Jul 25, 2019

Hey,

  <Markdown>{textAfterFormatted}</Markdown>

this is all ondevice-notes addon is doing.
So I assume the issue is in markdown-to-jsx lib

@robaxelsen
Copy link
Contributor

@Gongreg Thanks! I will try to reproduce with plain markdown-to-jsx, and report any findings to their project. Propose to keep this open for now, for visibility.

@robaxelsen
Copy link
Contributor

This has now been fixed in latest markdown-to-jsx release, 6.10.3. Huge thanks to @ariabuckles in the underlying simple-markdown for providing a fix and submitting it downstream as well.

Submitted a PR to upgrade the markdown-to-jsx version for notes addon.

robaxelsen added a commit to robaxelsen/storybook that referenced this issue Aug 6, 2019
@shilman
Copy link
Member

shilman commented Aug 7, 2019

Crikey!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.2.0-beta.24 containing PR #7694 that references this issue. Upgrade today to try it out!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants