signalwerk / gatsby-remark-table-of-contents

gatsby remark plugin to generate table of contents
21 stars 10 forks source link

Wrapping `<div class="toc">` does not work #5

Closed stevenocchipinti closed 3 years ago

stevenocchipinti commented 4 years ago

It looks like this plugin attempts to put a <div class="toc"> before the TOC and a </div> after the TOC, however this results in erroneous HTML for me:

Screen Shot 2020-06-01 at 3 02 41 pm

It looks like the opening tag is being automatically closed and the closing tag is undefined. Despite this erroneous HTML, the output still looks ok in the browser however it does give this error in the console:

Screen Shot 2020-06-01 at 5 39 45 pm
arnaud-deprez commented 4 years ago

Found someone who has similar issue and he solves it like this: https://joshuatz.com/posts/2020/gatsby-wrapping-markdown-in-html-divs-with-custom-classes/. Not sure I will have time to propose a fix soon, so...

signalwerk commented 4 years ago

@arnaud-deprez @stevenocchipinti if one of you found a solution for it I would highly appreciate a pull-request.

ianobermiller commented 4 years ago

One solution would be to just not add the closing tag and if you want to stye with CSS, use the sibling selector:

.toc ~ ul {
  margin: 1em 0;
  /* ... */
}
arnaud-deprez commented 4 years ago

That is indeed the workaround I'm currently using @ianobermiller :-)

signalwerk commented 4 years ago

😂 we all have the same idea...


.toc + ul {

}
ianobermiller commented 4 years ago

@signalwerk good call, I should use plus and not tilde.

zeropaper commented 3 years ago

It should not need such a workaround in the first place. :/

@stevenocchipinti are you also use MDX?

I do.

and when I add a bit of "HTML" code in my ".mdx" file, for instance:

# heading

<div class="html-test">test</div>

## other heading

The AST will consider the <div class="html-test">test</div> bit as "jsx" as follow...

// ...
    },
    {
      type: 'jsx',
      value: '<div class="html-test">test</div>',
      position: [Position]
    },
    {
// ...

And it feels that the following lines: https://github.com/signalwerk/gatsby-remark-table-of-contents/blob/379551c3de958c93d9e669ba9ee4205d649b7bad/src/index.js#L68-L82 of gatsby-remark-table-of-contents source code... may need to be adapted...

Update: I can't reproduce the problem using the "normal" gatsby-transformer-remark, it's almost certainly a problem that only occurs when using MDX.

I just started digging... made a fork and if I can fix that "properly", will make a PR...

zeropaper commented 3 years ago

Grüetzi @signalwerk :)

any chance to get my PR reviewed / merged in the coming days? That would be immensely appreciated!

signalwerk commented 3 years ago

@zeropaper Thank you for your contribution. I didn't time since last weekend to review and document your PR. I will try to do that this week. Sorry for the delay.

signalwerk commented 3 years ago

@stevenocchipinti @arnaud-deprez @ianobermiller I just merged @zeropaper PR and bumped the npm to version 1.0.0. Please make sure if you update to fix zur css definitions. The DOM is now correct and the .toc wraps the whole Table of content

new:

.toc .ul {
 …
}

Thank you @zeropaper for your highly appreciated work!

stevenocchipinti commented 3 years ago

Thank you all :D