-
-
Notifications
You must be signed in to change notification settings - Fork 557
refactor: support markviews #1437
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 Git ↗︎
|
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.
Other than the Markview impl question, this looks good
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.
Hm, this is subtly incompatible with the implementation that I wrote for Tiptap V3: https://github.com/ueberdosis/tiptap/blob/next/packages/react/src/ReactMarkViewRenderer.tsx
May be worth reviewing the differences and seeing if we should upstream some of this or if we should follow the one I wrote for V3.
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.
any specific parts that seem incompatible? We don't expose markviews directly (only indirectly via React custom Styles), so I wonder if it's an issue. Of course, I could see if I can swap out the current implementation for the one you linked to
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.
I looked into it more, specifically some of the CoreMarkViewUserOptions (as
and contentAs
), but it does not look like it is even used here so it should be fine.
We can deal with this later when we either want to expose this integration or migrate to the Tiptap V3 implementation of it. So, let's put it off until later
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.
We can move forward with this, just can't expose it on a public API without double-checking things
This PR adds support for prosemirror MarkViews, and uses them for BlockNote "styles" when defined with React (i.e.: https://www.blocknotejs.org/docs/custom-schemas/custom-styles#style-implementation-reactcustomstyleimplementation)
Previously we had to create a separate React root to make React custom styles work (see ReactRenderUtil), thanks to markviews this can now be cleaned up