fix: Allow for fully whitespace-sensitive nodes with whitespace: pre
#6962
Conversation
|
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| cy.get('.tiptap').then(([{ editor }]) => { | ||
| editor.commands.insertContent('<pre><div>foo</div>\n<code>foo\nbar</code></pre>') | ||
| cy.get('.tiptap').should('contain.html', '<pre><div>foo</div>\n<code>foo\nbar</code></pre>') | ||
| }) |
There was a problem hiding this comment.
This is a specific issue for our project, in which we have nodes like:
<my-custom-node {whitespace: `pre`}>
<subnode>Item 1</subnode>
<subnode>Item 2</subnode>
</my-custom-node>
The whitespace is getting collapsed, so we end up with a document like:
<my-custom-node><subnode>Item 1</subnode><subnode>Item 2</subnode>
</my-custom-node>
and a tree of
my-custom-node
subnodesubnode
We actually want a tree like:
my-custom-node
subnode-whitespacesubnodesubnode-whitespacesubnode
reteps
changed the title
whitespace: pre
|
I'm unsure what to do with your PR but I can look into the bug you're describing. Should we rather re-open #4792 instead of going forward with issue-related discussions on a PR? |
|
That would be good. I opened this as a PR because I am willing to open a PR fixing this, if you suggest which of the two strategies you would prefer @bdbch ! |
|
bump @bdbch |
|
bump @bdbch . Let me know if this is something I can contribute to. |
|
cc @arnaugomez |
reteps
changed the title
whitespace: prewhitespace: pre
bdbch
changed the title
whitespace: prewhitespace: pre
|
Sorry, I'm currently sick and can't work actively on Tiptap or react as fast as usually. I reopened the issue. Lets discuss the problem there. In general I'm having trouble with understanding your requirements. I guess you're using the default code block extension when you try to parse |
|
I'm also closing this PR because it's not really making sense (description is saying it's fixing something while it's just adding a test and is more a bug-report). I'd invite you to maybe explain your use case a little bit more in #4792. |
|
Thank you, I did that!! |
2 participants
Changes Overview
This PR serves as an issue report for #4792. I wanted to discuss implementation strategy before I implement this. Currently, whitespace is trimmed before the document is parsed by ProseMirror. This means that the
whitespace: preoption isn't correctly respected. You can see the failing test attached to this PR.I noticed this bug while implementing a custom node intended to function like a raw HTML node. The
removeWhitespaces(html)call insideelementFromStringprevents access to whitespace.Implementation Approach
There are a couple ways to solve this.
Fix this correctly
The "best" way to solve this would be to invert the whitespace stripping process:
whitespace: preisn't set on any parent of that node.(Currently whitespace is stripped before the document is parsed, causing issues).
Escape Hatch
The alternative approach is to allow an "escape hatch" for the
useEditorhook earlier in the process. The full call chain there looks like:useEditor(callsEditorInstanceManager)packages/react/EditorInstanceManager.createEditor()Editor()Editor.createDoccreateDocumentcreateNodeFromContentelementFromStringThis entire call chain is private, which means that the only way to circumvent this behavior is to reimplement + extend from:
useEditor,EditorInstanceManagerandEditor.This is a lot of code duplication to do this. Any option to override this in
createNodeFromContentwould make this much easier, such as:CreateNodeFromContentOptions.removeWhitespaceNodes(passed toelementFromStringto strip whitespace or not)CreateNodeFromContentOptions.elementFromString(optional function override)Testing Done
[WIP] Will update
Verification Steps
[WIP] Will update
Additional Notes
N/A
Checklist
Related Issues
#4792
Let me know which approach is preferred (if any) and I will add it.