feat(docs): improve sidebar nav with filter and scroll fades#497
Conversation
Rework the docs sidebar so category titles link to overview pages, add smart filtering, and keep the nav list scrollable with edge fades. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRewrites DocsSidebarNav to add a "docs" filterable variant and scroll-edge overlays, introduces variant/className props, sets header height via --site-header-height and sticky aside sizing, updates docs config to exclude category index pages and removes some icons, and passes variant="plain" in the blog layout. ChangesDocs sidebar search and filtering
Sequence Diagram(s)sequenceDiagram
participant DocsSidebarNav
participant useSidebarScrollEdges
participant DocsSidebarNavItems
DocsSidebarNav->>useSidebarScrollEdges: enable when variant="docs"
useSidebarScrollEdges->>useSidebarScrollEdges: observe scroll/resize -> compute edges
DocsSidebarNav->>DocsSidebarNavItems: pass filteredItems, query, depth
DocsSidebarNavItems->>DocsSidebarNavItems: recursively filter and render nested lists
useSidebarScrollEdges-->>DocsSidebarNav: edges.top / edges.bottom
DocsSidebarNav->>DocsSidebarNav: render gradient overlays based on edges
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Keep nested changelog entries for search and mobile nav only. Co-authored-by: Cursor <cursoragent@cursor.com>
Deploying animata with
|
| Latest commit: |
3f8570e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://043634fb.animata.pages.dev |
| Branch Preview URL: | https://feat-docs-sidebar.animata.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/(main)/layout.tsx (1)
14-17: 💤 Low valueConsider TypeScript typing for CSS custom properties.
The inline
styleobject with--site-header-heightmay cause TypeScript errors depending on yourtsconfig.jsonsettings. If you encounter type errors, you can type-cast the style object.💡 Optional TypeScript fix
-<div - style={{ "--site-header-height": "50px" }} - className="relative flex min-h-screen flex-col bg-background" -> +<div + style={{ "--site-header-height": "50px" } as React.CSSProperties} + className="relative flex min-h-screen flex-col bg-background" +>Or define a type:
style={{ "--site-header-height": "50px" } as React.CSSProperties & { "--site-header-height": string; }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(main)/layout.tsx around lines 14 - 17, The inline style object setting the CSS custom property "--site-header-height" can trigger TypeScript errors; update the style prop on the div (the element using style={{ "--site-header-height": "50px" }}) by casting the object to React.CSSProperties combined with a custom index for that property (e.g., React.CSSProperties & { "--site-header-height": string }) or otherwise type-asserting it so TypeScript accepts the custom property; modify the style expression in layout.tsx where the div is defined to use that cast.components/sidebar-nav.tsx (1)
19-19: ⚖️ Poor tradeoffMagic number creates fragile dependency on config ordering.
SPECIAL_HEADER_COUNT = 2assumes the first two sidebar items are always "Getting Started" and "Contributing". Ifconfig/docs.tssidebar order changes, the "Components" divider (line 259) and badge rendering logic (line 235) will break.Consider computing this dynamically by checking item titles or using a more explicit configuration flag on sidebar items.
🔧 Suggested approach
-const SPECIAL_HEADER_COUNT = 2; +function isSpecialHeader(item: SidebarNavItem) { + return ["Getting Started", "Contributing"].includes(item.title); +}Then update usage:
-{hasChildren && index >= SPECIAL_HEADER_COUNT ? ( +{hasChildren && !isSpecialHeader(item) ? (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/sidebar-nav.tsx` at line 19, Replace the hard-coded SPECIAL_HEADER_COUNT with a dynamic computation: inspect the sidebar items array (used where SPECIAL_HEADER_COUNT was referenced) and compute the count by finding items whose title matches the special headers (e.g., "Getting Started" and "Contributing") or, better, by checking a new explicit property (e.g., isSpecialHeader) on sidebar item objects; then use that computed count in the badge rendering logic and the "Components" divider logic (the places currently referencing SPECIAL_HEADER_COUNT) so ordering changes in config/docs.ts won't break rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/sidebar-nav.tsx`:
- Around line 235-239: The badge elements (the span rendered when hasChildren &&
index >= SPECIAL_HEADER_COUNT and the "new" label badges) use hardcoded Tailwind
colors (bg-gray-200, bg-lime-300, text-[`#000000`]); update those className usages
to use theme/token classes or CSS variables instead (e.g., replace
bg-gray-200/bg-lime-300 with your design-system background token like bg-muted
or bg-accent and replace text-[`#000000`] with the theme foreground token like
text-foreground or a CSS var such as var(--foreground)); locate the span in the
component where hasChildren, SPECIAL_HEADER_COUNT, and item.label/item.items are
used and update its className, and apply the same replacement to the other "new"
badge className occurrences so dark mode and theming work correctly.
- Around line 176-178: The selector using
document.querySelector(`[data-sidebar-link="${pathname}"]`) is unsafe for
pathnames with special chars; update the logic in the sidebar-nav component to
escape the pathname before building the attribute selector (use
CSS.escape(pathname)) or select all elements with [data-sidebar-link] and filter
by el.getAttribute('data-sidebar-link')/el.dataset.sidebarLink === pathname,
then call node?.scrollIntoView(...) and updateEdges(); reference the
variables/functions node, pathname, scrollIntoView, and updateEdges when
applying the fix.
---
Nitpick comments:
In `@app/`(main)/layout.tsx:
- Around line 14-17: The inline style object setting the CSS custom property
"--site-header-height" can trigger TypeScript errors; update the style prop on
the div (the element using style={{ "--site-header-height": "50px" }}) by
casting the object to React.CSSProperties combined with a custom index for that
property (e.g., React.CSSProperties & { "--site-header-height": string }) or
otherwise type-asserting it so TypeScript accepts the custom property; modify
the style expression in layout.tsx where the div is defined to use that cast.
In `@components/sidebar-nav.tsx`:
- Line 19: Replace the hard-coded SPECIAL_HEADER_COUNT with a dynamic
computation: inspect the sidebar items array (used where SPECIAL_HEADER_COUNT
was referenced) and compute the count by finding items whose title matches the
special headers (e.g., "Getting Started" and "Contributing") or, better, by
checking a new explicit property (e.g., isSpecialHeader) on sidebar item
objects; then use that computed count in the badge rendering logic and the
"Components" divider logic (the places currently referencing
SPECIAL_HEADER_COUNT) so ordering changes in config/docs.ts won't break
rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d928e62-5bd2-4a46-ac2d-b380bc7f9418
📒 Files selected for processing (7)
app/(main)/blog/layout.tsxapp/(main)/docs/layout.tsxapp/(main)/layout.tsxcomponents/sidebar-nav.tsxcomponents/site-header.tsxconfig/docs.tsstyles/docs.css
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/sidebar-nav.tsx (2)
48-55:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFilter logic includes entire category when any descendant matches.
collectSearchParts(item)aggregates titles/hrefs from all nested children. If the query matches any descendant,categoryMatchesis true and the entire category with all children is pushed—bypassingfilterNestedItems. Users searching for "button" will see every sibling component, not just Button.Consider restricting
categoryMatchesto the category's own title/href so nested matches are handled byfilteredChildren:🐛 Proposed fix
return items.reduce<SidebarNavItem[]>((acc, item) => { - const categoryMatches = matchesFilter(trimmed, ...collectSearchParts(item)); + const categoryMatches = matchesFilter(trimmed, item.title, item.href); const filteredChildren = item.items ? filterNestedItems(item.items, trimmed) : []; if (categoryMatches) { - acc.push(item); + acc.push(item); // Show full category when category name/href matches return acc; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/sidebar-nav.tsx` around lines 48 - 55, The current reduce logic uses collectSearchParts(item) to compute categoryMatches, which includes all descendants so any child match causes the whole category (and all children) to be pushed; change it so categoryMatches only checks the category's own title/href (not collectSearchParts) and rely on filteredChildren = filterNestedItems(item.items, trimmed) to include matched descendants: if categoryMatches push the original item, else if filteredChildren.length > 0 push a clone of the item with items = filteredChildren; update references in the reducer (categoryMatches, filteredChildren, items.reduce) and ensure collectSearchParts remains used by filterNestedItems (not by the top-level category match).
21-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegex escapes
\\sincorrectly—whitespace won't be normalized.In a JavaScript regex literal,
\\smatches a literal backslash followed bys, not whitespace. The filter will fail to collapse spaces, tabs, etc.🐛 Proposed fix
function normalizeForFilter(value: string) { return value .toLowerCase() - .replace(/[-_/\\s]+/g, " ") + .replace(/[-_/\s]+/g, " ") .trim(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/sidebar-nav.tsx` around lines 21 - 26, The regex in normalizeForFilter is escaping \s incorrectly (using \\s), so whitespace isn't matched; update the character class to use an actual whitespace escape and escape the literal forward slash, e.g. replace /[-_/\\s]+/g with /[-_\/\s]+/g in the normalizeForFilter function so spaces, tabs, and other whitespace are normalized correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@components/sidebar-nav.tsx`:
- Around line 48-55: The current reduce logic uses collectSearchParts(item) to
compute categoryMatches, which includes all descendants so any child match
causes the whole category (and all children) to be pushed; change it so
categoryMatches only checks the category's own title/href (not
collectSearchParts) and rely on filteredChildren = filterNestedItems(item.items,
trimmed) to include matched descendants: if categoryMatches push the original
item, else if filteredChildren.length > 0 push a clone of the item with items =
filteredChildren; update references in the reducer (categoryMatches,
filteredChildren, items.reduce) and ensure collectSearchParts remains used by
filterNestedItems (not by the top-level category match).
- Around line 21-26: The regex in normalizeForFilter is escaping \s incorrectly
(using \\s), so whitespace isn't matched; update the character class to use an
actual whitespace escape and escape the literal forward slash, e.g. replace
/[-_/\\s]+/g with /[-_\/\s]+/g in the normalizeForFilter function so spaces,
tabs, and other whitespace are normalized correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b168b53-dbc4-451e-8e81-328e2004cb79
📒 Files selected for processing (1)
components/sidebar-nav.tsx
Use title-based special header checks, theme tokens for badges, and a safe sidebar link lookup instead of an interpolated attribute selector. Co-authored-by: Cursor <cursoragent@cursor.com>
Rework the docs sidebar so category titles link to overview pages, add smart filtering, and keep the nav list scrollable with edge fades.
Summary by CodeRabbit
New Features
Improvements
Chores