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

ReactNodeViewRenderer doesn't update correctly #1370

Closed
YousefED opened this issue May 24, 2021 · 2 comments
Closed

ReactNodeViewRenderer doesn't update correctly #1370

YousefED opened this issue May 24, 2021 · 2 comments
Labels
sponsor 💖 This issue or pull request was created by a Tiptap sponsor Type: Bug The issue or pullrequest is related to a bug

Comments

@YousefED
Copy link
Contributor

I think after the update() function of a react nodeview (

update(node: ProseMirrorNode, decorations: Decoration[]) {
), we should check whether ContentDOMElement still has the correct parent.

The issue I'm running into is a little hard to reproduce, but it comes down to this:

  • We have a custom React NodeView which renders Heading elements
  • When we change the Heading level from h1 to h2, our custom NodeView nicely rerenders, but the ContentDOM is still a child of the h1.
  • Because the ContentDOM doesn't have the correct parent, ProseMirrors setSelection logic fails

In my own project I hotfixed this with the following code:

export function CustomReactNodeViewRenderer(component: any, options?: any) {
  return (props: any) => {
    const renderer = ReactNodeViewRenderer(component, options)(props) as any;
    const oldUpdate = renderer.update;
    if (oldUpdate) {
      renderer.update = function () {
        let ret = oldUpdate.apply(this, arguments);
        this.contentDOM; // trigger fix
        return ret;
      };
    }
    return renderer;
  };
}
@YousefED YousefED added Type: Bug The issue or pullrequest is related to a bug v2 labels May 24, 2021
@github-actions github-actions bot added the sponsor 💖 This issue or pull request was created by a Tiptap sponsor label May 24, 2021
@YousefED YousefED reopened this May 24, 2021
@philippkuehn
Copy link
Contributor

Hey, hard to fix without a reproducing demo. I released a new version with the attached fix. Does that help?

@YousefED
Copy link
Contributor Author

Hey, hard to fix without a reproducing demo. I released a new version with the attached fix. Does that help?

Thanks! This doesn't fix it, but moving maybeMoveContentDom down does, i.e.:

this.node = node
this.decorations = decorations
this.renderer.updateProps({ node, decorations })
this.maybeMoveContentDOM()

andrewlu0 pushed a commit to trybaseplate/tiptap that referenced this issue Oct 20, 2023
andrewlu0 pushed a commit to trybaseplate/tiptap that referenced this issue Oct 20, 2023
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 Type: Bug The issue or pullrequest is related to a bug
Projects
None yet
Development

No branches or pull requests

2 participants