Skip to content

feat(website/docs): docs navigation, edit link, last updated, heading search - #709

Open
ajitpratap0 wants to merge 4 commits into
mainfrom
feat/docs-navigation
Open

ajitpratap0 wants to merge 4 commits into
mainfrom
feat/docs-navigation

Conversation

@ajitpratap0

Copy link
Copy Markdown
Owner

Summary

  • Mount a single Sidebar; on mobile it is an inline sticky "Docs menu" bar plus drawer (Esc closes), replacing the floating button.
  • Breadcrumb uses the sidebar label, single line with truncate.
  • Collapsible "On this page" below xl.
  • "Edit this page" (github edit link), "Last updated" (git commit date; mtime fallback on shallow clones) and a version badge from PROJECT.version.
  • Prev/next shows the category of the target doc.
  • Build-time search index (/docs/search-index.json, static route) with h2/h3 anchors; the search modal lazy-fetches it and deep-links to sections.
  • CHANGELOG.md intentionally not edited (release prepared in another session).

Test Plan

  • npm ci && npm run typecheck && npm run lint && npm run build (lint: 0 errors, pre-existing warnings only)
  • Built HTML for a doc page contains one sidebar, edit link, last-updated; index has ~1000 entries with anchors
  • Vercel preview: search a heading term, mobile menu, collapsible TOC

Breaking Changes

None.

Closes #656
Part of #634

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

… search

Single Sidebar mount with inline mobile menu bar (no FAB), truncated
breadcrumb using the sidebar label, collapsible On this page below xl,
Edit this page / Last updated / version badge, category-aware prev/next
and a build-time search index with h2/h3 deep links.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@vercel

vercel Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
gosqlx-website Ready Ready Preview Oct 14, 2026 6:27pm UTC

The sticky bar sat inside a wrapper that collapsed to the bar's height on
mobile (overlay and aside are fixed), so it scrolled away. Use display:
contents on mobile so the bar sticks within the page flex container.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@ajitpratap0

Copy link
Copy Markdown
Owner Author

Fixed in fa99a82: the Sidebar wrapper is now contents lg:block lg:w-64 lg:shrink-0, so on mobile the sticky bar sticks within the page's flex-col container instead of the collapsed wrapper. Re-checked in headless Chromium at 390x844 on /docs/sql-compatibility/: after scrollTo(0, 3000) the bar's top is at y=64 (pinned under the navbar), previously -2928. Desktop layout is unchanged (lg:block keeps the w-64 column). Gates: typecheck, lint (0 errors) and build pass.

Pushed to feat/docs-navigation as a normal push.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 27s —— View job


Review of PR #709: docs navigation, edit link, last updated, heading search

I read the full diff. I did not run the build, because the PR description says typecheck, lint and build passed. The change is clean overall and I found nothing blocking. These are the points worth a look, most important first.

1. Heading entries can flood search results (SearchModal.tsx, search-index.ts)

  • Every heading entry repeats the doc title and category, and Fuse searches title with weight 0.35.
  • A query like "parser" matches the page entry and also every h2/h3 entry under a doc titled "Parser". With limit: 10, one doc can fill the whole list.
  • Possible fixes:
    • Drop title and category from the heading entries' searchable keys, for example by storing the parent title in a non-indexed field such as parent.
    • Group or dedupe results per slug.
    • Raise the limit and cap to about 3 heading hits per doc.

2. git log is spawned per call with no memoization (docs.ts getLastUpdated)

  • getDocBySlug runs in generateMetadata, in the page, and in the search-index route. Each call forks git log, so a full build spawns it several times per doc.
  • A module-level Map<filePath, string> would remove the repeated spawns.
  • The shallow-clone check is already cached, which is good.
  • %cI is the committer date. A rebase or squash can change it, so %aI or %cI is a judgment call. Either is fine for this.

3. The "On this page" list is mounted twice (Toc.tsx, page.tsx)

  • The rail and collapsible variants both stay mounted and only CSS hides one of them. Each registers its own scroll/IntersectionObserver effect.
  • Both have aria-label="On this page". A hidden display:none nav is not exposed, so that is OK.
  • You could skip the observer effect in the collapsible variant, since it only needs links.
  • Clicking a link in the collapsible TOC leaves the <details> open. Closing it on click would be nicer on mobile.

4. The mobile drawer stays focusable when closed (Sidebar.tsx)

  • When closed, the <aside> is only translated off-screen. Its links are still in the tab order and exposed to screen readers.
  • Add inert, or invisible when closed on small screens, or use aria-hidden with tabIndex. Do this only below lg, since the sidebar must stay interactive at lg.
  • Esc closes the drawer, but focus is not moved into the drawer on open or back to the trigger on close.
  • Body scroll is not locked while the overlay is open.

5. Minor items

  • SearchModal: loadFailed is never reset. A retry on reopen works, because the effect re-runs when index.length is 0, but the stale error message shows until the fetch resolves. Call setLoadFailed(false) at the start of the effect.
  • SearchModal: when the index is empty and the user types, the "Loading..." message looks like "no results". That is fine, but consider a role="status" for it.
  • searchEntryHref hard-codes a trailing slash (/docs/slug/#anchor). Confirm it matches trailingSlash in next.config. A mismatch causes a redirect that can drop the hash on some hosts.
  • search-index.json/route.ts: it is static, so Vercel caches it. Make sure the file name is not rewritten by a middleware matcher or the CSP.
  • Sidebar.tsx: useEffect is declared after the sidebarContent constant. That is fine, because there is no early return, but moving it up with the other hooks reads better.
  • The edit link uses edit/main/${doc.file}. This assumes sidebarItem.file is relative to docs/. Verify one nested doc.
  • There are no tests. A small unit test for getAdjacentDocs and for the search-index shape would help, since the shape is now shared between the route and the modal.

Security and performance

  • I found no security concerns. The execFileSync call uses an argument array with no shell, and its paths come from the sidebar constants.
  • The lazy-fetched index (about 1000 entries) is small and keeps Fuse and the index out of the initial bundle.
    • Branch: feat/docs-navigation

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 32s —— View job


Review of PR #709: docs navigation, edit link, last updated, heading search

I read the full diff (git diff origin/main...HEAD) and did not build or run it. The change is clean and well scoped, and I found no blocking bugs. The points below are mostly about behavior and polish.

Worth addressing

  1. "Last updated" will be misleading on Vercel (website/src/lib/docs.ts, getLastUpdated)

    • The shallow-clone fallback uses fs.statSync().mtime. On a fresh Vercel checkout, every file's mtime is the checkout time. Every doc will therefore show the build date, which is the same inaccuracy the fallback is meant to avoid.
    • Better options:
      • Unshallow the clone at build time with git fetch --unshallow (or VERCEL_DEEPEN-style settings).
      • Omit the "Last updated" line when git history is unavailable, instead of showing a wrong date.
      • Precompute dates in CI and commit or inject them as a JSON map.
    • The same value feeds TechArticle.dateModified in the JSON-LD, so search engines will see a wrong modified date too.
  2. Repeated git log spawns (website/src/lib/docs.ts)

    • getDocBySlug now runs execFileSync('git', ['log', …]) on every call. It is called from generateMetadata, the page, and the search-index route, once per doc each time. This adds up during static generation of dozens of pages.
    • Memoizing by filePath in a Map, like the shallowClone cache, would make it a single spawn per file. Computing modifiedAt lazily would also help, because the search index doesn't need it.
  3. Mobile drawer accessibility (website/src/components/docs/Sidebar.tsx)

    • Esc handling and aria-expanded are good. The drawer still has no focus move on open, no focus return to the "Docs menu" button on close, and no body scroll lock. The drawer is also not inert or aria-hidden while closed, so off-screen links stay in the tab order on mobile.
    • A cheap fix for the tab-order problem is inert={!mobileOpen} below lg. Because the aside is always visible at lg, that needs a media-query-aware approach.
    • Selecting a link in the drawer should also close it. I couldn't see that in the diff; presumably existing code handles it, so please confirm.

Minor / nits

  • Toc is mounted twice (page.tsx: collapsible and rail). Each instance sets up its own scroll/IntersectionObserver effect, so one is wasted at any breakpoint. It's harmless but could be skipped by gating on matchMedia. Also, clicking a heading link in the collapsible variant leaves the <details> open.
  • searchEntryHref always emits /docs/${slug}/ with a trailing slash. Confirm this matches trailingSlash in next.config. Otherwise it costs a redirect, and the hash can be lost on some redirect paths.
  • Search modal state (SearchModal.tsx): loadFailed is never reset, and there is no retry path while the modal stays open. After a transient failure the user has to close and reopen it to retry. Resetting loadFailed when a fetch starts would fix that.
  • Search ranking: with heading weighted 0.4 and limit: 10, a page with many matching h2/h3 entries can crowd out the page-level result for the same term. Consider grouping, or boosting page-level entries.
  • Heading ids vs rehypeSlug: the deep links depend on extractHeadings (github-slugger) matching rehypeSlug output exactly, including duplicate-heading suffixes. Your build check of anchors is a good sign, but a small unit test comparing the two would lock it in.
  • Formatting: in page.tsx the new <span className="flex items-center gap-2"> wrapper and the Toc list body have off indentation, which Prettier or lint would normally normalize.
  • Version badge: PROJECT.version is shown on every page without saying what it is. A "v" prefix or a title attribute would clarify it. Check that the value already includes the "v".

Security / performance

  • Using execFileSync with an argument array and a fixed cwd avoids shell injection. The only input is a path derived from the sidebar config, so it is safe.
  • The edit link uses rel="noopener noreferrer", which is correct.
  • Moving the index to a lazily fetched static route keeps it out of the client bundle. The route has force-static, so it is cached at the CDN. Heading entries omit description, which keeps the JSON small.

Tests

  • The website has no tests for the new logic: getLastUpdated fallbacks, getAdjacentDocs category data, searchEntryHref, and the anchor consistency noted above. The pure helpers (searchEntryHref, getAdjacentDocs) would be easy to cover if the website has a test runner.
  • The unchecked item in the test plan (Vercel preview: heading search, mobile menu, collapsible TOC) should be verified before merge. Pay particular attention to the "Last updated" dates there (see point 1).

Overall: LGTM once the "Last updated" accuracy on shallow clones is decided, and ideally with the git-call memoization.
· Branch: feat/docs-navigation

… bar

The translucent bg-surface/95 variant was not matched by the html.light
override, leaving a near-black bar with low-contrast text in light mode.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@ajitpratap0

Copy link
Copy Markdown
Owner Author

Fixed in the latest push: the mobile Docs menu bar in Sidebar.tsx now uses bg-surface instead of bg-surface/95. bg-surface is already overridden for light mode (html.light .bg-surface), so the bar is white with the recoloured label text instead of near-black with grey text. Typecheck, lint (0 errors) and build pass.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 19s —— View job


Review of PR #709: docs navigation, edit link, last updated, heading search

I read the diff against origin/main. I did not build or run it. I found no blocking bugs, and trailingSlash: true is set in next.config, so the searchEntryHref slash is correct. The two earlier automated reviews raised most of these points already, so I've only listed what I confirmed in the code.

Worth addressing

  1. "Last updated" on shallow clones (website/src/lib/docs.ts, getLastUpdated)

    • On a Vercel shallow clone the code falls back to mtime. That is the checkout time, so every doc will show the build date. The same value goes into TechArticle.dateModified.
    • Better options are to deepen the clone at build time, or to omit the date (and dateModified) when history is unavailable.
    • The comment above the function says the fallback avoids reporting the tip commit date. The mtime fallback has the same inaccuracy.
  2. git log is not memoized (docs.ts)

    • getDocBySlug now runs git log on every call. It is called from generateMetadata, the page and the search-index route.
    • A module-level Map<filePath, string> would cut this to one spawn per file.
    • The search index only needs the doc content, so computing modifiedAt lazily would also help.
  3. Search results can be crowded out (SearchModal.tsx)

    • Heading entries repeat title and category, and Fuse searches both. A query matching a doc title also matches every h2/h3 under that doc.
    • With limit: 10, one doc can fill the list.
    • Possible fixes: store the parent title in a non-indexed field, cap heading hits per slug, or boost page-level entries.
  4. Mobile drawer accessibility (Sidebar.tsx)

    • When closed, the <aside> is only translated off-screen, so its links stay in the tab order.
    • Focus does not move into the drawer on open or back to the trigger on close.
    • Body scroll is not locked while the overlay is open.
    • inert while closed below lg would fix the tab order. It needs a media-query-aware condition, because the aside must stay interactive at lg.

Minor

  • SearchModal: loadFailed is never reset. The effect refetches when the modal reopens with an empty index, but the "unavailable" message stays until the fetch resolves. Call setLoadFailed(false) at the start of the effect.
  • SearchModal: the "Loading..." message could use role="status".
  • Toc is mounted twice, as the rail and collapsible variants, and each registers its own observer effect. The <details> also stays open after a link click.
  • Sidebar.tsx: the new useEffect sits after the sidebarContent constant. Moving it up with the other hooks would read better.
  • Heading anchors depend on extractHeadings (github-slugger) matching rehypeSlug output, including duplicate-heading suffixes. A small unit test would lock that in.
  • Please check one nested doc to confirm docs/${sidebarItem.file} produces a valid edit URL.
  • The route file name search-index.json must not be caught by a middleware matcher or the CSP.

Security and performance

  • execFileSync takes an argument array, with a fixed cwd and no shell, so I see no injection risk. The paths come from the sidebar constants.
  • The edit link has rel="noopener noreferrer".
  • Fetching the index lazily keeps it out of the initial bundle. Heading entries omit description, which keeps the JSON small.

Tests

  • The website has no tests for the new logic. getAdjacentDocs and searchEntryHref are pure and easy to cover, and the index shape is now shared between the route and the modal.
  • The unchecked Vercel preview item in the test plan should be done before merge. When you do, look at the "Last updated" dates (point 1).

Verdict: LGTM once you've decided what "Last updated" should show on shallow clones. Memoizing the git call is a cheap improvement.
· Branch: feat/docs-navigation

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@vercel

vercel Bot commented Oct 14, 2026

Copy link
Copy Markdown

Deployment failed for project gosqlx-website with the following error:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/coolajitpratapsingh0-9932s-projects?upgradeToPro=build-rate-limit

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 18s —— View job


Review of PR #709: docs navigation, edit link, last updated, heading search

I read the diff against origin/main. I did not build or run it. I found no blocking bugs. The earlier automated reviews cover most of the points below, so this list only includes what I confirmed in the code, ordered by priority.

Worth addressing before merge

  1. "Last updated" on shallow clones (website/src/lib/docs.ts, getLastUpdated)

    • On a shallow clone, which Vercel uses, the fallback is fs.statSync().mtime. That is the checkout time, so every doc reports the build date.
    • The same value feeds TechArticle.dateModified, so the JSON-LD gets a wrong date too.
    • The doc comment says the fallback avoids the tip-commit date. The mtime fallback has the same flaw.
    • Options:
      • Deepen the clone at build time.
      • Omit "Last updated" and dateModified when history is unavailable.
      • Precompute the dates in CI.
  2. git log is not memoized (docs.ts)

    • getDocBySlug forks git log on every call. It is called from generateMetadata, the page and the search-index route.
    • A module-level Map<string, string> keyed by filePath would give one spawn per file.
    • The search index only needs content, so modifiedAt could be computed lazily.
  3. Heading entries can crowd out search results (SearchModal.tsx, route.ts)

    • Each heading entry repeats title and category, and Fuse searches title with weight 0.35.
    • A query matching a doc title therefore also matches every h2/h3 under that doc. With limit: 10, one doc can fill the list.
    • Possible fixes:
      • Store the parent title in a non-indexed field such as parent.
      • Cap heading hits per slug.
      • Boost page-level entries.
  4. Mobile drawer accessibility (Sidebar.tsx)

    • When the drawer is closed, the <aside> is only translated off-screen. Its links stay in the tab order and are exposed to screen readers.
    • Focus does not move into the drawer on open or back to the "Docs menu" button on close.
    • Body scroll is not locked while the overlay is open.
    • To fix the tab order, make the aside inert when closed below lg. This needs a media-query-aware condition, because the aside must stay interactive at lg.
    • I could not tell from the diff whether tapping a nav link closes the drawer. Please confirm that existing code handles it.

Minor

  • SearchModal.tsx: loadFailed is never reset. Call setLoadFailed(false) at the start of the fetch effect. Otherwise the "unavailable" message stays visible while a retry on reopen is in flight.
  • SearchModal.tsx: the "Loading..." text shares the empty-results slot. Add role="status" so screen readers announce it.
  • Toc.tsx / page.tsx: the rail and collapsible variants are both mounted, and each registers its own observer effect. The <details> also stays open after a link click.
  • Sidebar.tsx: the new useEffect sits after the sidebarContent constant. Move it up with the other hooks.
  • Heading anchors rely on extractHeadings (github-slugger) matching rehypeSlug output, including suffixes for duplicate headings. A small unit test would lock this in.
  • Please verify one nested doc to confirm docs/${sidebarItem.file} produces a valid GitHub edit URL.
  • searchEntryHref emits a trailing slash. trailingSlash: true is set in next.config, so that is correct.
  • Make sure no middleware matcher or CSP rule catches /docs/search-index.json.

Security and performance

  • No security concerns. execFileSync takes an argument array with a fixed cwd and no shell, and the paths come from the sidebar constants.
  • The edit link uses rel="noopener noreferrer".
  • Fetching the index lazily keeps Fuse and the index out of the initial bundle. Heading entries omit description, which keeps the JSON small.

Tests

  • The website has no tests for the new logic. getAdjacentDocs and searchEntryHref are pure and easy to cover. The index shape is now shared between the route and the modal, so a shape test would help too.
  • The unchecked Vercel preview item (heading search, mobile menu, collapsible TOC) should be done before merge. Check the "Last updated" dates there (point 1).

Verdict: LGTM once you decide what "Last updated" should show on shallow clones. Memoizing the git call is a cheap improvement.
· Branch: feat/docs-navigation

This branch was successfully deployed

1 active (outdated) deployment
Preview — 49db8f5a Deployed Oct 14, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(website/docs): docs navigation, edit link, last updated, build-time search index [1.16]

1 participant