feat: add TOC sidebar for README and Markdown files #36250
Conversation
- Add floating TOC panel that displays table of contents for README and markdown files - Implement toggle button to show/hide TOC panel - Add localStorage persistence for TOC panel visibility state - Support both README in repo root and individual markdown file views - Add responsive design for mobile screens
GiteaBot
added
the
lgtm/need 2
github-actions
bot
added
modifies/go
|
1 there is a special background color on the toc icon which is unnecessary. |
…ectionObserver - Replace window resize and scroll event listeners with a ResizeObserver to monitor changes in the document body. - Implement an IntersectionObserver for the file header to trigger TOC position updates based on its visibility. - This improves performance and responsiveness of the TOC sidebar in file views.
I have remove
I have completed the new commit, and the effect is shown in the screenshot below. Please compile and test it, and if you find any issues, please let me know. I will fix them promptly. |
The second issue has been resolved, but I believe the first one has not. Additionally, the icons shown in the README and in the file view are different. The file view icon should have a border to match the style of the other icons. |
|
Regarding the sidebar naming, I think different file formats should be able to have their own sidebar content. At the moment, only Markdown supports a TOC sidebar. If we introduce some abstraction here, it would be much easier to support sidebars for other file formats in the future. |
- Change references from TOC to sidebar in the file view and README rendering functions. - Update CSS classes and JavaScript functions to reflect the new sidebar implementation. - Ensure proper visibility and positioning of the sidebar in the file view context.
- Enhance sidebar positioning to ensure it aligns with the file header and does not exceed the content's bottom. - Implement logic to hide the sidebar when the file content is scrolled out of view. - Replace IntersectionObserver with a scroll event listener for updating sidebar position, improving performance and responsiveness.
- Add a new Header type to encapsulate header data for generating the sidebar TOC. - Update the rendering logic to utilize SidebarTocHeaders, providing a more flexible structure for TOC generation. - Implement extraction of headers from orgmode documents to populate SidebarTocHeaders. - Ensure backward compatibility by maintaining the legacy SidebarTocNode for existing functionality.
- Simplify the calculation of the sidebar's top position to ensure it aligns with the file header or sticks to the top of the viewport when necessary. - Remove redundant opacity handling and improve clarity in the sidebar's visibility logic. - Maintain the sidebar's position next to the content with a consistent gap.
|
I recently had surgery, so I'll find time next weekend to address this. |
- Dynamically calculate the maximum height of the sidebar to prevent it from extending below the segment bottom. - Implement logic to hide the sidebar when the available height is insufficient, improving user experience. - Ensure the sidebar's top position does not exceed the segment's top during scrolling.
…position updates - Utilize IntersectionObserver to enhance performance and avoid issues associated with scroll events. - Implement fine-grained position updates using multiple thresholds for better responsiveness during scrolling.
- Update comments in RenderContext to clarify the deprecation of SidebarTocNode in favor of SidebarTocHeaders. - Remove unnecessary blank line in RenderSidebarTocHTML function for improved code readability.
- Add border styling to the sidebar toggle button in file view to match other buttons. - Update hover state to change border color for improved visual feedback.
Abstraction has been added. 
Are the current icons displayed correctly? |
|
Could we remove |
…endering - Eliminate the deprecated SidebarTocNode from RenderContext and related functions. - Update sidebar TOC rendering logic to exclusively use SidebarTocHeaders for improved clarity and maintainability. - Remove fallback logic for legacy TOC rendering to streamline the codebase.
|
You're right, I've updated it - removed SidebarTocNode and the fallback logic entirely. Thanks for the suggestion! |
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Signed-off-by: hamkido <hamki.do2000@gmail.com>
|
The two icons on the README have different style. One has borders, but another does not. In the file view UI, the size of the icons are different. |
There was a problem hiding this comment.
Pull request overview
This PR adds a floating Table of Contents (TOC) sidebar panel for README and Markdown files in Gitea, addressing issue #36249. The implementation provides users with an easy way to navigate long documentation by displaying document structure and enabling one-click jumps to any section.
Changes:
- Added TOC extraction and rendering infrastructure in the markup package with new
Headertype andRenderSidebarTocHTMLfunction - Extended orgmode and markdown renderers to extract headers and populate
SidebarTocHeadersin the render context - Implemented frontend JavaScript for sidebar positioning, toggle functionality, and localStorage persistence
- Added responsive CSS styling with mobile support and positioned floating panel
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/markup/render.go | Replaced SidebarTocNode with SidebarTocHeaders slice and defined new Header type, removed unused goldmark/ast import |
| modules/markup/sidebar_toc.go | New file implementing HTML generation for TOC sidebar with RenderSidebarTocHTML function |
| modules/markup/markdown/goldmark.go | Modified to populate SidebarTocHeaders slice instead of creating AST node for sidebar TOC |
| modules/markup/orgmode/orgmode.go | Added header extraction from org-mode document outline to populate SidebarTocHeaders |
| routers/web/repo/view.go | Added renderSidebarTocHTML helper function for TOC rendering |
| routers/web/repo/view_file.go | Integrated TOC rendering for individual markdown file views |
| routers/web/repo/view_readme.go | Integrated TOC rendering for README files in repository home |
| routers/web/repo/wiki.go | Updated wiki to use new RenderSidebarTocHTML approach |
| web_src/js/features/file-view.ts | Implemented sidebar toggle, positioning logic with IntersectionObserver/ResizeObserver, and localStorage persistence |
| web_src/css/repo/home.css | Added styling for floating TOC sidebar panel including responsive design and positioning |
| web_src/css/repo.css | Added base button styling for toggle button |
| templates/repo/view_file.tmpl | Added TOC toggle button and sidebar panel markup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .repo-view-content.sidebar-visible { | ||
| margin-right: 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
The CSS for the sidebar panel positions it with right: 0.5rem initially (line 119), but then the JavaScript overrides this with left positioning (line 95 in file-view.ts). On mobile devices (max-width: 768px), the sidebar is completely hidden with display: none !important (line 199). However, on tablet-sized devices between 768px and larger screens, there may be layout issues if the viewport isn't wide enough to accommodate both the content and the 270px sidebar margin.
Consider adding a media query for tablet sizes (e.g., 768px to 1024px) to either reduce the sidebar width, adjust margins, or provide a different layout strategy for these intermediate screen sizes.
| } | |
| } | |
| /* Adjust sidebar layout on tablet-sized screens to prevent layout issues */ | |
| @media (min-width: 769px) and (max-width: 1024px) { | |
| /* Slightly reduce sidebar width in the grid layout */ | |
| .repo-grid-filelist-sidebar { | |
| grid-template-columns: auto 220px; | |
| } | |
| /* Match reserved content margin to the narrower sidebar */ | |
| .repo-view-content.sidebar-visible { | |
| margin-right: 220px; | |
| } | |
| } |
| localStorage.setItem('file-view-sidebar-visible', 'false'); | ||
| } else { | ||
| showSidebar(); | ||
| localStorage.setItem('file-view-sidebar-visible', 'true'); |
There was a problem hiding this comment.
The localStorage access lacks error handling. While localStorage is generally reliable, it can throw exceptions in several scenarios: when storage is full, when cookies/storage are disabled in the browser, or in private browsing modes in some browsers. If an exception occurs during localStorage.getItem() or localStorage.setItem(), it will cause the entire initialization function to fail.
Consider wrapping the localStorage access in try-catch blocks to gracefully handle these edge cases and fall back to a default state (e.g., sidebar hidden) when localStorage is unavailable.
| localStorage.setItem('file-view-sidebar-visible', 'false'); | |
| } else { | |
| showSidebar(); | |
| localStorage.setItem('file-view-sidebar-visible', 'true'); | |
| try { | |
| localStorage.setItem('file-view-sidebar-visible', 'false'); | |
| } catch { | |
| // Ignore storage errors (e.g., disabled or full localStorage) | |
| } | |
| } else { | |
| showSidebar(); | |
| try { | |
| localStorage.setItem('file-view-sidebar-visible', 'true'); | |
| } catch { | |
| // Ignore storage errors (e.g., disabled or full localStorage) | |
| } |
| func RenderSidebarTocHTML(headers []Header, lang string) template.HTML { | ||
| if len(headers) == 0 { | ||
| return "" | ||
| } | ||
|
|
||
| var sb strings.Builder | ||
|
|
||
| // Start with <details open> | ||
| sb.WriteString(`<details open>`) | ||
| sb.WriteString(`<summary>`) | ||
| sb.WriteString(html.EscapeString(translation.NewLocale(lang).TrString("toc"))) | ||
| sb.WriteString(`</summary>`) | ||
|
|
||
| // Find the minimum level to start with | ||
| minLevel := 6 | ||
| for _, header := range headers { | ||
| if header.Level < minLevel { | ||
| minLevel = header.Level | ||
| } | ||
| } | ||
|
|
||
| // Build nested list structure | ||
| currentLevel := minLevel | ||
| sb.WriteString(`<ul>`) | ||
| openLists := 1 | ||
|
|
||
| for _, header := range headers { | ||
| // Close lists if we need to go up levels | ||
| for currentLevel > header.Level { | ||
| sb.WriteString(`</ul>`) | ||
| openLists-- | ||
| currentLevel-- | ||
| } | ||
|
|
||
| // Open new lists if we need to go down levels | ||
| for currentLevel < header.Level { | ||
| sb.WriteString(`<ul>`) | ||
| openLists++ | ||
| currentLevel++ | ||
| } | ||
|
|
||
| // Write the list item with link | ||
| sb.WriteString(`<li>`) | ||
| sb.WriteString(`<a href="#`) | ||
| sb.WriteString(url.QueryEscape(header.ID)) | ||
| sb.WriteString(`">`) | ||
| sb.WriteString(html.EscapeString(header.Text)) | ||
| sb.WriteString(`</a>`) | ||
| sb.WriteString(`</li>`) | ||
| } | ||
|
|
||
| // Close all remaining open lists | ||
| for openLists > 0 { | ||
| sb.WriteString(`</ul>`) | ||
| openLists-- | ||
| } | ||
|
|
||
| sb.WriteString(`</details>`) | ||
|
|
||
| return template.HTML(sb.String()) | ||
| } |
There was a problem hiding this comment.
The RenderSidebarTocHTML function lacks test coverage. Given that this is a new file and the repository has comprehensive test coverage for similar modules (as seen in modules/markup/html_test.go, modules/markup/markdown/markdown_test.go, and modules/markup/orgmode/orgmode_test.go), a test file should be added to verify the HTML generation logic, especially for edge cases like empty headers, single-level headers, multi-level headers, and headers with special characters.
Consider adding modules/markup/sidebar_toc_test.go with test cases covering various header level combinations and special characters in header text and IDs.
| // Use many thresholds to get fine-grained position updates during scroll | ||
| const thresholds = Array.from({length: 101}, (_, i) => i / 100); |
There was a problem hiding this comment.
The IntersectionObserver is created with 101 threshold values (from 0.00 to 1.00 in 0.01 increments) which may cause performance issues. Creating an intersection observer with so many thresholds means the callback will fire very frequently during scrolling, potentially causing layout thrashing and reduced scrolling performance, especially on lower-end devices.
Consider reducing the number of thresholds or using a simpler approach like a throttled scroll event listener or fewer thresholds (e.g., [0, 0.25, 0.5, 0.75, 1]).
| // Use many thresholds to get fine-grained position updates during scroll | |
| const thresholds = Array.from({length: 101}, (_, i) => i / 100); | |
| // Use a small set of thresholds to get periodic position updates during scroll | |
| const thresholds = [0, 0.25, 0.5, 0.75, 1]; |
| const resizeObserver = new ResizeObserver(() => { | ||
| updatePosition(); | ||
| }); | ||
| resizeObserver.observe(document.body); |
There was a problem hiding this comment.
The ResizeObserver is observing document.body which will trigger on any size change to the entire document body. This is overly broad and may cause unnecessary position updates when unrelated parts of the page resize (e.g., when a modal opens, when content loads elsewhere on the page, etc.).
Consider observing only the specific elements that affect the sidebar position (e.g., the file view container, the segment element, or the file header) to reduce unnecessary callback invocations.
| // Restore saved state from localStorage (default to hidden) | ||
| const savedState = localStorage.getItem('file-view-sidebar-visible'); | ||
| const isVisible = savedState === 'true'; // default to hidden | ||
|
|
||
| // Apply initial state | ||
| if (isVisible) { | ||
| showSidebar(); | ||
| } else { | ||
| hideSidebar(); | ||
| } |
There was a problem hiding this comment.
There's a potential race condition in the sidebar visibility initialization. The code checks localStorage and then calls showSidebar() or hideSidebar() synchronously (lines 156-160), but showSidebar() uses a timeout of 220ms (line 134) for file views. During this delay, if the user quickly toggles the sidebar or if the page navigates away, the delayed callback might execute on the wrong state or after cleanup.
Consider checking if the component is still mounted/active before executing the delayed callbacks, or use a more robust state management approach.
| // Render sidebar TOC | ||
| if len(rctx.SidebarTocHeaders) > 0 { | ||
| ctx.Data["WikiSidebarTocHTML"] = markup.RenderSidebarTocHTML(rctx.SidebarTocHeaders, ctx.Locale.Language()) |
There was a problem hiding this comment.
The wiki TOC rendering (line 282) is inconsistent with the file/readme TOC rendering. In wiki.go, the code directly checks len(rctx.SidebarTocHeaders) > 0 before calling RenderSidebarTocHTML, but in view_file.go (line 96) and view_readme.go (line 210), it calls the renderSidebarTocHTML helper which performs the same check. This inconsistency makes the codebase harder to maintain.
Consider either using the helper function consistently across all three places (wiki, file, readme) or directly calling markup.RenderSidebarTocHTML in all three places for consistency.
templates/repo/view_file.tmpl
Outdated
| </div> | ||
| <div class="file-header-right file-actions flex-text-block tw-flex-wrap"> | ||
| {{if .FileSidebarHTML}} | ||
| <button class="btn-octicon" id="toggle-sidebar-btn" data-tooltip-content="{{ctx.Locale.Tr "toc"}}">{{svg "octicon-list-unordered" 15}}</button> |
There was a problem hiding this comment.
The toggle button lacks proper ARIA attributes for accessibility. While it has a tooltip via data-tooltip-content, it should also have aria-label or aria-labelledby to ensure screen readers properly announce its purpose. Additionally, the button should have aria-expanded attribute that reflects whether the sidebar is currently visible or hidden, and this should be updated when the toggle state changes.
Consider adding aria-label="{{ctx.Locale.Tr "toc"}}" and aria-expanded="false" attributes to the button, and update the aria-expanded value in the JavaScript when toggling the sidebar visibility.
| <button class="btn-octicon" id="toggle-sidebar-btn" data-tooltip-content="{{ctx.Locale.Tr "toc"}}">{{svg "octicon-list-unordered" 15}}</button> | |
| <button class="btn-octicon" id="toggle-sidebar-btn" data-tooltip-content="{{ctx.Locale.Tr "toc"}}" aria-label="{{ctx.Locale.Tr "toc"}}" aria-expanded="false">{{svg "octicon-list-unordered" 15}}</button> |
| // Wait for CSS transition to complete (200ms) before calculating position | ||
| setTimeout(showAfterLayout, 220); |
There was a problem hiding this comment.
The hardcoded timeout value of 220ms (200ms transition + 20ms buffer) is fragile and tightly coupled to the CSS transition duration. If the CSS transition duration in web_src/css/repo/home.css line 79 changes, this code will break or behave incorrectly.
Consider using a named constant that can be shared or referenced, or better yet, use the transitionend event to know when the CSS transition has actually completed, which is more reliable and maintainable.
| // Wait for CSS transition to complete (200ms) before calculating position | |
| setTimeout(showAfterLayout, 220); | |
| // Wait for CSS transition to actually complete before calculating position | |
| const onTransitionEnd = (event: TransitionEvent) => { | |
| if (event.target !== repoViewContent) return; | |
| repoViewContent.removeEventListener('transitionend', onTransitionEnd); | |
| showAfterLayout(); | |
| }; | |
| repoViewContent.addEventListener('transitionend', onTransitionEnd); |
| // Parse the document first to extract outline for TOC | ||
| doc := org.New().Silent().Parse(input, "") | ||
| if doc.Error != nil { | ||
| return fmt.Errorf("orgmode.Parse failed: %w", doc.Error) | ||
| } | ||
|
|
||
| // Extract headers from the document outline for sidebar TOC | ||
| ctx.SidebarTocHeaders = extractHeadersFromOutline(doc.Outline) | ||
|
|
||
| res, err := doc.Write(w) | ||
| if err != nil { | ||
| return fmt.Errorf("orgmode.Render failed: %w", err) | ||
| } | ||
| _, err = io.Copy(output, strings.NewReader(res)) | ||
| return err | ||
| } | ||
|
|
||
| // extractHeadersFromOutline recursively extracts headers from org document outline | ||
| func extractHeadersFromOutline(outline org.Outline) []markup.Header { | ||
| var headers []markup.Header | ||
| collectHeaders(outline.Section, &headers) | ||
| return headers | ||
| } | ||
|
|
||
| // collectHeaders recursively collects headers from sections | ||
| func collectHeaders(section *org.Section, headers *[]markup.Header) { | ||
| if section == nil { | ||
| return | ||
| } | ||
|
|
||
| // Process current section's headline | ||
| if section.Headline != nil { | ||
| h := section.Headline | ||
| // Convert headline title nodes to plain text | ||
| titleText := org.String(h.Title...) | ||
| *headers = append(*headers, markup.Header{ | ||
| Level: h.Lvl, | ||
| Text: titleText, | ||
| ID: h.ID(), | ||
| }) | ||
| } | ||
|
|
||
| // Process child sections | ||
| for _, child := range section.Children { | ||
| collectHeaders(child, headers) | ||
| } | ||
| } |
There was a problem hiding this comment.
The orgmode rendering implementation lacks test coverage for the new TOC header extraction functionality. While the file has existing tests in orgmode_test.go, there are no tests verifying that extractHeadersFromOutline and collectHeaders correctly extract headers from org-mode documents, or that the ctx.SidebarTocHeaders is properly populated.
Consider adding test cases in modules/markup/orgmode/orgmode_test.go to verify that headers are correctly extracted from org-mode documents with various heading structures.
- Remove border from the TOC button in the README view for a cleaner appearance. - Ensure hover state maintains no border for consistency with the overall design.
3 participants
for this issue #36249