-
Notifications
You must be signed in to change notification settings - Fork 536
blog: add table of contents to left sidebar #3866
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
Co-Authored-By: [email protected] <[email protected]>
✅ Deploy Preview for hyprnote-storybook canceled.
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
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.
| <aside | ||
| className={cn([ | ||
| "hidden xl:flex fixed left-0 top-0 h-screen z-10", | ||
| "w-64 items-center", | ||
| ])} |
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.
🔴 Fixed full-height aside blocks pointer events on blog content for common desktop viewports
The TableOfContents renders a fixed left-0 top-0 h-screen z-10 w-64 aside that covers the left 256px of the viewport at z-index 10, but the centered blog content (max-w-6xl mx-auto = 1152px) has no left offset to account for it.
Root cause and impact analysis
On viewports between 1280px (the xl breakpoint where the aside becomes visible) and ~1664px (where the margin becomes wide enough), the aside overlaps the blog content. At 1280px, (1280−1152)/2 = 64px margin on each side, so 256−64 = 192px of content is covered. At 1440px (common laptop), the overlap is still 256−144 = 112px.
Because the aside element has no pointer-events: none and sits at z-10, it captures all mouse events across the full h-screen × w-64 area — not just the 200px-tall nav in the center. This means:
- Users cannot click links, select text, or interact with the left portion of blog content on screens 1280–1664px wide.
- The
navelement (200px tall, centered) additionally hasonWheel={handleWheel}withe.preventDefault()/e.stopPropagation(), which blocks page scrolling when the cursor is over that region — even though it visually appears to be over blog content.
This affects the most common desktop/laptop resolutions (1366px, 1440px, 1536px). The docs RightSidebar this was modeled after does not have this problem because it uses an in-flow layout (hidden lg:block w-64 shrink-0) rather than position: fixed.
Prompt for agents
The aside needs pointer-events: none on the container so clicks pass through to the content beneath it, with pointer-events: auto restored on the interactive nav child. Additionally, consider only showing the TOC at a wider breakpoint (e.g. 2xl / 1536px) or adding a left margin/padding to the content area to prevent visual overlap. Here are the specific changes:
1. In apps/web/src/routes/_view/blog/$slug.tsx, on the aside element (around line 429-433), add pointer-events-none to the className array so the transparent area doesn't block clicks:
"hidden xl:flex fixed left-0 top-0 h-screen z-10",
"w-64 items-center pointer-events-none",
2. On the nav element (around line 435-438), add pointer-events-auto to re-enable interactions on the actual TOC:
className="relative w-full overflow-hidden cursor-ns-resize pointer-events-auto"
3. Consider changing the breakpoint from xl to 2xl (or a custom min-width) to avoid visual overlap with the centered content on common laptop screens, or add responsive left padding to the content container.
Was this helpful? React with 👍 or 👎 to provide feedback.
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.
| const scrollToHeading = useCallback((id: string) => { | ||
| document.getElementById(id)?.scrollIntoView({ | ||
| behavior: "smooth", | ||
| block: "start", | ||
| }); | ||
| }, []); |
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.
🔴 BlogTocSubBar's scrollToHeading doesn't update activeId or suppress IntersectionObserver
When the user taps prev/next in the mobile BlogTocSubBar, it calls blogToc.scrollToHeading which is the context's version from route.tsx:41-46. This version only calls scrollIntoView — it does not update activeId or set isUserScrollingToc to suppress the IntersectionObserver.
Root Cause and Impact
The BlogTocContext provides a simple scrollToHeading (apps/web/src/routes/_view/route.tsx:41-46):
const scrollToHeading = useCallback((id: string) => {
document.getElementById(id)?.scrollIntoView({ behavior: "smooth", block: "start" });
}, []);Meanwhile, the TableOfContents component has its own richer scrollToHeading (apps/web/src/routes/_view/blog/$slug.tsx:287-301) that also:
- Sets
isUserScrollingToc = trueto suppress the IntersectionObserver during scroll - Calls
setActiveId(id)to immediately update the active heading
The BlogTocSubBar in header.tsx:810 destructures scrollToHeading from the context (blogToc), getting the simple version. When the user taps prev/next (header.tsx:814-822), the sub-bar heading text won't update immediately because activeId isn't set. Additionally, since isUserScrollingToc isn't set, the IntersectionObserver fires during the smooth scroll animation, causing the active heading to flicker through intermediate headings before settling on the target.
Impact: On mobile, the TOC sub-bar heading text flickers or doesn't update promptly when navigating between headings via the prev/next buttons.
Prompt for agents
The context's scrollToHeading in apps/web/src/routes/_view/route.tsx:41-46 needs to also update the activeId state when called. The simplest fix is to have it call setBlogActiveId(id) before scrollIntoView. Change the scrollToHeading callback to:
const scrollToHeading = useCallback((id: string) => {
setBlogActiveId(id);
document.getElementById(id)?.scrollIntoView({
behavior: "smooth",
block: "start",
});
}, []);
This ensures the sub-bar heading text updates immediately. However, this still doesn't suppress the IntersectionObserver during the scroll animation (the isUserScrollingToc flag is local to TableOfContents). For a complete fix, consider either: (1) lifting the isUserScrollingToc flag into the context, or (2) having the BlogTocSubBar call a shared scrollToHeading that the TableOfContents component provides to the context instead of the simple one from route.tsx.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Adds a table of contents (TOC) to the left sidebar of blog post pages. The TOC data was already being extracted in
content-collections.tsviaextractToc()and included in the article transform — this PR renders it in the blog layout.The implementation mirrors the existing docs
RightSidebarTOC pattern: sticky nav, indentation by heading level, hidden belowlgbreakpoint.Changes:
TableOfContentscomponent on the leftTableOfContentsrenders heading links (h2–h4) with indentation, returns null if no headings existReview & Testing Checklist for Human
top-21.25): Confirm the TOC doesn't overlap or sit too far from the site header when scrolling — this value was copied from docs and may need adjustment if the blog page header height differsNotes