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: saving tables #942

Merged
merged 6 commits into from
Jul 19, 2024
Merged

fix: saving tables #942

merged 6 commits into from
Jul 19, 2024

Conversation

kellyjosephprice
Copy link
Collaborator

@kellyjosephprice kellyjosephprice commented Jul 17, 2024

PR App RM-9793

🧰 Changes

Fixes saving 'complex' tables.

In a prior PR, I added a new node type tableau to represent tables with block content. This mostly worked for the editor, but not when doing mdx(mdast(doc)). This PR adds a transformer, so the a tableau is converted to a JSX table just like regular table's.

This also adds the check so, if a tableau no longer has block content, it's saved as markdown!

🧬 QA & Testing

Hard to test outside the main app.

@kellyjosephprice kellyjosephprice changed the base branch from next to beta July 17, 2024 20:44
@kellyjosephprice kellyjosephprice marked this pull request as ready for review July 18, 2024 17:43
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.

Not sure I fully understand the content logic, but generally makes sense to me so I'm good with this!

@@ -35,7 +40,10 @@ const visitor = (table: Table, index: number, parent: Parents) => {
};

visit(table, 'tableCell', tableCellVisitor);
if (!hasFlowContent) return;
if (!hasFlowContent) {
table.type = 'table';
Copy link
Contributor

Choose a reason for hiding this comment

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

Ohhh is this where the tableau error was coming from?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea, I added a custom table component, silly-ly named tableau, to handle block content. I believe I ran into some weirdness the last time I tried to override a built in type.

And yea, I forgot to add a compiler or transform it or anything.

@kellyjosephprice
Copy link
Collaborator Author

kellyjosephprice commented Jul 19, 2024

Not sure I fully understand the content logic, but generally makes sense to me so I'm good with this!

Well, it's mostly just looking for any block content in a table cell. Except for the case when the editor likes to wrap the table cell content in a paragraph. So it tries to ignore that paragraph.

@kellyjosephprice kellyjosephprice merged commit 50c392e into beta Jul 19, 2024
8 checks passed
@kellyjosephprice kellyjosephprice deleted the fix/saving-tables branch July 19, 2024 00:56
rafegoldberg pushed a commit that referenced this pull request Jul 19, 2024
## Version 6.75.0-beta.76

### 🛠 Fixes & Updates

* saving tables ([#942](#942)) ([50c392e](50c392e))

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

This PR was released!

🚀 Changes included in v6.75.0-beta.76

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

Successfully merging this pull request may close these issues.

2 participants