-
-
Notifications
You must be signed in to change notification settings - Fork 492
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 for numbered list item when exporting to PDF, DOCX #1357
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@mfanselmo is attempting to deploy a commit to the TypeCell Team on Vercel. A member of the Team first needs to authorize it. |
bb4b001
to
9b67f69
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks. We'll fix the playwright tests, I think it's unrelated. Could you:
- also add support to the docx exporter?
- add it to the unit tests of the exporters? (add a case to the default test case and update the related snapshots)
9b67f69
to
7886b25
Compare
Hey! I spent quite a bit of time on this but ran into some tricky edge cases I couldn’t get to work perfectly. Let me break them down: For PDFs, numbering is straightforward since we can manually set the numbering. However, Word has a few quirks: Here’s an example of content that doesn’t render correctly in Word. Based on how instance works, I would expect this to work fine: I’ve updated the snapshots for both PDF and DOCX to test these cases. The PDF output works perfectly, but DOCX still has this one issue. @YousefED I will leave this one to this point for now, but if I find time in the future I will give it another go |
|
||
for (const b of blocks) { | ||
if (b.type === "numberedListItem") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is a bit messy and could definitely be improved, but its the best I could come up with to ensure a unique instance id is given to each numbered list. In theory this id should guarantee no weird continuations of non adjacent lists happen, but for some reason they still are. I would not merge this as is
This PR #1326 introduced the ability to start numbered lists from indices other than 1. I noticed on the demo to export to pdf/docx this was not taked into account.
I was not able to run playwright locally and update snapshots if that is needed, can you give me guidance here or do it?