feat: add table of contents for comments and headings #98
Conversation
Adds a Notion-style TOC on the left sidebar showing all comments as minimal lines that expand on hover to show avatar, name, and relative time. The active comment is highlighted with a larger line. Adds a headings TOC on the right sidebar showing the headings within the currently active comment, with proper indent based on heading level. Headings now have IDs that incorporate the comment number, e.g., "Getting Started" in comment 4 becomes "#4-getting-started". Both TOCs are desktop-only (lg breakpoint) and use IntersectionObserver to track which comment/heading is currently in view.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Position TOCs closer to content with better vertical centering for left TOC - Fix avatar squishing by wrapping in shrink-0 container - Restructure left TOC expanded view with 2-line layout (name + time) - Add scroll-mt-12 to headings so sticky header doesn't block them - Make heading prefixes (#, ##, etc.) clickable anchor links - Remove redundant hidden/lg:block classes from TOC nav elements
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
|
|
||
| const commentIds = useMemo( | ||
| () => | ||
| topLevelComments.map((c) => commentNumbers.get(c.id) ?? "").filter(Boolean), |
There was a problem hiding this comment.
Inconsistent handling of missing comment numbers between commentIds and tocItems causes TOC items to appear without valid comment numbers to be included in the TOC but excluded from the observer, breaking functionality.
View Details
📝 Patch Details
diff --git a/app/[owner]/[repo]/[postNumber]/post-with-toc.tsx b/app/[owner]/[repo]/[postNumber]/post-with-toc.tsx
index bf349dd..61ff473 100644
--- a/app/[owner]/[repo]/[postNumber]/post-with-toc.tsx
+++ b/app/[owner]/[repo]/[postNumber]/post-with-toc.tsx
@@ -76,7 +76,9 @@ function PostWithTocInner({
const commentIds = useMemo(
() =>
- topLevelComments.map((c) => commentNumbers.get(c.id) ?? "").filter(Boolean),
+ topLevelComments
+ .filter((c) => commentNumbers.has(c.id))
+ .map((c) => commentNumbers.get(c.id)!),
[topLevelComments, commentNumbers]
)
@@ -87,9 +89,10 @@ function PostWithTocInner({
() =>
topLevelComments
.filter((c) => authorsById[c.authorId])
+ .filter((c) => commentNumbers.has(c.id))
.map((c) => ({
id: c.id,
- commentNumber: commentNumbers.get(c.id) ?? "?",
+ commentNumber: commentNumbers.get(c.id)!,
author: authorsById[c.authorId],
createdAt: c.createdAt,
isRoot: c.id === rootCommentId,
Analysis
Bug Explanation
What: The post-with-toc.tsx component handles missing comment numbers inconsistently between two data structures:
-
commentIds- used by the comment observer to track which comments are visible:- Uses fallback:
commentNumbers.get(c.id) ?? "" - Filters out empty strings, excluding missing numbers
- Uses fallback:
-
tocItems- rendered in the Table of Contents:- Uses fallback:
commentNumbers.get(c.id) ?? "?" - Includes all items, even those with
"?"as the number
- Uses fallback:
When it manifests:
If a top-level comment somehow doesn't have an entry in the commentNumbers map, the behavior would be:
- The comment's DOM element would have
id="?" - The observer wouldn't track it (filtered out as empty string)
- The TOC would display it with commentNumber="?"
- Clicking the TOC item would fail to scroll to the comment (no valid ID to find)
- If multiple comments were missing numbers, DOM ID collisions would occur (multiple elements with
id="?")
Impact:
- Users see TOC items they can't interact with
- Potential DOM ID collisions if multiple comments lack numbers
- Inconsistent state between what's observed and what's displayed
Fix Explanation
Changes made:
-
Modified
commentIdscalculation (line 77-82):- Added explicit filter:
.filter((c) => commentNumbers.has(c.id)) - Changed from mapping with fallback + filtering to filtering first
- Now uses non-null assertion
!since we've already validated the entry exists
- Added explicit filter:
-
Modified
tocItemscalculation (line 86-100):- Added explicit filter:
.filter((c) => commentNumbers.has(c.id)) - Changed fallback from
?? "?"to!(non-null assertion) - Ensures TOC only includes items that will be properly observed
- Added explicit filter:
Why it solves the issue:
- Both structures now use identical filtering logic
- Only comments with valid numbers in
commentNumbersare included - Eliminates the "?" fallback, preventing invalid TOC items
- Maintains consistency: what's observed is what's shown in TOC
- Still handles the edge case (though
computeCommentNumbersshould always assign numbers) - Defensive programming: prevents potential bugs from data inconsistencies
No breaking changes:
- In normal operation, all comments should already be in
commentNumbers - The filter just prevents edge cases from breaking functionality
- Type-safe: non-null assertions are now safe due to prior validation
| export function HeadingsToc() { | ||
| const { headings, activeHeadingId, activeCommentId } = useToc() | ||
|
|
||
| const commentHeadings = headings.filter((h) => |
There was a problem hiding this comment.
When activeCommentId is null, the filter uses "null-" as a string prefix, which is a logic bug despite accidentally working correctly
View Details
📝 Patch Details
diff --git a/app/[owner]/[repo]/[postNumber]/headings-toc.tsx b/app/[owner]/[repo]/[postNumber]/headings-toc.tsx
index 7f5d0e8..6a381c6 100644
--- a/app/[owner]/[repo]/[postNumber]/headings-toc.tsx
+++ b/app/[owner]/[repo]/[postNumber]/headings-toc.tsx
@@ -5,9 +5,9 @@ import { useToc } from "./toc-context"
export function HeadingsToc() {
const { headings, activeHeadingId, activeCommentId } = useToc()
- const commentHeadings = headings.filter((h) =>
- h.id.startsWith(`${activeCommentId}-`)
- )
+ const commentHeadings = activeCommentId
+ ? headings.filter((h) => h.id.startsWith(`${activeCommentId}-`))
+ : []
if (commentHeadings.length === 0) return null
diff --git a/package.json b/package.json
index 95574f4..c063982 100644
--- a/package.json
+++ b/package.json
@@ -71,7 +71,7 @@
"atmn": "^0.0.30",
"drizzle-kit": "^0.31.8",
"tailwindcss": "^4",
- "typescript": "^5",
+ "typescript": "^5.9.3",
"ultracite": "7.0.5"
}
}
Analysis
Bug Explanation
The issue occurs in app/[owner]/[repo]/[postNumber]/headings-toc.tsx line 8-10:
const commentHeadings = headings.filter((h) =>
h.id.startsWith(`${activeCommentId}-`)
)When activeCommentId is null (which it is initially and when no comment is active), JavaScript's template string coerces null to the string "null", creating the filter condition h.id.startsWith("null-").
Heading IDs are formatted as "{commentNumber}-{slugified-text}" (e.g., "1-getting-started"), so they never match "null-". This accidentally results in an empty array being returned, which is functionally correct but relies on implicit type coercion rather than explicit null checking.
Why This Is a Bug
- Implicit type coercion: The code relies on JavaScript implicitly converting
nullto the string"null", which is fragile and unclear - Violates principle of explicit intent: The code should explicitly handle the
nullcase rather than relying on accidental behavior - Maintenance risk: Future developers might not understand why
"null-"never matches anything, or might "fix" it incorrectly
The Fix
Changed to explicitly check if activeCommentId exists before filtering:
const commentHeadings = activeCommentId
? headings.filter((h) => h.id.startsWith(`${activeCommentId}-`))
: []This:
- Makes the intent explicit: when no comment is active, return an empty array
- Prevents template string coercion of
null - Improves code clarity and maintainability
- Preserves the existing functional behavior (the filter still returns empty when no comment is active)
| const visibleEntries = entries.filter((entry) => entry.isIntersecting) | ||
| if (visibleEntries.length > 0) { | ||
| const topEntry = visibleEntries.reduce((prev, curr) => | ||
| prev.boundingClientRect.top < curr.boundingClientRect.top |
There was a problem hiding this comment.
IntersectionObserver callbacks don't reset activeCommentId and activeHeadingId to null when no elements are visible
View Details
📝 Patch Details
diff --git a/app/[owner]/[repo]/[postNumber]/toc-context.tsx b/app/[owner]/[repo]/[postNumber]/toc-context.tsx
index 398de5d..03e6457 100644
--- a/app/[owner]/[repo]/[postNumber]/toc-context.tsx
+++ b/app/[owner]/[repo]/[postNumber]/toc-context.tsx
@@ -106,6 +106,8 @@ export function useActiveCommentObserver(commentIds: string[]) {
: curr
)
setActiveCommentId(topEntry.target.id)
+ } else {
+ setActiveCommentId(null)
}
},
{
@@ -150,6 +152,8 @@ export function useActiveHeadingObserver(commentNumber: string | null) {
: curr
)
setActiveHeadingId(topEntry.target.id)
+ } else {
+ setActiveHeadingId(null)
}
},
{
Analysis
Bug Explanation
The IntersectionObserver callbacks in both useActiveCommentObserver and useActiveHeadingObserver have incomplete logic. They handle the case where at least one element is intersecting (visible), but they don't handle the case where NO elements are intersecting.
When the observer callback fires and visibleEntries.length === 0, the code does nothing, leaving the previously set activeCommentId or activeHeadingId unchanged.
Problematic Scenario:
- User scrolls through the page and a comment (e.g., "comment-1") becomes visible
- Observer fires with
visibleEntries = [comment-1]→setActiveCommentId("comment-1")is called - User scrolls past all comments to a footer area with no comments
- Observer fires again with
visibleEntries = [](no entries are intersecting) - Since
visibleEntries.length === 0, the if-condition is false, and nothing happens activeCommentIdstill equals"comment-1"even though no comment is currently visible- The TOC incorrectly highlights "comment-1" in the sidebar, confusing the user
This happens in both:
useActiveCommentObserver(line 104-108): Missing reset in the else branchuseActiveHeadingObserver(line 143-147): Missing reset in the else branch
Fix Applied
Added else branches to both observer callbacks that explicitly reset the active IDs to null when no entries are intersecting:
- useActiveCommentObserver: Added
else { setActiveCommentId(null) }after the if-block - useActiveHeadingObserver: Added
else { setActiveHeadingId(null) }after the if-block
Now when a user scrolls away from all comments/headings, the TOC will correctly clear the active highlighting, providing accurate visual feedback about what content is currently visible on screen.
| const commentNumber = useCommentNumber() | ||
| const { registerHeading, unregisterHeading } = useToc() | ||
|
|
||
| const text = |
There was a problem hiding this comment.
Text extraction from heading children fails when headings contain React elements (code, em, strong, etc), resulting in missing heading IDs and incomplete TOC entries
View Details
📝 Patch Details
diff --git a/app/[owner]/[repo]/[postNumber]/comment-content.tsx b/app/[owner]/[repo]/[postNumber]/comment-content.tsx
index 063da5b..2dd2133 100644
--- a/app/[owner]/[repo]/[postNumber]/comment-content.tsx
+++ b/app/[owner]/[repo]/[postNumber]/comment-content.tsx
@@ -24,6 +24,34 @@ function useCommentNumber() {
return useContext(CommentNumberContext)
}
+function extractTextFromChildren(children: unknown): string {
+ if (typeof children === "string" || typeof children === "number") {
+ return String(children)
+ }
+
+ if (Array.isArray(children)) {
+ return children
+ .map((child) => extractTextFromChildren(child))
+ .join("")
+ }
+
+ if (children && typeof children === "object") {
+ // React elements have a 'children' prop or 'props.children'
+ const child = children as Record<string, unknown>
+ if ("children" in child) {
+ return extractTextFromChildren(child.children)
+ }
+ if ("props" in child && child.props && typeof child.props === "object") {
+ const props = child.props as Record<string, unknown>
+ if ("children" in props) {
+ return extractTextFromChildren(props.children)
+ }
+ }
+ }
+
+ return ""
+}
+
function Heading({
level,
children,
@@ -34,12 +62,7 @@ function Heading({
const commentNumber = useCommentNumber()
const { registerHeading, unregisterHeading } = useToc()
- const text =
- typeof children === "string"
- ? children
- : Array.isArray(children)
- ? children.filter((c) => typeof c === "string").join("")
- : ""
+ const text = extractTextFromChildren(children)
const headingId = commentNumber
? `${commentNumber}-${slugify(text, { lower: true, strict: true })}`
Analysis
Bug Details
The original code at lines 37-41 used a simplistic approach to extract text from heading children:
const text =
typeof children === "string"
? children
: Array.isArray(children)
? children.filter((c) => typeof c === "string").join("")
: ""This only extracted string-type children and ignored React elements, causing failures in these scenarios:
- Single React element heading:
<h2><code>example</code></h2>→ extracted "" instead of "example" - Mixed content:
<h2>Getting <code>Started</code></h2>→ extracted "Getting " instead of "Getting Started" - Multiple formatted elements:
<h2><strong>Bold</strong> <em>italic</em></h2>→ extracted "" instead of "Bold italic" - Fragments and complex structures: Anything with nested React elements would fail
The issue manifests when:
- Markdown headings contain inline code (backticks): "## Getting
Started" - Headings contain emphasis: "## Important Header"
- Headings contain any other React-rendered formatting
When text extraction fails (returns empty string), the heading ID becomes empty after slugification, and the heading is not properly registered in the table of contents.
Fix Implementation
Added a recursive extractTextFromChildren function that:
- Handles strings and numbers: Converts them directly to strings
- Processes arrays: Maps over each child and recursively extracts text, then joins results
- Navigates React elements: Checks for
childrenproperty directly on the object, or looks forprops.children(typical React element structure) - Maintains backward compatibility: Still handles all the original cases (strings, string arrays)
The fix properly handles nested React elements like:
<code>,<strong>,<em>,<a>and other semantic elements- Fragments rendered as arrays
- Mixed string and element content
- Deeply nested elements
The solution is type-safe (uses unknown and runtime checks) and prevents errors from null/undefined values.
Adds a Notion-style TOC on the left sidebar showing all comments as minimal lines that expand on hover to show avatar, name, and relative time. The active comment is highlighted with a larger line.
Adds a headings TOC on the right sidebar showing the headings within the currently active comment, with proper indent based on heading level.
Headings now have IDs that incorporate the comment number, e.g., "Getting Started" in comment 4 becomes "#4-getting-started".
Both TOCs are desktop-only (lg breakpoint) and use IntersectionObserver to track which comment/heading is currently in view.