-
-
Notifications
You must be signed in to change notification settings - Fork 637
fix: External HTML parse/export cases #1991
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Why use a CSS variable? Can't BlockNote use the data-text-color attribute, and everything else can just get the raw color that is set?
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.
Fixed this in #1998
tests/src/unit/core/formatConversion/parse/__snapshots__/html/orderedListStart.json
Show resolved
Hide resolved
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
nperez0111
left a comment
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.
Good add.
| background: "#f4dfeb", | ||
| }, | ||
| }; | ||
| } as Record<string, { text: string; background: string }>; |
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.
Intentionally not an exact type?
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.
Yep correct I was running into an issue with that somewhere, IIRC it was parsed color strings from HTML were just regular strings and the default colors keys were a union of string literals, so there was a mismatch.
* Added plugin for collab schema migration * Moved color props to block content nodes * Fixed export for list items and updated unit test snapshots * Rename * Updated test snapshots * Implemented PR feedback * Fixed build
Implemented Changes
This PR adds the non-breaking changes from #1605, updating them to work with the default blocks refactor. These are:
Styles:
Props:
startprop parsing for ordered list items.Tests:
Missing Changes
There are still changes that are relevant for this PR that are either breaking or cannot be implemented atm:
addNodeViewandrender. But we can't do this for styles asaddMarkViewis not supported in TipTap v2, so we'd need to upgrade to v3. Also, if we wanted to use the custom style API instead to create the text & background color styles, this is also a problem astoExternalHTMLis not in the API. Adding this would also requireaddMarkView. There is a hack in the React package that we use to add a mark view, but this seems to have its own issues (see Text disapears when composing #1849).blockContainernode, meaning it's responsible for parsing the attributes from HTML. However, when parsing external HTML, we never actually parse anything as ablockContainer, so the text/background color attributes can never be parsed either. To fix this, we'd need to move the attributes to be on content nodes (paragraph,heading, etc) like text alignment currently is. The issue with this is that it changes the underlying BlockNote schema, making this a breaking change for any apps that store the TipTap/PM state underlying BlockNote (notably YDocs for collaboration).Next steps
toExternalHTMLto the custom styles API, and redefine text/background color using the custom styles API directly rather than porting TipTap marks.blockContainernodes and fix parsing them from inline styles in external HTML, without it being a breaking change.createBlockSpecFromTiptapNode, handling exporting/parsing to/from inline styles must be done differently to the other default blocks. I think this is doable, but haven't looked super far into this yet.