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

improve UX on bubble menu when selecting text #1493

Closed
michelson opened this issue Jun 19, 2021 · 10 comments · Fixed by #3385
Closed

improve UX on bubble menu when selecting text #1493

michelson opened this issue Jun 19, 2021 · 10 comments · Fixed by #3385
Assignees
Labels
Info: Stale The issue or pullrequest has not been updated in a while and might be stale sponsor 💖 This issue or pull request was created by a Tiptap sponsor Type: Bug The issue or pullrequest is related to a bug

Comments

@michelson
Copy link

michelson commented Jun 19, 2021

When users selects text on multiple lines the bubble menu gets repositioned on every selection change, this is not optimal because the bubble menu will jump because the selection will overlap the menu and jump around as the selection will be toggled (focus/blur). to illustrate:

Screen.Cast.2021-05-23.at.4.16.17.PM.mp4

In my implementation I've changed the behavior on how the mouseup should be handled, this will only reposition the menu when the user text selection stops

Screen.Cast.2021-05-23.at.4.13.44.PM.mp4

Please check this behavior on medium or discord editor's bubble menu to check how it should work, also check my implementation https://www.dante-editor.dev/ 😄

the code changes of menu bubble on dante3 can be seen at https://github.com/michelson/Dante/blob/master/packages/dante3/src/popovers/bubble-menu/bubble-menu-extension/bubble-menu-plugin.ts

@michelson michelson added Type: Bug The issue or pullrequest is related to a bug v2 labels Jun 19, 2021
@github-actions github-actions bot added the sponsor 💖 This issue or pull request was created by a Tiptap sponsor label Jun 19, 2021
@michelson
Copy link
Author

this is somewhat related with #1313

@robertvanhoesel
Copy link
Contributor

I agree with this, had to work around it as well. See #1522

Happy to implement this and make this my first PR if you see the merit. @hanspagel

@robertvanhoesel
Copy link
Contributor

#1556

@hanspagel hanspagel removed the v2 label Sep 28, 2021
@charesbast
Copy link

Hello everyone,

Just found about that bug: the cause is that when the cursor comes inside the bubble, the focus modify text selection. You can try to set the property: interactive: false to tippy options of BubbleMenu component, the selection works well.
I've tried to update the interactive option with a local state but tippy does not update. Not sure if it's intended or if it's a bug. (I think it's a bug).

The best solution for me would be to always display the bubble menu, but make it interactive (with pointer-events) only when mouseup event if fired (and text selection finished).

What do you think ? :)

@stale
Copy link

stale bot commented Jul 6, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 6, 2022
@michelson
Copy link
Author

it seems this is still an issue.

@stale stale bot removed the stale label Jul 6, 2022
@sereneinserenade
Copy link
Contributor

I've fixed it by adding a debounce to BubbleMenuPlugin.

bubble-menu-plugin.ts
import { debounce } from "lodash";
import {
  Editor,
  isNodeSelection,
  isTextSelection,
  posToDOMRect,
} from "@tiptap/core";
import { EditorState, Plugin, PluginKey } from "prosemirror-state";
import { EditorView } from "prosemirror-view";
import tippy, { Instance, Props } from "tippy.js";

export interface BubbleMenuPluginProps {
  pluginKey: PluginKey | string;
  editor: Editor;
  element: HTMLElement;
  tippyOptions?: Partial<Props>;
  shouldShow?:
    | ((props: {
        editor: Editor;
        view: EditorView;
        state: EditorState;
        oldState?: EditorState;
        from: number;
        to: number;
      }) => boolean)
    | null;
}

export type BubbleMenuViewProps = BubbleMenuPluginProps & {
  view: EditorView;
};

export class BubbleMenuView {
  public editor: Editor;

  public element: HTMLElement;

  public view: EditorView;

  public preventHide = false;

  public tippy: Instance | undefined;

  public tippyOptions?: Partial<Props>;

  public shouldShow: Exclude<BubbleMenuPluginProps["shouldShow"], null> = ({
    view,
    state,
    from,
    to,
  }) => {
    const { doc, selection } = state;
    const { empty } = selection;

    // Sometime check for `empty` is not enough.
    // Doubleclick an empty paragraph returns a node size of 2.
    // So we check also for an empty text size.
    const isEmptyTextBlock =
      !doc.textBetween(from, to).length && isTextSelection(state.selection);

    if (!view.hasFocus() || empty || isEmptyTextBlock) return false;

    return true;
  };

  constructor({
    editor,
    element,
    view,
    tippyOptions = {},
    shouldShow,
  }: BubbleMenuViewProps) {
    this.editor = editor;
    this.element = element;
    this.view = view;

    if (shouldShow) {
      this.shouldShow = shouldShow;
    }

    this.element.addEventListener("mousedown", this.mousedownHandler, {
      capture: true,
    });
    this.view.dom.addEventListener("dragstart", this.dragstartHandler);
    this.editor.on("focus", this.focusHandler);
    this.editor.on("blur", this.blurHandler);
    this.tippyOptions = tippyOptions;
    // Detaches menu content from its current parent
    this.element.remove();
    this.element.style.visibility = "visible";
  }

  mousedownHandler = () => {
    this.preventHide = true;
  };

  dragstartHandler = () => {
    this.hide();
  };

  focusHandler = () => {
    // we use `setTimeout` to make sure `selection` is already updated
    setTimeout(() => this.update(this.editor.view));
  };

  blurHandler = ({ event }: { event: FocusEvent }) => {
    if (this.preventHide) {
      this.preventHide = false;

      return;
    }

    if (
      event?.relatedTarget &&
      this.element.parentNode?.contains(event.relatedTarget as Node)
    ) {
      return;
    }

    this.hide();
  };

  updateHandler = debounce((view: EditorView, oldState?: EditorState) => {
    const { state, composing } = view;
    const { doc, selection } = state;
    const isSame =
      oldState && oldState.doc.eq(doc) && oldState.selection.eq(selection);

    if (composing || isSame) return;

    this.createTooltip();

    // support for CellSelections
    const { ranges } = selection;
    const from = Math.min(...ranges.map((range) => range.$from.pos));
    const to = Math.max(...ranges.map((range) => range.$to.pos));

    const shouldShow = this.shouldShow?.({
      editor: this.editor,
      view,
      state,
      oldState,
      from,
      to,
    });

    if (!shouldShow) {
      this.hide();

      return;
    }

    this.tippy?.setProps({
      getReferenceClientRect:
        this.tippyOptions?.getReferenceClientRect ||
        (() => {
          if (isNodeSelection(state.selection)) {
            const node = view.nodeDOM(from) as HTMLElement;

            if (node) return node.getBoundingClientRect();
          }

          return posToDOMRect(view, from, to);
        }),
    });

    this.show();
  }, 250);

  createTooltip() {
    const { element: editorElement } = this.editor.options;
    const editorIsAttached = !!editorElement.parentElement;

    if (this.tippy || !editorIsAttached) {
      return;
    }

    this.tippy = tippy(editorElement, {
      duration: 0,
      getReferenceClientRect: null,
      content: this.element,
      interactive: true,
      trigger: "manual",
      placement: "top",
      hideOnClick: "toggle",
      ...this.tippyOptions,
    });

    // maybe we have to hide tippy on its own blur event as well
    if (this.tippy.popper.firstChild) {
      (this.tippy.popper.firstChild as HTMLElement).addEventListener(
        "blur",
        (event) => {
          this.blurHandler({ event });
        }
      );
    }
  }

  update(view: EditorView, oldState?: EditorState) {
    this.updateHandler(view, oldState);
  }

  show() {
    this.tippy?.show();
  }

  hide() {
    this.tippy?.hide();
  }

  destroy() {
    this.tippy?.destroy();
    this.element.removeEventListener("mousedown", this.mousedownHandler, {
      capture: true,
    });
    this.view.dom.removeEventListener("dragstart", this.dragstartHandler);
    this.editor.off("focus", this.focusHandler);
    this.editor.off("blur", this.blurHandler);
  }
}

export const BubbleMenuPlugin = (options: BubbleMenuPluginProps) => {
  return new Plugin({
    key:
      typeof options.pluginKey === "string"
        ? new PluginKey(options.pluginKey)
        : options.pluginKey,
    view: (view) => new BubbleMenuView({ view, ...options }),
  });
};

This how it looks:

Screen.Recording.2022-07-11.at.20.29.25.mp4

@github-actions
Copy link
Contributor

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the Info: Stale The issue or pullrequest has not been updated in a while and might be stale label Oct 10, 2022
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 17, 2022
@bdbch bdbch reopened this Nov 4, 2022
@bdbch
Copy link
Member

bdbch commented Nov 4, 2022

Thanks @sereneinserenade for the idea. I created a pull request that implements a debounce on default (250ms) which is overwriteable (or deactivateable by setting the delay to 0).

@sereneinserenade
Copy link
Contributor

@bdbch Glad to know my solution could be of help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Info: Stale The issue or pullrequest has not been updated in a while and might be stale 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

Successfully merging a pull request may close this issue.

7 participants