Table of contents
klaascuvelier opened this issue · 17 comments
Hi @toddmotto, this a great plugin.
I was wondering if you ever got the request to get a list of generated links.
I use this plugin myself and wanted to have some sort of "table of contents" dynamically generated based upon the links that were generated by your plugin.
I achieved it this way now:
var linkNodes = [].slice.call(document.getElementsByClassName(linkjuiceClass));
var menu = document.getElementById('table-of-contents');
menu.innerHTML = linkNodes
.map(function (node) {
return '<li><a href="' + node.href + '">' + node.innerHTML + '</a></li>';
})
.join('');This works, but is obviously not the most desirable way of doing it.
I could also populate an array using the contentFn function, but that's very impure.
An alternative would be to give the list of links generated (id + title) as function return or have something link linkjuice.links (Array/NodeList) which can be iterated on.
I can help you implement this if you'd want that, but if I am the only person who ever had this request, you can just ignore this :)
Hey man, this is an awesome idea. Was actually speaking to colleague of mine about this a few weeks back. Simple flag/element selector with a new { tableOfContents: '.my-class' }? With a false default?
I'm not sure if I completed understand your suggestion:
You want to add an option tableOfContents? And when it's a string (not false) add that class onto the wrapped element? So the developer would still have to do document.querySelectorAll('table-of-contents-class') himself/herself to create the actual table.
Is that correct?
Well, people might want to use it without generating a TOC. So the idea is they'd do (at the top of their document):
<!-- blog title etc -->
<div class="toc"></div>
<!-- blog content -->And then the init:
linkjuice.init({
tableOfContents: '.toc'
});This would automagically compile + inject the list for them at runtime :) that was my idea anyway. And if they don't even have a TOC, they just won't use the option.
Makes sense. I assume this would also require contentFn for the TOC?
So 2 new options: tableOfContents + tableOfContentsContentFn?
Follow up question(s):
If the selectors are h1, h2, h3, ..., do you nest lists in the table of contents, or would you keep it flat?
Do you make nesting an option?
The tableOfContentsContentFn could also have a 2nd argument which is level, or let the user detect the level through node.tag. But that makes it semantically incorrect.
Haha, this was what I was thinking also for nested TOCs. Maybe we could somehow implement header mapping yeah.
Thoughts on this?
linkjuice.init({
tableOfContents: {
nested: true, // boolean: creates the nested TOC
selector: '.toc', // string: element Node to inject into
contentFn() {...} // function: customise template + injected content
}
});Yeah that sounds good. Keeps the config quite simple and gives end user a lot of control.
Is the contentFn used for the "title" node only and the plugin takes care of ul and li elements?
Yeah, I think we can detect nested of headings - this may need to become a configurable option though to tell it what headings to map to nested lists?
linkjuice.init({
tableOfContents: {
nested: ['h1', 'h2'], // array: creates the nested TOC, nests based off new heading selectors?
selector: '.toc', // string: element Node to inject into
contentFn() {...} // function: customise template + injected content
}
});Maybe if you use nested: true, the TOC will automatically be generated from h1/h2/h3 ? Might be nice feature.
Yeah, that makes sense to me.
Btw, the Contribution section mentions unit test. You want me to set up some Karma infrastructure for this as well?
Another question, I'm just trying to understand your code decisions, any specific reason scope, nodes and inject are defined at the top of the script?
Only inject is used outside the linkjuice const scope, and even then you could just pass it along in the wrapNode call.
I took a first try on this, let me know what you think. https://github.com/klaascuvelier/linkjuice/pull/1/files
(It's not a PR to your repo yet)
ping @toddmotto, did you see this?
@klaascuvelier Ahh, ja sorry maatje! Echt goed. Are there any edge cases you think we should be concerned of? If not, ping the PR here and we can test it and merge it :)
I tried this out myself with some different test cases and it worked well.
I just did another small change, now the TOC gets generated before the links are added.
So TOC elements have original content (before running contentFn, so no icons, ...), and TOC links should work as the original elements should have an id already.
From my side this seems finished, let me know what you think @toddmotto
implemented in #9