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

Build the right-side table of contents from javascript #1049

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

fekete-robert
Copy link
Collaborator

@fekete-robert fekete-robert commented Jun 15, 2022

By default, Docsy shows the table of contents for the current page in the right sidebar using the built-in function of Hugo. You can replace that with a JavaScript-based ToC that uses the [https://tscanlin.github.io/tocbot/](Tocbot library) by setting the following in your config.toml file:

[params.jstoc]
enable = true

Compared to the default sidebar ToC, this solution:

  • has a marker that shows the current location of the screen (useful for long pages)
  • shows the correct title even if the title contains a shortcode
  • shows the title in the toc even if it was included from another file

For a working example, see https://kube-logging.dev/docs/

@jmichelgarcia
Copy link

Exciting! Can't wait for this!

@fekete-robert
Copy link
Collaborator Author

Thanks @jmichelgarcia, glad you like it!
Rebased and resolved conflicts. @chalin , might you have some time to review this PR?

@sftim
Copy link

sftim commented Oct 18, 2022

@fekete-robert would you be willing to rebase this against main?

If you're not sure what that means, reply here and someone who does know how may be able to make time to guide you through that.

@sftim
Copy link

sftim commented Oct 18, 2022

😕 @fekete-robert did you mean to close this PR?

    Uses the https://tscanlin.github.io/tocbot/ library to build the sidebar toc
    if the following is set in your config.toml file:

    [params.jstoc]
    enable = true

    Note that by default, h2-h4 headings are included in the toc. To change that,
    provide a comma-separated list of headings like this:

    [params.jstoc]
    enable = true
    custom_headings = "h2, h3"
    custom_headings = "h2, h3"

    Compared to the default sidebar toc, this solution has the following benefits:

    - a marker shows the current location of the screen (useful for long pages)
    - shows the correct title even if the title contains a shortcode
    - shows the title in the toc even if it was included from another file
@fekete-robert fekete-robert reopened this Oct 18, 2022
@fekete-robert
Copy link
Collaborator Author

Hi @sftim, sorry I didn't intend to close it, just messed something up during the rebase.

@fekete-robert
Copy link
Collaborator Author

Should be ok now

Copy link

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in place of static/js/tocbot.min.js we should import the unminified source, including a comment about its provenance.

My understanding is that Hugo can invoke a minifier as needed.


userguide/static/images/sidebar-toc-with-tocbot.png doesn't seem to show any highlight.

@fekete-robert
Copy link
Collaborator Author

@sftim :
It seems that the tocbot release archive (https://github.com/tscanlin/tocbot/releases/tag/v4.12.0) contain only the minified version (that is, the same minified file under the tocbot.min.js and tocbot.js name), so I don't think it makes any difference.

The highlighting is not very visible on the screenshot, because not the entire title is highlighted, just the section of the vertical splitbar next to the title. It's more apparent when you are actually scrolling on the page.

@sftim
Copy link

sftim commented Oct 18, 2022

How are we honoring the tocbot LICENSE?

@fekete-robert
Copy link
Collaborator Author

fekete-robert commented Oct 18, 2022 via email

@fekete-robert
Copy link
Collaborator Author

fekete-robert commented Oct 18, 2022 via email

@shuuji3
Copy link
Contributor

shuuji3 commented Jul 9, 2023

Hi, @fekete-robert. Thank you for your work and the update! This is a better and complete solution for the ToC feature.🙂

Although using the original tocbot code itself is allowed by MIT License, it also requires including the original MIT License in the copied code (please refer to the original LICENSE here: tocbot/LICENSE at master · tscanlin/tocbot · GitHub - https://github.com/tscanlin/tocbot/blob/master/LICENSE#L12-L13). So I think one option could be to include the entire MIT License text as a code comment in static/js/tocbot.min.js and static/scss/tocbot.css.

Also, I'd recommend including the original URL and tocbot version so that we can easily track the version and refer to it during a future update.

@fekete-robert
Copy link
Collaborator Author

@shuuji3 Many thanks for spotting that, I've added the licenses and updated the js/css to use a newer version (4.21.0)

assets/scss/tocbot.css Outdated Show resolved Hide resolved
Co-authored-by: chulcher <31546889+chulcher@users.noreply.github.com>
@jmichelgarcia
Copy link

I still have hope that this will get merged one day. 🥇

@deining
Copy link
Collaborator

deining commented Mar 14, 2024

I still have hope that this will get merged one day. 🥇

Me too. I realize that this PR pulls in tocbot.min.js from static folder. Since tocbot is actively maintained —latest version is 4.25.0, while this PR still uses 4.21.0— it might be useful to pull in the latest version of the script from CDN at build time via hugo's resources.GetRemote function.
@chalin: any timeline for this PR? I'm surprised to see that this PR is not assigned to any of the existing milestones yet 😟.

@fekete-robert
Copy link
Collaborator Author

@deining: If I have some time this weekend I'll update the PR to use the CDN, and resolve the conflicts as well. (Originally I didn't use the CDN because I prefer to use local copies, and was also hoping that #1204 gets merged, and wanted to use that approach )

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

Successfully merging this pull request may close these issues.

6 participants