-
Notifications
You must be signed in to change notification settings - Fork 5
TOC expand-collapse icons #659
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
Conversation
Added icons for collapsable/expandable TOC sections. minor minor
|
I wonder why the preview shows the TOC running out of it's surrounding shaded div at the bottom? Is that related to this PR, the partial build here and will it also be present when rebuilding everything?
UPDATE: It's the same for main branch. It's not a problem for the final full docs. So probably just related to the way this demo-doc is setup. @copilot Create a separate issue for this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds expand/collapse functionality to the table of contents (TOC) navigation sidebar with visual icons. The feature is configurable and enabled by default, allowing users to interactively toggle visibility of nested navigation items by clicking on parent items with children.
- Adds configurable TOC toggle icons with a new theme option
toc_toggle_icons(enabled by default) - Implements JavaScript logic to detect navigation items with children and handle expand/collapse interactions
- Provides CSS styling for chevron icons with proper ARIA accessibility attributes
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/crate/theme/rtd/crate/theme.conf | Adds toc_toggle_icons configuration option with default value "true" |
| src/crate/theme/rtd/crate/static/js/toc-toggle.js | New JavaScript module implementing expand/collapse logic, click detection, and accessibility features |
| src/crate/theme/rtd/crate/static/js/index.js | Imports the new toc-toggle module |
| src/crate/theme/rtd/crate/static/css/crateio-rtd.css | Adds CSS for expand/collapse icons, hover effects, and visibility control; changes .bs-docs-sidenav > li display from "none" to "block" |
| src/crate/theme/rtd/crate/sidebar.html | Conditionally adds toc-toggle-icons-enabled class based on theme configuration |
| docs/conf.py | Adds documentation comment about the new feature and how to disable it |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kneth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we have verified that #659 (comment) is resolved, I am fine with merging it.
@kneth, As mentioned, there is the same issue with the main branch, and that doesn't result in any problems. So this PR doesn't introduce the issue. |
| /** | ||
| * TOC Navigation Expand/Collapse Toggle | ||
| * | ||
| * This module adds interactive expand/collapse functionality to the table of | ||
| * contents (TOC) navigation sidebar. It allows users to click on parent items | ||
| * to show/hide their children. | ||
| * | ||
| * Features: | ||
| * - Click on items with children to toggle expand/collapse | ||
| * - Auto-expand current page's parent hierarchy | ||
| * - Add .has-children class for browsers without :has() support | ||
| * - ARIA attributes for accessibility | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. Is it using the implementation from Furo like planned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not at all.
I don't see anything mentioned in that PR about the icons? Perhaps I'm missing something and we can get this in a different way? Possibly even more advanced or nicer looking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should have clarified better: While the patch referenced above is about bringing in alternative navigation definition mechanics, it demonstrates that all the CSS/JS elements to support Furo's navigation runtime mechanics are already included into the assets bundle, so I was wondering if we can use those instead of bringing in a separate implementation about the same topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me be more explicit by referring to relevant components of Furo defined in assets/styles/components/_sidebar.sass:
[...] use those instead of bringing in a separate implementation [...]
Possibly even more advanced or nicer looking?
I didn't dive into finding out how the currently defined navigation can be connected to those Furo mechanics, also because my patch was exploring another direction. If you can make sense how to connect the HTML in sidebartoc.html with Furo's JS/CSS, that could possibly bring both worlds together, with the benefit of perfect rendering and collapsing mechanics by battle-tested Furo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all the CSS/JS elements to support Furo's navigation runtime mechanics are already included
Furo's assets/styles/components/_sidebar.sass has been vendorized into our theme already, apparently from Furo 2024.05.06.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pointers!
I wonder if these are the attractive benefits we could gain:
- Better tested
- collapsing/uncollapsing sections independently of others (so uncollapsing don't close other uncollapsed sections - as today)
- visuals better
The main (80%) value is already achieved through this PR, but I can take a look.
I'm not sure what the effort would be, but if it's not too much I can do it. Otherwise we could take this in as a first step, and improve as a next step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thanks. Please proceed at your disposal.
|
An alternative reusing more of Furos and enhanced with storing state for section expansion is here |
|
We decided to use the alternative implementation. |

Added icons for collapsable/expandable TOC sections.
It makes it easier to see which sections can be expanded when browsing for what might be relevant.
Preview
https://crate-docs-theme--659.org.readthedocs.build/en/659/
Questions