frappe / wiki

Free and Open Source Wiki built on top of Frappe
https://frappe.io/wiki
MIT License
226 stars 174 forks source link

feat: add support for tiptap task-item #179

Closed Zyten closed 11 months ago

Zyten commented 12 months ago

Fixes #177

Zyten commented 12 months ago

That glitch seems to be due to this line:

const content = `<div markdown="1">${$(".editor-space .ProseMirror")
  .html()
  .replace(/<h1>.*?<\/h1>/, "")}</div>`;

Sample output:

<ul data-type="taskList">
  <li data-checked="true">
    <label contenteditable="false">
      <input type="checkbox" checked="checked">
      <span></span>
    </label>
    <div>
      <p>Task 1</p>
      <p>Description</p>
    </div>
  </li>
</ul>

Fixed when updated like so:

const content = `<div markdown="1">${editor.getHTML()
  .replace(/<h1>.*?<\/h1>/, "")}</div>`;

Sample output:

<ul data-type="taskList">
  <li data-checked="true" data-type="taskItem">
    <label>
      <input type="checkbox" checked="checked">
      <span></span>
    </label>
    <div>
      <p>Task 1</p>
      <p>Description</p>
    </div>
  </li>
</ul>

But it seems like changing it would introduce regression to existing code though. The issue mentioned there is still open (ueberdosis/tiptap#4044)

Zyten commented 12 months ago

On a related note,

It seems like task-item state is only saved in Edit mode but it's still editable in View mode.

I think the optimal solution would be to allow saving state for it regardless of whether or not the editor is in editable mode (same as Github) but there seems to be a known bug for this option (ueberdosis/tiptap#3676) that hasn't been resolved.

Alternatively I think having the checkbox be read only when in View mode would also be acceptable - as it should already be with the default settings but that does not work as expected (I believe it's due to the same bug). So I added a temporary manual override instead.

BreadGenie commented 11 months ago

Hey, sorry this PR got drifted out of my mind We were initially using editor.getHTML method but it broke the table and we decided to use jquery to fetch HTML.

We aren't using view mode in from tiptap due to SEO reasons which could also be why the checkbox isn't disabled while just viewing the page. I think we could've more control if we used tiptap view mode but it's not viable in our situation.

Zyten commented 11 months ago

No worries :)

If that's the case, I'll update the code such that the jquery method is retained and see if I can fix it using an alternative method. I suspect I just need to append data-type="taskItem" to the list item prior to saving.

As for the other issue, since not using tiptap view mode is a design decision, would it be okay to keep my manual patch to make the checkbox readonly when in Wiki's View Mode? I added comments to remove the patch once the upstream bug is fixed.

BreadGenie commented 11 months ago

As for the other issue, since not using tiptap view mode is a design decision, would it be okay to keep my manual patch to make the checkbox readonly when in Wiki's View Mode? I added comments to remove the patch once the upstream bug is fixed.

Works for me

cypress[bot] commented 11 months ago

Passing run #234 ↗︎

0 5 0 0 Flakiness 0

Details:

Merge 32c2654c0e71f0f0fd901823a678a30202353da9 into 5c2917a3fa3e980b308aa35b2827...
Project: Wiki Commit: ec751e11bd ℹ️
Status: Passed Duration: 00:29 💡
Started: Sep 20, 2023 2:10 PM Ended: Sep 20, 2023 2:10 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.