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

Allow user to set classes for ul, li, and a #16

Open
peterdesmet opened this issue May 8, 2018 · 3 comments
Open

Allow user to set classes for ul, li, and a #16

peterdesmet opened this issue May 8, 2018 · 3 comments

Comments

@peterdesmet
Copy link

Hi, really cool and functional plugin! I've started to use it to create a scrollspy-like TOC on the side of an article. Works great, but I currently have to use jQuery to have the TOC look like a Bootstrap nav:

$(document).ready(function() {
    $("#toc ul").addClass("nav flex-column");
    $("#toc li").addClass("nav-item");
    $("#toc a").addClass("nav-link");
});

Being a static website generator, it would be a lot better though if those classes could be passed to the plugin via the Pelican settings, so the TOC is rendered with those classes. Something like:

TOC = {
    "TOC_HEADERS": "^h2",
    "TOC_INCLUDE_TITLE": False,
    "TOC_UL": ["nav", "flex-column"],
    "TOC_LI": ["nav-item"],
    "TOC_A": ["nav-link"]
}

I've started an implementation, but got lost in passing parameters from function to function. There also too many design decisions involved, which I rather not take. 😄 But some thoughts:

  • I think having the option to set classes for ul, li, and a is sufficient. Optionally, classes could be supported for the wrapping div.
  • By default, no class should be set. Ideally, this does not result in an empty class attribute (<li class="">....</li>).
  • I would remove the current toc-href class for links. Personally, I would also remove the title attribute for links.
  • Since the user can pass a number as a class (e.g. "TOC_A": [1, "test"]), the list concatenation should convert to strings first ', '.join(map(str, content.settings[TOC_KEY]['TOC_A']))

Just a thought.

@ingwinlu
Copy link
Owner

ingwinlu commented May 9, 2018

Hi

it used to work with bootstrap3. I only upgraded all my pelican stuff (or at least tried to) to bootstrap 4 this week.

I will have to look into it, but I don't think I will add it in this plugin. For bootstrap specific tagging purposes I wrote pelican-bootstrapify.

@peterdesmet
Copy link
Author

Thanks! I use pelican-bootstrapify, but it targets article.content and page.content, while the toc lives outside of that. The suggested functionality to add classes would not be bootstrap specific, buy cater for any classes that need to be added.

@fostermaier
Copy link

fostermaier commented Jul 15, 2020

but it targets article.content and page.content, while the toc lives outside of that.

Same problem here. I like pelican-toc pretty much, but the current lack of additional class attributes makes it impossible to be used in combination with scrollspy. Boostrap 4.5 explicitly requires either nav-link or list-group-item to be present within the anchor tag <a> (scrollspy.js)

I tried subclassing .toc-href in scss to @extend any of the former without any success as the new class cannot be parsed and resolved by scrollspy.js.

My current workaround is a hard-coded patch to the local version of the plugin expanding the <a> tag.

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

No branches or pull requests

3 participants