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

[Bug]: shouldShow broken in BubbleMenu extension from Vue 3 package #4870

Closed
1 of 2 tasks
mchestnut opened this issue Feb 9, 2024 · 3 comments · Fixed by #5252, #5343 or #5373
Closed
1 of 2 tasks

[Bug]: shouldShow broken in BubbleMenu extension from Vue 3 package #4870

mchestnut opened this issue Feb 9, 2024 · 3 comments · Fixed by #5252, #5343 or #5373
Labels
Category: Open Source The issue or pull reuqest is related to the open source packages of Tiptap. Type: Bug The issue or pullrequest is related to a bug

Comments

@mchestnut
Copy link

Which packages did you experience the bug in?

core, pm, starter-kit, vue-3

What Tiptap version are you using?

2.2.2

What’s the bug you are facing?

I'm noticing that the shouldShow callback is not functioning as expected in the Vue implementation of the Bubble Menu. When using editor.isActive as part of the condition, the value returned from isActive doesn't represent that state that I expect.

What browser are you using?

Chrome

Code example

https://stackblitz.com/edit/vitejs-vite-kgmrcn?file=src%2Fcomponents%2FTiptap.vue,package.json

What did you expect to happen?

Consider shouldShow: ({ editor }) => editor.isActive('bold'). I expect on first click into a bolded word that the bubble menu would become visible. However, it isn't and logging out the value of isActive in the method at that point returns false. But upon clicking again on any text (bold or not), the value then returns true. It's as if the editor state inside the callback is from before the click is registered.

Anything to add? (optional)

I do not see this same behavior in the vanilla JS implementation of Bubble Menu. I have not tested any other frameworks.

For comparison to the code example below, here is a working vanilla JS port of the same code.
https://stackblitz.com/edit/vitejs-vite-djsnpl?file=index.html,main.js&terminal=dev

Did you update your dependencies?

  • Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

  • Yes, I’m a sponsor. 💖
@mchestnut mchestnut added Category: Open Source The issue or pull reuqest is related to the open source packages of Tiptap. Type: Bug The issue or pullrequest is related to a bug labels Feb 9, 2024
@mchestnut
Copy link
Author

Finally found a workaround to this issue. The underlying problem seems to stem from the reactiveState property in the Editor class of the Vue implementation. For some reason this reactive state is not yet updated when the bubble menu plugin is calling the shouldShow callback. I thought at first it was because of the use of requestAnimationFrame found in the debounce function that reactiveState uses, but disabling the rAF calls didn't solve the issue. My guess now is it's something in the timing of Vue's reactivity system in general.

For now, I'm using the following workaround in my shouldShow callback.

const shouldShowBubbleMenu = ({ editor }) => {
    return isMarkActive(editor.view.state, 'link');
};

I've found importing isMarkActive directly from @tiptap/core allows me to directly pass in the view state, which seems to be up-to-date at the time shouldShow is called. Using editor.isActive returns the wrong value because it uses editor.state under the hood, which in turn uses the out-of-sync editor.reactiveState property.

Using isNodeActive instead of isMarkActive could also work for those checking if the selection is in a node.

I hope this helps others!

@sytexa-julia
Copy link

I can confirm @mchestnut's workaround is valid, you must use the state from editor.view.
In my case, I wanted to show a BubbleMenu when text is selected, so I adapted the isTextSelected function like this:

function isTextSelected({ editor }: { editor: Editor }) {
  const {
    state: {
      doc,
      selection,
      selection: { empty, from, to },
    },
  } = editor.view // destructure from editor.view instead of just editor

  const isEmptyTextBlock = !doc.textBetween(from, to).length && isTextSelection(selection)

  if (empty || isEmptyTextBlock || !editor.isEditable)
    return false

  return true
}```

segevfiner added a commit to segevfiner/tiptap that referenced this issue Jun 19, 2024
segevfiner added a commit to segevfiner/tiptap that referenced this issue Jun 19, 2024
segevfiner added a commit to segevfiner/tiptap that referenced this issue Jul 2, 2024
nperez0111 added a commit that referenced this issue Jul 15, 2024
…saction due to reactiveState fixes #4870 (#5252)"

This reverts commit 509676e.
nperez0111 added a commit that referenced this issue Jul 15, 2024
…ng a transaction due to reactiveState fixes #4870 (#5252)""

This reverts commit dbab8e4.
@nperez0111 nperez0111 mentioned this issue Jul 22, 2024
5 tasks
nperez0111 added a commit that referenced this issue Jul 22, 2024
…ng a transaction due to reactiveState fixes #4870 (#5252)""

This reverts commit dbab8e4.
@nateeo
Copy link

nateeo commented Jul 26, 2024

@mchestnut thanks for sharing your fix, I was pulling my hair out for ages! FWIW this fixes the same issue for @tiptap/react

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Open Source The issue or pull reuqest is related to the open source packages of Tiptap. Type: Bug The issue or pullrequest is related to a bug
Projects
Status: Done
3 participants