buttondown / tiptap-footnotes

A footnotes extension for Tiptap
MIT License
33 stars 3 forks source link

Loading text with footnotes #6

Open TimMensch opened 3 weeks ago

TimMensch commented 3 weeks ago

I installed tiptap-footnotes, and it seems to work great while the text is still in the editor. Thanks for this! It's exactly what I want it to be.

But it doesn't seem to round-trip. :(

I'm saving out the exact text from getHTML(), and then loading it back in using content=, and after loading the text back in, Tiptap strips all of the class info from the <ol> and <li> entries, which prevents tiptap-footnotes from identifying them as footnotes.

If I try adding a footnote, it will add TWO footnotes below the existing footnote.

Thoughts?

jmduke commented 3 weeks ago

Confirming that I see the same thing — I suspect the culprit is the ol:has(.footnotes) selector here, which we actually want to just be ol.footnotes.

Should be a trivial fix; I'll try to get to it in the next few days!

TimMensch commented 3 weeks ago

OK, I dug into the code. In /package/src/footnotes/footnotes.ts:

  parseHTML() {
    return [
      {
        tag: "ol:has(.footnotes)",
        priority: 1000,
      },
    ];
  },

That selector doesn't work. The <ol> element is the one with a footnotes class, and :has() is looking for a child of the ol to have that class. The <li> elements don't have a class at all.

It should just be tag: "ol.footnotes". That should work.

I guess I'll put together a PR.

TimMensch commented 3 weeks ago

@jmduke Somehow our posts went up out-of-order? I guess the comments are only eventually consistent. :)

I sent over a PR.

I tested it manually and it seems to work.

TimMensch commented 3 weeks ago

OH, and if you want to do the right thing and add tests and whatnot, I won't be offended if you just ditch my PR. It's not exactly a major change. 😄

jmduke commented 3 weeks ago

Ha, jinx!

The PR looks good. Unless you're really under the time-sensitivity gun, I would like to take this as an excuse to set up a testing harness; if by EOD tomorrow I don't carve out the time, I'll merge in yours!

TimMensch commented 3 weeks ago

Thanks! Let me know when a new version is published and I'll switch over from my temporary fork.