Refactor docs nav into tabbed sections#904
Conversation
✅ Deploy Preview for tanstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds canonical docs tabs and types, heuristics and utilities to infer and group menu items into tabs, extends config/schema to accept optional ChangesTabbed Docs Navigation System
Sequence DiagramsequenceDiagram
participant User as User/Browser
participant Layout as DocsLayout
participant TabUtil as Tab Utilities
participant Menu as Menu Renderer
participant Sidebar as Sidebar Renderer
User->>Layout: Navigate to /docs/some/path
Layout->>TabUtil: getTabbedMenuConfig(menuConfig)
TabUtil->>Layout: tabbedMenuConfig
Layout->>TabUtil: getActiveDocsNavTabId({pathname, relativePathname, isExample})
TabUtil->>Layout: activeTabId
Layout->>Layout: visibleMenuConfig = tabbedMenuConfig[activeTabId] || menuConfig
Layout->>Menu: Render docsTabs (highlight activeTabId)
Layout->>Sidebar: Render sidebar from visibleMenuConfig
Menu->>User: Tab strip with active tab
Sidebar->>User: Filtered sidebar menu
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/utils/docsNavTabs.ts`:
- Around line 115-117: The current firstItem selection (variable firstItem using
groups.flatMap(group => group.children).find(child =>
!child.to.startsWith('http'))) can pick non-doc entries like ".."; change the
predicate to only select children whose to target is an internal docs path
(e.g., child.to.startsWith('/docs') or otherwise matches your docs root) and
still exclude external URLs, so update the find used to set firstItem to require
both not child.to.startsWith('http') and child.to.startsWith('/docs') (or the
project-specific docs prefix) so tabs always open into /docs.
- Around line 138-143: active-tab detection uses strict equality (child.to ===
relativePathname) which fails for absolute or prefixed internal paths (like
/$libraryId/$version/docs/..., ./framework, ../x). Update the matching logic in
the block that computes activeGroup and activeChild to normalize both strings
(e.g., remove leading slashes, resolve ./ and ../, and strip dynamic prefixes
like $libraryId/$version) and then compare in a tolerant way (for example
compare normalized values or check endsWith for the route suffix). Locate the
comparisons against menuConfig children (symbols: activeGroup, activeChild,
menuConfig, relativePathname, child.to) and replace the strict equality check
with a normalized/tolerant path comparison helper so special/absolute internal
paths correctly match their tab entries.
🪄 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: 4ae98a77-2d6b-4d59-9366-bb82cc1c9701
📒 Files selected for processing (3)
src/components/DocsLayout.tsxsrc/utils/config.tssrc/utils/docsNavTabs.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/components/DocsLayout.tsx`:
- Around line 980-985: The Link currently sets aria-current='page' based solely
on tab.id === activeTabId which is misleading when the link's destination
(tab.firstItem / target.to) is not the actual current URL; update the Link in
DocsLayout (the block that renders Link for each tab) to set aria-current to a
boolean or undefined instead of the string 'page' — e.g., aria-current={isActive
? true : undefined} — or conditionally only set aria-current when target.to
matches the current location, using getActiveDocsNavTabId, tab.id and
tab.firstItem/getTabbedMenuConfig to determine correctness.
- Around line 810-839: The mobile docs dialog (`#docs-mobile-menu`) currently
opens/closes but does not move focus into the dialog, trap Tab/Shift+Tab while
open, or restore focus to the toggle button on close; update the component that
uses mobileMenuOpen / setMobileMenuOpen so that when opening you save
document.activeElement (the toggle button with
aria-controls="docs-mobile-menu"), move focus into the dialog (focus the first
focusable element inside `#docs-mobile-menu` or a programmatic focus sentinel),
add a keydown handler that implements Tab/Shift+Tab cycling (or integrate a
tested focus-trap library) to keep keyboard focus inside the dialog while
aria-modal="true", and on close remove the handler and restore focus to the
previously focused element; ensure focus restoration also runs when the dialog
is closed via the existing X button or Escape handling in setMobileMenuOpen.
🪄 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: 71e34d2e-0b2c-46d7-8728-d80a195f37a4
📒 Files selected for processing (2)
src/components/DocsLayout.tsxsrc/components/Navbar.tsx
| <Link | ||
| key={tab.id} | ||
| from="/$libraryId/$version/docs" | ||
| to={target.to} | ||
| params={linkParams} | ||
| aria-current={isActive ? 'page' : undefined} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What does aria-current="page" mean in WAI-ARIA/HTML, and is it appropriate for a navigation link that points to a different URL than the current page?
💡 Result:
The aria-current="page" attribute is a WAI-ARIA state used to programmatically indicate which item in a set of related elements represents the current page, such as a link within a navigation menu or breadcrumb [1][2][3]. When applied, it informs users of assistive technologies (like screen readers) that the element represents the user's current location, typically resulting in an announcement like "current page" appended to the link's accessible name [4]. Regarding your question about using it on a link that points to a different URL than the current page, the answer is generally no; it is not appropriate [4]. The attribute should only be used on the element that represents the page the user is currently viewing [1][2]. Applying it to a link that points to a different destination will misinform users, as it incorrectly indicates that they are already on that target page [4]. Best practices for using aria-current include: - Only mark one element in a set as current [1][4]. - Update the attribute dynamically when the user navigates, removing it from the old item and adding it to the new one [4]. - Use it on the specific element that matches the current location, such as the active link in a navigation bar or the final item in a breadcrumb trail [1][2][5]. - If an item in a navigation list is the current page, it is often recommended to either keep the link as is with the attribute applied or to remove the link entirely (turning it into plain text) while keeping the attribute [6][7][8]. If you keep it as a link, it must point to the actual current page [2][3].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-current
- 2: https://www.w3.org/WAI/WCAG22/Techniques/aria/ARIA26
- 3: https://w3c.github.io/wcag/techniques/aria/ARIA26
- 4: https://www.a11y-collective.com/blog/aria-current/
- 5: https://www.godaddy.com/resources/skills/best-practices-for-website-navigation
- 6: New ARIA technique for the aria-current attribute w3c/wcag#1707
- 7: https://stackoverflow.com/questions/76060257/should-i-not-use-aria-current-page-on-ancestor-link-of-current-page-in-navbar
- 8: https://heydonworks.com/article/the-accessible-current-page-link-conundrum/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/DocsLayout.tsx"
# Show the exact snippet around the referenced lines
nl -ba "$FILE" | sed -n '930,1040p'
# Extract the relevant tab/link logic nearby (best-effort search for isActive/tab.firstItem/linkParams)
rg -n "tab\.firstItem|isActive|aria-current|linkParams|target\.to" "$FILE" || trueRepository: TanStack/tanstack.com
Length of output: 108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/DocsLayout.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
ls -la src/components || true
exit 0
fi
# Show the exact snippet around the referenced lines (use cat -n since nl is unavailable)
cat -n "$FILE" | sed -n '960,1010p'
# Search nearby definitions/usages for isActive/tab.firstItem/target.to/aria-current
rg -n "tab\.firstItem|isActive|aria-current|linkParams|target\.to" "$FILE" || trueRepository: TanStack/tanstack.com
Length of output: 3095
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/DocsLayout.tsx"
# Locate activeTabId definition/usage
rg -n "activeTabId" "$FILE"
# Locate tabbedMenuConfig definition/shape
rg -n "tabbedMenuConfig" "$FILE"
# Show surrounding code where activeTabId is computed
# (grab a broad window around matches to include any helper fns used)
python3 - <<'PY'
import re
path="src/components/DocsLayout.tsx"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,l in enumerate(lines,1):
if 'activeTabId' in l:
start=max(1,i-60)
end=min(len(lines),i+60)
print("\n--- snippet around line",i,"---")
for j in range(start,end+1):
print(f"{j:5d}:{lines[j-1].rstrip()}")
PY
# Show tabbedMenuConfig map to see what tab.firstItem represents
python3 - <<'PY'
import re
path="src/components/DocsLayout.tsx"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,l in enumerate(lines,1):
if 'const target = tab.firstItem' in l:
start=max(1,i-80); end=min(len(lines),i+30)
print("\n--- snippet around tab.firstItem map start (line",i,") ---")
for j in range(start,end+1):
print(f"{j:5d}:{lines[j-1].rstrip()}")
PYRepository: TanStack/tanstack.com
Length of output: 24361
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/DocsLayout.tsx"
# Find where getActiveDocsNavTabId / getTabbedMenuConfig are imported/defined
rg -n "getActiveDocsNavTabId|getTabbedMenuConfig" "$FILE"
# Locate the function definitions in the repo (fast search)
rg -n "function getActiveDocsNavTabId|getActiveDocsNavTabId\s*=" src || true
rg -n "function getTabbedMenuConfig|getTabbedMenuConfig\s*=" src || true
# Also capture the type/shape of tabbed menu config (where tab.firstItem comes from)
rg -n "firstItem" src || trueRepository: TanStack/tanstack.com
Length of output: 624
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/utils/docsNavTabs.ts"
# Show the region that documents tab.firstItem and the function bodies
cat -n "$FILE" | sed -n '1,220p'Repository: TanStack/tanstack.com
Length of output: 7148
Avoid aria-current="page" for docs section tab links when to isn’t the current URL (src/components/DocsLayout.tsx, ~980-985)
aria-current is set to 'page' whenever tab.id === activeTabId, but activeTabId comes from the currently matched docs child (getActiveDocsNavTabId), while the link’s destination is always tab.firstItem (the section entry point chosen by getTabbedMenuConfig). On non-entry pages, that makes 'page' misleading for assistive tech; use aria-current={isActive ? true : undefined} or remove aria-current on these links.
🤖 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 `@src/components/DocsLayout.tsx` around lines 980 - 985, The Link currently
sets aria-current='page' based solely on tab.id === activeTabId which is
misleading when the link's destination (tab.firstItem / target.to) is not the
actual current URL; update the Link in DocsLayout (the block that renders Link
for each tab) to set aria-current to a boolean or undefined instead of the
string 'page' — e.g., aria-current={isActive ? true : undefined} — or
conditionally only set aria-current when target.to matches the current location,
using getActiveDocsNavTabId, tab.id and tab.firstItem/getTabbedMenuConfig to
determine correctness.
2 participants
Add docs-level navigation tabs for getting started, tutorials, guides, API, examples, and community links. Extract tab grouping logic out of
DocsLayoutand add optional config metadata for explicit tab assignment while preserving fallback classification for existing docs configs.Summary by CodeRabbit
New Features
Style