fix(task-item): fix read only checked handling

[nicksellen]

Fixes #3676

Changes Overview

onReadOnlyChecked allows you to determine whether the checking/unchecking the task item should apply the change, or not.

It did not work because:

• it only ever passed the original node to the function, not the current node
• even if you returned true it would not apply the change to the document

Implementation Approach

Now it runs the logic inside the transaction where we have access to the current node, and we can either abort it as before if it returns false, or allow it to proceed with the transaction if it returns true.

Testing Done

Only manual testing whilst clicking in a browser.

Verification Steps

Configure your editor like this:

new Editor({
    editable: false,
    // + other options
    extensions: [
        TaskItem.configure({
            // Allow checking/unchecking when in read only mode
            onReadOnlyChecked: () => true,
        }),
        // + other extensions
    ]
})    

Then, ideally, open a couple of windows with a collaborative session running, and notice that you can check/uncheck the box in one window, but the change is not propagated to the other.

Additionally, if you want to have more logic (e.g. the kind of logic in #4521 - where the node descendent are checked for matching), it should be possible to observe the node is now the correct node, and can be matched if desired:

onReadOnlyChecked: (node, checked) => {
  editor.state.doc.descendants((subnode, pos) => {
    if (node.eq(subnode)) {
      const { tr } = editor!.state;
      tr.setNodeMarkup(pos, undefined, {
        ...node.attrs,
        checked: checked,
      });
      editor.view.dispatch(tr);
    }
  });
  return true;
},

Additional Notes

There is also another bug, in the update method because it uses checkbox.setAttribute('checked', 'checked') and checkbox.removeAttribute('checked') which is not the correct way to change the state of a checkbox, should just be checkbox.checked = checked.

Should I add a fix to this PR? Maybe best in another one....

Update: I submitted a separate PR for that #5953

Checklist

• I have created a changeset for this PR if necessary.
• My changes do not break the library.
• I have added tests where applicable.
• I have followed the project guidelines.
• I have fixed any lint issues.

Related Issues

#3676
#4521

[netlify]

✅ Deploy Preview for tiptap-embed ready!

Name	Link
🔨 Latest commit	4a01efc
🔍 Latest deploy log	https://app.netlify.com/sites/tiptap-embed/deploys/6766b3b384fbad0008bd3bf5
😎 Deploy Preview	https://deploy-preview-5952--tiptap-embed.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 4a01efc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[nperez0111]

This looks good code-wise. Would love to have a test for this added though to make sure that we don't regress

[nicksellen]

Great! I'm interested in adding a test, although it'll expand my task quite a bit, to set up a tiptap dev env, plus get my head around your testing environment, so let's see how that goes! Also happy if somebody else wants to jump in with that... but yeah, I'll add it to my todo list 👍