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

ignoreMutation and atom nodes #1448

Closed
YousefED opened this issue Jun 10, 2021 · 8 comments
Closed

ignoreMutation and atom nodes #1448

YousefED opened this issue Jun 10, 2021 · 8 comments
Labels
sponsor 💖 This issue or pull request was created by a Tiptap sponsor

Comments

@YousefED
Copy link
Contributor

I'm not (yet) an expert on prosemirror and how atom / leaf nodes work, but I noticed the following line in TipTap:

https://github.com/ueberdosis/tiptap/blob/main/packages/core/src/NodeView.ts#L186

// a leaf/atom node is like a black box for ProseMirror    
// and should be fully handled by the node view    
if (this.node.isLeaf) {      return true    }

Based on the comments, it seems unexpected for me that you only check for isLeaf, and not whether the node is an atom.

@github-actions github-actions bot added the sponsor 💖 This issue or pull request was created by a Tiptap sponsor label Jun 10, 2021
@philippkuehn
Copy link
Contributor

as far as I know isLeaf is always true for atom nodes.

@YousefED
Copy link
Contributor Author

From prosemirror docs:

isAtom: bool
True when this is an atom, i.e. when it does not have directly editable content. This is usually the same as isLeaf, but can be configured with the leaf property on a node's spec (typically when the node is displayed as an uneditable node view).

Though not super explicit, I'm fairly sure you can define a nodeview with content (which would make it not a leaf), but define it as atom (to indicate PM should not handle editing)

@YousefED
Copy link
Contributor Author

This confirms it's possible (@philippkuehn ):

image

@philippkuehn
Copy link
Contributor

Hmm you are right. But I’m still not sure about a use case for this configuration? 🤔

@YousefED
Copy link
Contributor Author

@philippkuehn this is a relevant example:

https://prosemirror.net/examples/codemirror/

Let's say you want to embed an editor (could also be a textarea) inside tiptap. You want to store the editor contents in the tiptap document, but want to handle interactions yourself (or in the above example, by codemirror. In that case the element should be an atom (it should be opaque to tiptap / prosemirror), but not a leaf (it should be capable of containing text contents)

@philippkuehn
Copy link
Contributor

Ah makes totally sense! Fixed it.

@YousefED
Copy link
Contributor Author

Ah makes totally sense! Fixed it.

Cool. Just to be clear; I based my feedback on your original comment in the code, which indicated it seemed erroneous. I don't know whether there are any real-life bugs that are prevented by this, for this I don't know enough about how ignoreMutation works. Although I'm working on a project which has a scenario similar to the codemirror-in-prosemirror example, I did not (yet) run into an issue that was caused exactly by this.

Do you know what ignoreMutation actually prevents from happening on the PM side when it returns true?

@philippkuehn
Copy link
Contributor

ignoreMutation is a handler to decide if ProseMirror should re-render the node on any DOM mutations by itself. Like changes on node attributes or node content. For node views you probably don’t want to re-render the node for these kind of changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sponsor 💖 This issue or pull request was created by a Tiptap sponsor
Projects
None yet
Development

No branches or pull requests

2 participants