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

fix(task-item): fix updating checked status #5953

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

nicksellen
Copy link

Changes Overview

The code currently uses setAttribute / removeAttribute to update the checked status of the checkbox, this is the incorrect method to use. Checkboxes have a checked property that can be set.

See https://aileenrae.co.uk/blog/programatically-check-uncheck-checkbox-javascript/

Implementation Approach

Switch to setting checkbox.checked property

Testing Done

Manually in browser.

Verification Steps

Open a collaborative session with task item extension available.

In one window check/uncheck the box a few times, then in the other window do the same.

Notice that the update appears to no longer propagate, the checkbox state does not change in the other window.

It is actually changing the attribute on the document node correctly, which you can see if have styling applied such as this (on the task item):

li[data-checked='true'] {
    text-decoration: line-through;
}

With the fix, it works again!

Additional Notes

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

I couldn't find an existing issue for it, which I thought was surprising, but maybe I didn't look well enough.

Copy link

netlify bot commented Dec 21, 2024

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 87315ba
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/6766b1d6c403d00008f5190d
😎 Deploy Preview https://deploy-preview-5953--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant