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.

Sign up for GitHub

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

BUG - stop using Scrollspy #2107

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

BUG - stop using Scrollspy #2107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28560cb
BUG - stop using Scrollspy
gabalafou Jan 24, 2025
52133b3
Update src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js
gabalafou Jan 27, 2025
5a73836
[pre-commit.ci] Automatic linting and formatting fixes
pre-commit-ci[bot] Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 30 additions & 21 deletions src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,30 +96,39 @@ function addModeListener() {
}

/*******************************************************************************
* TOC interactivity
* Right sidebar table of contents (TOC) interactivity
*/
function setupPageTableOfContents() {
const pageToc = document.querySelector("#pst-page-toc-nav");
pageToc.addEventListener("click", (event) => {
const clickedLink = event.target.closest(".nav-link");
if (!clickedLink) {
return;
}

/**
* TOC sidebar - add "active" class to parent list
*
* Bootstrap's scrollspy adds the active class to the <a> link,
* but for the automatic collapsing we need this on the parent list item.
*
* The event is triggered on "window" (and not the nav item as documented),
* see https://github.com/twbs/bootstrap/issues/20086
*/
function addTOCInteractivity() {
window.addEventListener("activate.bs.scrollspy", function () {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I somewhat kept this behavior here of expanding nested portions of the TOC.

Previously, the TOC would auto-expand as the user scrolled down the page into sections nested deeper and deeper. For example if the sidebar page TOC was configure to only show links up to level 2 headings, then as the user scrolled into level 3, level 4, level 5, the sidebar TOC would also expand to show increasingly nested and indented links to those sections.

Now, the TOC does not update as the user scroll, but if the user clicks a link in the TOC that has sub-entries, the sub-entries will be shown, and if the user clicks one of those sub-entries and those sub-entries have sub-entries, then it will expand one more level, and so on and so forth.

For more info, refer to show_toc_level.

const navLinks = document.querySelectorAll(".bd-toc-nav a");

navLinks.forEach((navLink) => {
navLink.parentElement.classList.remove("active");
// First, clear all the added classes and attributes
// -----
pageToc.querySelectorAll("a[aria-current]").forEach((el) => {
el.removeAttribute("aria-current");
});

const activeNavLinks = document.querySelectorAll(".bd-toc-nav a.active");
activeNavLinks.forEach((navLink) => {
navLink.parentElement.classList.add("active");
pageToc.querySelectorAll(".active").forEach((el) => {
el.classList.remove("active");
});

// Then add the classes and attributes to where they should go now
// -----
clickedLink.setAttribute("aria-current", "true");
clickedLink.classList.add("active");
Comment on lines +120 to +121
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will add styles to the link that make it look like the current link. This matches the behavior of the GitHub Actions docs, for example see Understanding GitHub Actions.

Note that once the user clicks on the link, it keeps this styling even if they scroll to a completely different section. It only gets cleared and applied to a different TOC link if the user clicks on a different TOC link.

// Find all parents (up to the TOC root) matching .toc-entry and add the
// active class. This activates style rules that expand the TOC when the
// user clicks a TOC entry that has nested entries.
let parentElement = clickedLink.parentElement;
while (parentElement && parentElement !== pageToc) {
if (parentElement.matches(".toc-entry")) {
parentElement.classList.add("active");
}
parentElement = parentElement.parentElement;
}
});
}

Expand Down Expand Up @@ -1021,7 +1030,7 @@ documentReady(fetchRevealBannersTogether);

documentReady(addModeListener);
documentReady(scrollToActive);
documentReady(addTOCInteractivity);
documentReady(setupPageTableOfContents);
documentReady(setupSearchButtons);
documentReady(setupSearchAsYouType);
documentReady(setupMobileSidebarKeyboardHandlers);
Expand Down
3 changes: 2 additions & 1 deletion src/pydata_sphinx_theme/assets/styles/components/_toc-inpage.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ nav.page-toc {

@include link-sidebar;

&.active {
&.active,
&[aria-current="true"] {
@include link-sidebar-current;

background-color: transparent;
Expand Down
2 changes: 1 addition & 1 deletion src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/page-toc.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
class="page-toc tocsection onthispage">
<i class="fa-solid fa-list"></i> {{ _('On this page') }}
</div>
<nav class="bd-toc-nav page-toc" aria-labelledby="{{ page_navigation_heading_id }}">
<nav id="pst-page-toc-nav" class="page-toc" aria-labelledby="{{ page_navigation_heading_id }}">
{{ page_toc }}
</nav>
{%- endif %}
10 changes: 4 additions & 6 deletions src/pydata_sphinx_theme/theme/pydata_sphinx_theme/layout.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,14 @@
{% endif %}
{%- endblock extrahead %}
{% block body_tag %}
{# set up with scrollspy to update the toc as we scroll #}
{# ref: https://getbootstrap.com/docs/4.0/components/scrollspy/ #}
<body data-bs-spy="scroll" data-bs-target=".bd-toc-nav" data-offset="180" data-bs-root-margin="0px 0px -60%" data-default-mode="{{ default_mode }}">
<body data-default-mode="{{ default_mode }}">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved the skip link out of the body_tag block for two reasons:

  1. I noticed that if consumers of this theme wanted to customize the scroll spy behavior they would need to override this block, but if they did that, there's a good chance they might nuke the skip link in the process.
  2. It's counterintuitive to put anything in a block named "body_tag" other than, well, the body tag.

{%- endblock %}

{% block header %}
{# A button hidden by default to help assistive devices quickly jump to main content #}
{# ref: https://www.youtube.com/watch?v=VUR0I5mqq7I #}
<div id="pst-skip-link" class="skip-link d-print-none"><a href="#main-content">{{ _("Skip to main content") }}</a></div>

{%- endblock %}
{% endblock %}

{%- block content %}
{# A tiny helper pixel to detect if we've scrolled #}
Expand Down Expand Up @@ -148,7 +147,6 @@
</footer>
{%- endblock footer %}
{# Silence the sidebars and relbars since we define our own #}
{% block header %}{% endblock %}
{% block relbar1 %}{% endblock %}
{% block relbar2 %}{% endblock %}
{% block sidebarsourcelink %}{% endblock %}
Loading