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: tables with inline code with newlines #979

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

kellyjosephprice
Copy link
Collaborator

PR App Ref RM-10616

🧰 Changes

Fixes migrating tables with inline code with newlines.

Our previous check for converting markdown tables to JSX only scanned text nodes. That would skip inline code and inline html. Uh oh.

🧬 QA & Testing

Copy link
Contributor

@rafegoldberg rafegoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like I had a lot of Big Questions on this one, but tests seem sound and run well so…lgtm! 🤘

@@ -19,6 +19,8 @@ const alignToStyle = (align: 'left' | 'center' | 'right' | null) => {

const isTableCell = (node: Node) => ['tableHead', 'tableCell'].includes(node.type);

const isLiteral = (node: Node): node is Literal => 'value' in node;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, I know about as but what is the meaning of is in TS???

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the term off hand, but it means the function is restricting the type?

Comment on lines +44 to +45
visit(cell, isLiteral, (node: Literal) => {
if (node.value.match(/\n/)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a "literal" node anyways?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything with a text value. I think that's text, inlineCode, code, html, yaml

'h-0': 'Field',
'h-1': 'Description',
'0-0': 'orderby',
'0-1': '`{\n "field": "ID",\n "type": "ASC"\n }`',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen if they'd wrapped this in a code block instead of an inline? Would that "just work"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. RDMD should compile it back to magic block.

@kellyjosephprice kellyjosephprice merged commit 1de4f28 into next Sep 30, 2024
13 checks passed
@kellyjosephprice kellyjosephprice deleted the fix/migrating-tables-with-codes-with-newlines branch September 30, 2024 20:42
rafegoldberg pushed a commit that referenced this pull request Sep 30, 2024
## Version 7.6.3

### 🛠 Fixes & Updates

* dont render markdown in toc ([#980](#980)) ([c929bd0](c929bd0))
* tables with inline code with newlines ([#979](#979)) ([1de4f28](1de4f28))

<!--SKIP CI-->
@rafegoldberg
Copy link
Contributor

This PR was released!

🚀 Changes included in v7.6.3

@kellyjosephprice kellyjosephprice restored the fix/migrating-tables-with-codes-with-newlines branch October 7, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants