tiptap icon indicating copy to clipboard operation
tiptap copied to clipboard

update event not triggered on checkbox when onReadOnlyChecked returns true and editable is false

Open deterralba opened this issue 2 years ago • 8 comments
trafficstars

What’s the bug you are facing?

Hello & thank you for this awesome lib :+1:

The 'update' event is not not triggered on checkbox elements when the editor is not in its editable state but onReadOnlyChecked returns true (tested with react)

Which browser was this experienced in? Are any special extensions installed?

latest firefox version on ubuntu

How can we reproduce the bug on our side?

with this sandbox : https://codesandbox.io/s/update-not-trigger-on-checkbox-when-editable-is-false-v87v22?file=/src/App.js:468-485 you can see in the console that the update event is not trigger when editable is false, but it is when editable is true.

Can you provide a CodeSandbox?

here https://codesandbox.io/s/update-not-trigger-on-checkbox-when-editable-is-false-v87v22?file=/src/App.js:468-485

What did you expect to happen?

I would expect the update event to be trigger when the checkbox state changes, even if editable is set to false. Is this is not a bug but a API design choice, do I have the ability to trigger manually an update event?

Anything to add? (optional)

No response

Did you update your dependencies?

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

Are you sponsoring us?

  • [ ] Yes, I’m a sponsor. 💖

deterralba avatar Jan 30 '23 17:01 deterralba

We're struggling with the exact same issue while trying to save the contents of a non-editable editor via an API call. (And yes, we're sponsors.)

parsch avatar Mar 14 '23 12:03 parsch

We're not able to save updated contents (to a DB) because the contents are not updated when toggling a TaskItem (with onReadOnlyChecked configured) in a non-editable editor. I've got no idea how else we could get updated contents.

I've forked/updated the CodeSandbox of @deterralba (thx) to better illustrate the problem we're facing: https://codesandbox.io/s/tiptap-taskitem-content-not-updated-when-not-editable-tnsbkx?file=/src/App.js

parsch avatar Mar 16 '23 08:03 parsch

I found the same issue and tried to use the first parameter node of onReadOnlyChecked to find out which taskItem changed, but there seemed to be no way to know

yenche123 avatar Mar 20 '23 12:03 yenche123

We're not able to save updated contents (to a DB) because the contents are not updated when toggling a TaskItem (with onReadOnlyChecked configured) in a non-editable editor. I've got no idea how else we could get updated contents.

I've forked/updated the CodeSandbox of @deterralba (thx) to better illustrate the problem we're facing: https://codesandbox.io/s/tiptap-taskitem-content-not-updated-when-not-editable-tnsbkx?file=/src/App.js

I updated the Sandbox to Tiptap 2.0.3, the problem still exists. Any chance that a team member looks into this or can give a hint?

parsch avatar Apr 26 '23 07:04 parsch

I faced the same problem, in the end i used patch-package to changed the source code, i am using @tiptap/[email protected], this is the change:

diff --git a/node_modules/@tiptap/extension-task-item/dist/index.js b/node_modules/@tiptap/extension-task-item/dist/index.js
index 39c698c..3001751 100644
--- a/node_modules/@tiptap/extension-task-item/dist/index.js
+++ b/node_modules/@tiptap/extension-task-item/dist/index.js
@@ -100,9 +100,17 @@ const TaskItem = Node.create({
                 }
                 if (!editor.isEditable && this.options.onReadOnlyChecked) {
                     // Reset state if onReadOnlyChecked returns false
-                    if (!this.options.onReadOnlyChecked(node, checked)) {
-                        checkbox.checked = !checkbox.checked;
+                    if (checkbox.checked) {
+                        checkbox.setAttribute('checked', 'checked');
+                    } else {
+                        checkbox.removeAttribute('checked');
                     }
+                    listItem.dataset.checked = checkbox.checked;
+                    const editorDom = editor.view.dom;
+                    Array.from(editorDom.querySelectorAll('li[data-checked]')).forEach(item => {
+                        item.dataset.type = "taskItem";
+                    });
+                    this.options.onReadOnlyChecked(node, checked, editorDom.innerHTML);
                 }
             });
             Object.entries(this.options.HTMLAttributes).forEach(([key, value]) => {

the original code only passes node and checked to onReadOnlyChecked, what i did is,

  1. firstly update DOM of list-item and the input to reflect the new checked value;
  2. then add "data-type=taskItem" to all li[data-checked] element (for some reason, if i don't do this, all li won't have data-type, then the checkbox won't be checked);
  3. then get the whole html from editor.view.dom.innerHTML, and pass it to the third param. (i tried to get the whole html from editor.getHTML(), but it doesn't work, seems its value is not updated at this moment.)

i also ignored the return value, if there is a onReadOnlyChecked, i will just update the checked value.

penghuili avatar Aug 10 '23 21:08 penghuili

FWIW here's another solution that works for me... although still feels quite hacky

NOTE 1: I'm working in Vue but imagine the approach is similar for any other JS NOTE 2: This is based off using the UniqueID extension so that each node has an ID to scan for

Quick rundown of the approach:

  1. Firstly, I update the actual DOM so that all my styling for checked/unchecked works correctly straight away
  2. Then instead of trying to interact with the editor in it's non-editable state (which seemed very hit and miss), I actually update the value that I have stored in my Vue component. I used my "stringified" version of the JSON instead of the actual JSON object so that I didn't have to use a costly recursive search through the JSON object to find the matching block.
  3. Then I can use that stringified JSON to update the actual JSON value and also the editor value (not even sure that last part is needed but wanted to make sure it's updated say if then the user switches into full edit mode without reloading the page)
  4. Lastly I just check if the value has actually changed, and then emit an event from my Tiptap component to tell it's parent that the value should be updated in the database, and then handle that "quietly" so the user doesn't really notice that the data has been passed back and forth between the database.

Welcome any suggestions or corrections... as I said this still feels quite hacky but might help someone.

TaskItem.configure({
  nested: true,
  onReadOnlyChecked: (node, checked) => {
    // get the dom from the editor and then find the node by block id
    let editorDom = editor.value.view.dom
    let nodeDom = editorDom.querySelector(`[data-blockid="${node.attrs.blockId}"]`)
    // change the data-checked attribute on the node to checked value
    nodeDom.setAttribute('data-checked', checked)
    // change the input checked value
    nodeDom.querySelector('input').setAttribute('checked', checked)
    // update the currentValueJSON by finding the node in the json string and replacing it with the new checked value
    currentValueJSON.value = currentValueJSON.value.replace(`"blockId":"${node.attrs.blockId}","checked":${!checked}`, `"blockId":"${node.attrs.blockId}","checked":${checked}`)
    // update the current value array
    currentValue.value = JSON.parse(currentValueJSON.value)
    // update the editor value based off the updated array
    editor.value.commands.setContent(currentValue.value, false)
    nextTick(() => {
      // check if the value has changed and if so then emit update:shouldSave
      if(JSON.stringify(currentValue.value) !== JSON.stringify(props.value)) {
        emit('update:shouldSaveQuietly')
      }
    })
    return true
  }
}),

mgtcampbell avatar Sep 05 '23 03:09 mgtcampbell

This is still a problem in 2.1.13.

kolaente avatar Dec 11 '23 22:12 kolaente

Any chance to get this fixed? I'm currently pouring workaround over workaround to make this work, but I don't think that should be necessary, neither does it seem like doing that is maintainable.

kolaente avatar Feb 17 '24 21:02 kolaente