tiptap: Bubble menu doesn't position correctly if image has alignment

Description When using bubble menu with image nodes and if there is image alignment functionality present which is achieved with float, the bubble menu above image is not positioned correctly.

Steps to reproduce the bug Steps to reproduce the behavior:

  1. Go to https://codesandbox.io/s/tiptap-issue-template-forked-p5sny
  2. Click on image and see how the bubble menu is position in the middle of the editor
  3. Click on alignment button and change to left and see how the bubble menu is outside of editor
  4. Click on text nodes, bubble menu works as expetced

CodeSandbox I created a CodeSandbox to help you debug the issue: https://codesandbox.io/s/tiptap-issue-template-forked-p5sny

Expected behavior This should behave same as on text nodes.

Additional context I did some investigation on the bubble menu and how it calculates position. It looks like the float changes the start and the end positioning of the node.

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 3
  • Comments: 32 (21 by maintainers)

Most upvoted comments

Hello, I have a guess here:

  • it seems that the coords are retrieved from a selection, but in a non editable context, like image blocks the selection does not exist, but at least we have an active element, the coords in this context will contain width == 0px, so the tippy is relocated to the left at px 0 .
  • the solution to this is “detect” the non editable element and call the .getBoundingClientRect() on it, this value will be enough for tippy to display properly.

a proof of concept to fix this:

  • in bubble-menu I’ve changed the logic on update method to this:
    this.tippy.setProps({
      getReferenceClientRect: () => {
        const selectedElement = this.editor.view.dom.querySelector(".is-selected")
        if(selectedElement){
          return selectedElement.getBoundingClientRect()
        } else {
          return posToDOMRect(view, from, to)
        }
      },
    })

I’m not sure how can we get the selected element effectively , in my case I just get the element with is-selected class I hope this will give some light to fix the issue 😃

Screen Cast 2021-05-15 at 11 52 45 PM

I think there are two issues here.

First, we have to provide an option to overwrite the logic for checking if a bubble menu should be shown or hidden. Basically these lines are doing it in the bubble menu plugin: https://github.com/ueberdosis/tiptap/blob/2c48bc09eac3f3ed65e01d7b7efd37984534db91/packages/extension-bubble-menu/src/bubble-menu-plugin.ts#L97-L104

So I’m thinking of adding something like this (would be available as prop for vue/react components as well):

BubbleMenu.configure({
  // overwrite logic
  validate({ editor }) {
    const { empty, $anchor } = editor.selection
    
    // don’t hide for images
    if (empty || !$anchor.parent.textContent && !editor.isActive('image')) {
      return false
    }

    return true
  },
})

It’s still too much code for overwriting in my eyes. Maybe there is some better way of doing this. Also the name validate is bad I think. Anyone an idea for the naming?

Second, there is probably an issue with calculating coords for atom nodes. I have to check that.

@zcuric this is because the getBoundingClientRect() is pointing to the wrapper itself, in react-views the wrapper does not have the style. as I commented above the getBoundingClientRect() should be done on the first child and not on the wrapper. Also it would be a good idea to make this configurable

I created a pull request for this in #3385

The only way I’m getting consistent results is with these changes:

function coordsAtPos(view, pos, end = false) {
  const { offset, node }= view.domAtPos(pos); // view.docView.domFromPos(pos);
  let side = null;
  let rect = null;
  if (node.nodeType === 3) {
    const nodeValue = node.nodeValue || '';
    if (end && offset < nodeValue.length) {
      rect = singleRect(textRange(node, offset - 1, offset), -1);
      side = 'right';
    }
    else if (offset < nodeValue.length) {
      rect = singleRect(textRange(node, offset, offset + 1), -1);
      side = 'left';
    }
  }
  else if (node.firstChild) {
    if (offset < node.childNodes.length) {
      const child = node.childNodes[offset];
      rect = singleRect(child.nodeType === 3 ? textRange(child) : child, -1);
      side = 'left';
    }
    if ((!rect || rect.top === rect.bottom) && offset) {
      const child = node.childNodes[offset - 1];
      rect = singleRect(child.nodeType === 3 ? textRange(child) : child, 1);
      side = 'right';
    }
  }
  else {
    const element = node;
    rect = element.getBoundingClientRect();
    side = 'left';
  }
  if (rect && side) {
    const x = rect[side];
    return {
-     top: rect.top,
-     bottom: rect.bottom,
-      left: x,
-     right: x,
+     ...rect.toJSON(),
    };
  }
  return {
    top: 0,
    bottom: 0,
    left: 0,
    right: 0,
  };
}

function posToDOMRect(view, from, to) {
  const start = coordsAtPos(view, from)
+ if (to - from === 1) return { ...start, toJSON: () => start };
  const end = coordsAtPos(view, to, true)
  const top = Math.min(start.top, end.top)
  const bottom = Math.max(start.bottom, end.bottom)
  const left = Math.min(start.left, end.left)
  const right = Math.max(start.right, end.right)
  const width = right - left
  const height = bottom - top
  const x = left
  const y = top
  const data = {
    top,
    bottom,
    left,
    right,
    width,
    height,
    x,
    y,
  }

  return {
    ...data,
    toJSON: () => data,
  }
}

@zcuric I’ve not tested your code but you can read about the reason of our custom coordsAtPos method here:

https://github.com/ueberdosis/tiptap/issues/215 https://github.com/ProseMirror/prosemirror-view/pull/47

That’s a cool solution! I tried to set getReferenceClientRect on the bubble menu’s tippy-options to keep from editing the original file but it’s set explicitly as we can see from the link you shared.

If a solution that covers all use cases could be made in the bubble menu extension, that would be great - but if that’s not possible I wonder whether it could be re-jigged slightly so we can pass in our own getReferenceClientRect for each scenario.

I also get inconsistent coordinates from coordsAtPos even when the image aren’t floated when the editor alignment is anything other than left (where I believe it is returning the correct left pos by happy accident).

I cloned the repo and logged/traced it, and just got a bit lost seeing exactly what was happening in coordsAtPos (first time I’m digging into this repo so I’m lacking some context for what all the bits being passed in are). While debugging, side was coming out as left for one image node, and then right for another identically sized/positioned/etc. image.

It appears to be a rework of the ProseMirror function and although the tiptap one has been updated more recently time-wise, it might have been based off an older version of the original? I found a few bubble menu position issues from the ProseMirror code while hunting this.

So, no actual help there - sorry! 🙈