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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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
…into nav-update
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
| <div | ||
| id="docs-mobile-menu" | ||
| role="dialog" | ||
| aria-modal="true" | ||
| aria-label="Documentation navigation" | ||
| hidden={!mobileMenuOpen} | ||
| className="fixed inset-x-0 top-[var(--navbar-height)] z-50 | ||
| max-h-[calc(100dvh-var(--navbar-height))] overflow-y-auto | ||
| bg-white dark:bg-black/95 backdrop-blur-lg border-b border-gray-500/20 shadow-xl" | ||
| > | ||
| <summary className="py-2 px-4 flex gap-2 items-center"> | ||
| <div className="flex gap-2 items-center shrink-0 pr-2"> | ||
| <Menu className="cursor-pointer" /> | ||
| Docs | ||
| </div> | ||
| <div className="w-px h-6 bg-gray-300 dark:bg-gray-600 shrink-0" /> | ||
| <MobilePartnersStrip | ||
| partners={activePartners} | ||
| onLabelClick={() => { | ||
| const details = detailsRef.current as HTMLDetailsElement | null | ||
| if (details) { | ||
| details.open = !details.open | ||
| } | ||
| }} | ||
| /> | ||
| </summary> | ||
| <div className="flex flex-col gap-4 p-4 overflow-y-auto border-t border-gray-500/20 bg-white/20 text-lg dark:bg-black/20"> | ||
| <div className="flex items-center justify-between py-2 px-4 border-b border-gray-500/20"> | ||
| <span className="font-bold">Docs</span> | ||
| <button | ||
| type="button" | ||
| aria-label="Close menu" | ||
| className="p-1 rounded-md hover:bg-gray-500/10 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-current" | ||
| onClick={() => setMobileMenuOpen(false)} | ||
| > | ||
| <X className="w-5 h-5" /> | ||
| </button> | ||
| </div> | ||
| <div className="flex flex-col gap-4 p-4 text-lg"> | ||
| <div className="flex flex-col gap-1"> | ||
| <FrameworkSelect libraryId={libraryId} /> | ||
| <VersionSelect libraryId={libraryId} /> | ||
| </div> | ||
| <SearchButton /> | ||
| {menuItems} | ||
| </div> | ||
| </details> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
According to the WAI-ARIA Authoring Practices, what keyboard focus behavior is expected for a modal dialog opened with role="dialog" and aria-modal="true"?
💡 Result:
According to the WAI-ARIA Authoring Practices (APG), a modal dialog (role="dialog" with aria-modal="true") is expected to implement the following keyboard focus behaviors [1][2]: 1. Initial Focus Placement: When the dialog opens, keyboard focus must move into the dialog [1][3]. While focus is generally set to the first focusable element, the most appropriate placement depends on the dialog's content [1]. For example, if the dialog contains a large amount of static text, it may be more appropriate to place focus on a focusable static element (e.g., the title or first paragraph) rather than the first interactive control [1]. 2. Focus Trapping: Modal dialogs must contain their tab sequence [4]. Users should not be able to move keyboard focus outside the dialog window using Tab or Shift+Tab [4][1]. The focus must be trapped within the dialog until it is explicitly closed [1][5]. 3. Keyboard Navigation: Within the dialog, the Tab key moves focus to the next focusable element, and Shift+Tab moves focus to the previous focusable element [1][2]. When focus reaches the last focusable element, it should wrap to the first; similarly, when focus is on the first element, pressing Shift+Tab should wrap it to the last element [1][2]. 4. Closing Behavior and Focus Restoration: The Escape key is expected to close the dialog [2][5]. Upon closing, keyboard focus must return to the element that originally invoked the dialog [1][3]. This ensures the user's point of regard is maintained [2]. Note: The aria-modal="true" attribute is used to inform assistive technologies that content outside the dialog is inert [6][7], but it does not automatically provide the required keyboard focus management; developers must implement the trapping and focus restoration logic manually using JavaScript [6][5].
Citations:
- 1: https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/
- 2: https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/examples/dialog/
- 3: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/dialog_role
- 4: https://github.com/w3c/aria-practices/blob/main/content/patterns/dialog-modal/dialog-modal-pattern.html
- 5: https://www.levelaccess.com/blog/the-importance-of-keyboard-accessibility-why-aria-widgets-dont-work-as-expected-in-voice-navigation-software/
- 6: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-modal
- 7: https://www.w3.org/TR/2021/NOTE-wai-aria-practices-1.2-20211129/examples/dialog-modal/dialog.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the mobile dialog markup and nearby handlers
sed -n '780,920p' src/components/DocsLayout.tsx | nl -ba | sed -n '1,200p'
echo "----"
# Show the trigger/open/close handlers mentioned (around 947-953)
sed -n '920,1010p' src/components/DocsLayout.tsx | nl -ba | sed -n '1,200p'
echo "----"
# Search for focus management / trap / restoration keywords in the file
rg -n --hidden --no-ignore -S "focus|trap|restore|aria-modal|role=\"dialog\"|inert|tabindex|onKeyDown|Escape|Esc" src/components/DocsLayout.tsxRepository: TanStack/tanstack.com
Length of output: 108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Mobile dialog markup near the cited lines
echo "=== DocsLayout.tsx mobile dialog markup (around 810-880) ==="
sed -n '780,880p' src/components/DocsLayout.tsx | cat -n
echo "=== DocsLayout.tsx mobile dialog tail (around 880-920) ==="
sed -n '880,940p' src/components/DocsLayout.tsx | cat -n
echo "=== DocsLayout.tsx trigger/open/close handlers (around 920-1020) ==="
sed -n '920,1020p' src/components/DocsLayout.tsx | cat -n
echo "----"
# Search for any existing focus management
echo "=== Searching for focus management / trap / restoration logic ==="
rg -n "role=\"dialog\"|aria-modal|focus trap|focusTrap|trapFocus|restoreFocus|setTimeout\(|useEffect\(|onKeyDown|Escape|Esc|tabIndex|inert|aria-hidden" src/components/DocsLayout.tsxRepository: TanStack/tanstack.com
Length of output: 12843
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect around the effects near the Escape handler
echo "=== Inspect useEffect(s) around keydown/Escape handling ==="
sed -n '540,660p' src/components/DocsLayout.tsx | cat -n
echo "----"
# Inspect around the useEffect block(s) nearer dialog open state (the file hit at 672)
echo "=== Inspect around React.useEffect near line ~660-740 ==="
sed -n '650,760p' src/components/DocsLayout.tsx | cat -n
echo "----"
# Show the exact dialog toggle effects near where dialog markup starts
echo "=== Inspect around where mobileMenuOpen is declared/used (around 780-850) ==="
sed -n '760,840p' src/components/DocsLayout.tsx | cat -nRepository: TanStack/tanstack.com
Length of output: 12300
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== All aria-current occurrences in DocsLayout.tsx ==="
rg -n "aria-current" src/components/DocsLayout.tsx
echo "----"
echo "=== Search for focus trap / restoration patterns in DocsLayout.tsx ==="
rg -n "focus trap|focusTrap|trapFocus|restoreFocus|previousActiveElement|returnFocus|focus\W*\(|tabbable|inert|aria-hidden" src/components/DocsLayout.tsx || trueRepository: TanStack/tanstack.com
Length of output: 304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "id=\"docs-mobile-menu\"|aria-modal=\"true\"|aria-controls=\"docs-mobile-menu\"|onClick=\(\(\) => setMobileMenuOpen|setMobileMenuOpen\(\(open\) => !open\)" src/components/DocsLayout.tsxRepository: TanStack/tanstack.com
Length of output: 241
Trap keyboard focus in the mobile docs modal (role="dialog" aria-modal="true")
APG expects: move focus into the dialog on open, trap Tab/Shift+Tab within it while it’s open, and restore focus back to the opener on close. The current implementation closes on Escape/X but doesn’t add focus handoff, focus trapping, or trigger-focus restoration for #docs-mobile-menu (id/role/aria-modal at ~811-813) or the toggle button (aria-controls="docs-mobile-menu" at ~951-952).
🤖 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 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.
| <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.
1 participant
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