feat: default theme layout with tab bar + page nav types#37
feat: default theme layout with tab bar + page nav types#37
Conversation
Prepare Page type for backend-provided prev/next links used by the docs sub-navbar. Adds PageNavLink/PageNav types, extends Page via extends PageNav, unifies PageData with Page in page context, and threads null placeholders through SSR + hydration until backend computes real values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nitro@3.0.260311-beta lists std-env in devDependencies but its runtime imports it. Node ESM resolution from nitro's isolated cache path fails without std-env at top-level node_modules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the top Navbar; move navigation into a per-section tab bar alongside the sidebar. Resize sidebar to 262px, swap button-based nav links for pill-style tabs using mini typography tokens. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 25 minutes and 24 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds prev/next page navigation metadata through types, server SSR, API, client hydration, and page context; updates theme layout to a tabbed header and moves search/theme controls into the sidebar; small doc and dependency tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant ClientApp as Client App
participant Server
participant PageAPI as /api/page
participant PageTree as Page Tree (source)
Browser->>Server: Request page (SSR)
Server->>PageTree: getPageTree() / getPage(slug)
Server->>PageTree: getPageNav(slug)
PageTree-->>Server: pageData + nav(prev/next)
Server-->>Browser: HTML with embedded __PAGE_DATA__ (includes prev/next)
Browser->>ClientApp: Hydrate (reads window.__PAGE_DATA__)
ClientApp->>PageContext: setPage(page + prev/next)
ClientApp->>PageAPI: (later) fetch /api/page?slug (returns prev/next)
PageAPI->>PageTree: getPageNav(slug)
PageTree-->>PageAPI: nav(prev/next)
PageAPI-->>ClientApp: page payload with prev/next
ClientApp-->>Layout: render prev/next navigation (IconButtons)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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
🧹 Nitpick comments (5)
packages/chronicle/src/themes/default/Layout.module.css (1)
36-67: Tab styles look good; minor consideration onborder: 0.5px.Sub-pixel borders (
0.5px) render inconsistently across browsers/DPRs — on 1x displays some browsers will round to 0 and the border disappears. If the intent is a thinner-looking divider, consider1pxwith a lighter border color token, or rely ontransform: scaleY(0.5)patterns. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/themes/default/Layout.module.css` around lines 36 - 67, Replace the sub-pixel border in the .tab rule: avoid using "0.5px" (in Layout.module.css .tab) because it can disappear on some DPRs; change it to "1px" and pair it with a lighter border token (e.g., use the existing var(--rs-color-border-*-secondary) style) or alternatively implement a visual thinner divider using transform scaling on a pseudo-element (apply to .tab or .tabActive as appropriate) so the thin visual is preserved across browsers and DPRs.packages/chronicle/src/server/entry-server.tsx (1)
38-60:prev/nexthardcoded tonull— track the follow-up.Per the PR description this is intentional placeholder wiring until the backend computes adjacent-page links from the page tree. Worth leaving a
TODOhere (and symmetric sites) referencing the tracking issue so this doesn't silently ship to consumers expecting real navigation data.Happy to open a follow-up issue to compute
prev/nextfromgetPageTree()+ currentslugif you'd like.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/server/entry-server.tsx` around lines 38 - 60, The pageData currently sets prev and next to null and embeddedData mirrors those nulls; add a clear TODO comment in the pageData/embeddedData block noting these are intentional placeholders and reference the tracking issue (e.g. "TODO: compute prev/next from getPageTree() using current slug — see tracking issue #<ID>") so consumers and future maintainers know to replace the hardcoded nulls with adjacent-page links computed from getPageTree() + slug.packages/chronicle/src/server/entry-client.tsx (1)
13-22:EmbeddedDatadrifts fromPageshape — consider reusing types.
EmbeddedDataduplicates fields that now exist onPage/PageNav. As more nav metadata lands (e.g., section, breadcrumbs), keeping two hand-maintained shapes in sync is error-prone. Consider typing asPick<Page, 'slug' | 'frontmatter' | 'prev' | 'next'> & { config; tree; relativePath; originalPath? }or defining a sharedEmbeddedPagePayloadin@/types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/server/entry-client.tsx` around lines 13 - 22, The EmbeddedData interface duplicates fields already defined on Page/PageNav (slug, frontmatter, prev, next); replace the handwritten shape by reusing existing types—e.g., change EmbeddedData to compose Pick<Page, 'slug' | 'frontmatter' | 'prev' | 'next'> & { config: ChronicleConfig; tree: Root; relativePath: string; originalPath?: string } or create/use a shared EmbeddedPagePayload type in the types package and import it; update imports to pull Page (or EmbeddedPagePayload) and PageNavLink as needed and remove the duplicated field declarations from EmbeddedData.packages/chronicle/src/themes/default/Layout.tsx (2)
1-9: Nit: merge duplicate@heroicons/react/24/outlineimports.Lines 1 and 2–5 import from the same module in two separate statements.
♻️ Proposed tidy-up
-import { RectangleStackIcon } from '@heroicons/react/24/outline'; import { + RectangleStackIcon, DocumentTextIcon, CodeBracketSquareIcon } from '@heroicons/react/24/outline'; -import { +import { Flex, Sidebar } from '@raystack/apsara';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/themes/default/Layout.tsx` around lines 1 - 9, There are two import statements pulling icons from the same module; merge them into a single import from '@heroicons/react/24/outline' that includes RectangleStackIcon, DocumentTextIcon, and CodeBracketSquareIcon to avoid duplicate imports (leave the existing imports for Flex and Sidebar from '@raystack/apsara' unchanged); update the import block in Layout.tsx so all three icon symbols are imported in one statement.
77-100: Accessibility: tabs semantics and focus.The tab bar uses
<nav>+RouterLinks styled as tabs. If these are genuinely tabs (mutually exclusive sections), considerrole="tablist"/role="tab"+aria-current="page"on the active link — at minimum addaria-current="page"to the active tab so assistive tech announces selection. Verify keyboard focus states are visible under the new.tab/.tabActiverules (no:focus-visiblestyles are defined in the module).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/themes/default/Layout.tsx` around lines 77 - 100, The tab bar currently renders RouterLink elements as tabs but lacks proper ARIA and focus semantics; update the nav and links in Layout.tsx to add role="tablist" on the nav, role="tab" on each RouterLink, and set aria-current="page" on the active tab (use the existing isApiRoute logic for the API links and the root tab check for '/'), and ensure the styles.tab / styles.tabActive CSS module includes visible focus styles (or add a :focus-visible rule) so keyboard focus is clearly visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/chronicle/src/lib/page-context.tsx`:
- Around line 108-113: The API response may omit prev/next so normalize them to
match Page['prev'] (PageNavLink | null) before calling setPage: when handling
the promise result in the .then callback (the function that calls loadMdx,
setErrorStatus and setPage with slug and data.frontmatter), coerce data.prev and
data.next to null if they are undefined (e.g., const prev = data.prev ?? null;
const next = data.next ?? null) and pass those normalized values into setPage to
preserve the SSR-hydrated shape and avoid undefined prev/next on client
navigation.
In `@packages/chronicle/src/themes/default/Layout.tsx`:
- Around line 86-95: The API tabs all use the shared boolean isApiRoute causing
every API entry from config.api to be marked active; update the tab active logic
to compute per-entry activity by checking the current pathname against each
api.basePath (e.g., replace the shared isApiRoute in the RouterLink className
expression with a per-item check like pathname.startsWith(api.basePath) or a
helper like isApiBasePath(api.basePath)); ensure the RouterLink rendering for
each api uses that per-api active boolean when toggling styles.tabActive (and
revisit the Docs tab's !isApiRoute usage if you later add non-/apis sections).
---
Nitpick comments:
In `@packages/chronicle/src/server/entry-client.tsx`:
- Around line 13-22: The EmbeddedData interface duplicates fields already
defined on Page/PageNav (slug, frontmatter, prev, next); replace the handwritten
shape by reusing existing types—e.g., change EmbeddedData to compose Pick<Page,
'slug' | 'frontmatter' | 'prev' | 'next'> & { config: ChronicleConfig; tree:
Root; relativePath: string; originalPath?: string } or create/use a shared
EmbeddedPagePayload type in the types package and import it; update imports to
pull Page (or EmbeddedPagePayload) and PageNavLink as needed and remove the
duplicated field declarations from EmbeddedData.
In `@packages/chronicle/src/server/entry-server.tsx`:
- Around line 38-60: The pageData currently sets prev and next to null and
embeddedData mirrors those nulls; add a clear TODO comment in the
pageData/embeddedData block noting these are intentional placeholders and
reference the tracking issue (e.g. "TODO: compute prev/next from getPageTree()
using current slug — see tracking issue #<ID>") so consumers and future
maintainers know to replace the hardcoded nulls with adjacent-page links
computed from getPageTree() + slug.
In `@packages/chronicle/src/themes/default/Layout.module.css`:
- Around line 36-67: Replace the sub-pixel border in the .tab rule: avoid using
"0.5px" (in Layout.module.css .tab) because it can disappear on some DPRs;
change it to "1px" and pair it with a lighter border token (e.g., use the
existing var(--rs-color-border-*-secondary) style) or alternatively implement a
visual thinner divider using transform scaling on a pseudo-element (apply to
.tab or .tabActive as appropriate) so the thin visual is preserved across
browsers and DPRs.
In `@packages/chronicle/src/themes/default/Layout.tsx`:
- Around line 1-9: There are two import statements pulling icons from the same
module; merge them into a single import from '@heroicons/react/24/outline' that
includes RectangleStackIcon, DocumentTextIcon, and CodeBracketSquareIcon to
avoid duplicate imports (leave the existing imports for Flex and Sidebar from
'@raystack/apsara' unchanged); update the import block in Layout.tsx so all
three icon symbols are imported in one statement.
- Around line 77-100: The tab bar currently renders RouterLink elements as tabs
but lacks proper ARIA and focus semantics; update the nav and links in
Layout.tsx to add role="tablist" on the nav, role="tab" on each RouterLink, and
set aria-current="page" on the active tab (use the existing isApiRoute logic for
the API links and the root tab check for '/'), and ensure the styles.tab /
styles.tabActive CSS module includes visible focus styles (or add a
:focus-visible rule) so keyboard focus is clearly visible.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 324e5e64-cd4d-411d-9155-dc48131055f8
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
docs/chronicle.yamlpackage.jsonpackages/chronicle/src/lib/page-context.tsxpackages/chronicle/src/pages/DocsPage.tsxpackages/chronicle/src/server/entry-client.tsxpackages/chronicle/src/server/entry-server.tsxpackages/chronicle/src/themes/default/Layout.module.csspackages/chronicle/src/themes/default/Layout.tsxpackages/chronicle/src/types/content.ts
Add getPageNav helper that flattens the page tree and returns the adjacent PageNavLinks for a slug. Both the /api/page handler and SSR pageData now populate prev/next so the client can render navigation without re-flattening the tree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Default theme now renders the main content inside a rounded, bordered card with a sub-navbar at the top containing prev/next IconButtons driven by the server-provided page nav, followed by breadcrumbs. Breadcrumbs move out of Page.tsx since the Layout owns the chrome. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add sidebar top navbar with placeholder logo, icon-only search, and IconButton-based theme switcher using resolvedTheme so system mode lands on the correct icon. - Simplify Search component to IconButton only, drop cmdk kbd trigger. - Wrap content in a bordered card, remove TOC from Page, align heading + paragraph typography with Aurora tokens. - Secondary bg on sidebar + card wrapper, primary bg on content area. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/chronicle/src/themes/default/Layout.module.css (1)
103-124: Hardcoded shadow color and near-no-opbackdrop-filter.
- Lines 110–111:
rgba(0, 0, 0, 0.04)won't render meaningfully in dark themes; consider a token-based shadow (e.g.,var(--rs-shadow-...)) to stay theme-aware.- Lines 123 / 31:
backdrop-filter: blur(1px)is visually imperceptible. Either increase the radius or drop it to avoid paying for an unused GPU pass.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/themes/default/Layout.module.css` around lines 103 - 124, Update the theme-unaware hardcoded shadow and the near-no-op backdrop filter: replace the rgba(...) shadow used in the .card rule with a theme token (e.g., var(--rs-shadow-*, or an appropriate existing shadow variable) so the elevation responds to dark/light themes and accessibility settings, and for .subNav either increase the blur radius (e.g., to a perceptible value) or remove the backdrop-filter property to avoid unnecessary GPU work—change the rules in the .card and .subNav selectors accordingly.packages/chronicle/src/themes/default/Page.module.css (1)
33-45: Minor: redundant/odd values in content typography.
- Line 34:
margin-top: 0on.content h1is redundant with the:first-childreset above and also removes top spacing for any non-firsth1(uncommon, but worth a conscious call).- Line 44:
line-height: 171.429%is a computed literal — prefer a design token (e.g.,var(--rs-line-height-regular)) or a clean unitless value like1.71for readability and consistency with the rest of the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/themes/default/Page.module.css` around lines 33 - 45, Remove the redundant margin-top: 0 from the .content h1 rule (or replace it with a comment/intent if you explicitly need to override the :first-child reset) to avoid unintentionally removing top spacing for non-first h1s; and change the .content p rule's line-height: 171.429% to a design token or a unitless value (e.g., var(--rs-line-height-regular) or 1.71) so it matches the file's styling conventions and improves readability.packages/chronicle/src/themes/default/Layout.tsx (1)
121-150: Consider hiding the sub-nav on pages without navigation data.On routes where
pageisnull(404, non-content routes, API index) both IconButtons will be disabled and Breadcrumbs may render empty — producing a visually empty sub-nav bar. Low priority, but you may want to conditionally render<nav className={styles.subNav}>when there is meaningful content to show, or collapse it to avoid an empty 48px bar.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/themes/default/Layout.tsx` around lines 121 - 150, The sub-nav should be omitted when there's no navigation to display; update the Layout component to only render the <nav className={styles.subNav}> block when there is meaningful content (e.g. prev or next exist, or Breadcrumbs would render). Compute a boolean like hasNav = Boolean(prev || next || (slug && tree && tree.length)) and wrap the existing nav (including the IconButton group and <Breadcrumbs slug={slug} tree={tree} />) in a conditional render using that flag so the empty 48px sub-nav is not shown on pages with no page data.packages/chronicle/src/components/ui/client-theme-switcher.tsx (1)
19-34: Minor: hydration placeholder causes a brief layout shift.Returning
nulluntilisClientflips avoids SSR/CSR theme mismatch, but the sidebar navbar's right-side actions row visibly reflows on mount as this button appears. Consider rendering a fixed-size placeholder (or usingvisibility: hiddenon a sized wrapper) to reserve space, e.g.:💡 Suggestion
- if (!isClient) return null - - const isDark = resolvedTheme === 'dark' + const isDark = resolvedTheme === 'dark' + if (!isClient) { + return <IconButton size={3} aria-hidden tabIndex={-1} style={{ visibility: 'hidden' }} /> + }Also note: the
sizeprop only controls the inner icon dimensions; the outerIconButtonis pinned tosize={3}. If that's intentional, it may be worth documenting or dropping the prop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/components/ui/client-theme-switcher.tsx` around lines 19 - 34, When isClient is false the component returns null causing a layout shift on mount; instead render a fixed-size placeholder that matches the IconButton's outer dimensions so space is reserved (for example render the same IconButton structure with visibility:hidden or a sized wrapper) inside the ClientThemeSwitcher (use the existing isClient check and IconButton/size props to mirror size), and either align the outer IconButton's size with the incoming size prop or document why IconButton uses size={3} if you intentionally keep the inner icon size independent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/chronicle/src/components/ui/search.tsx`:
- Around line 55-62: The keyboard shortcut (Cmd/Ctrl+K) is still wired in the
useEffect but the visual hint was removed; add a discoverability hint to the
IconButton by setting a title attribute (e.g., title="Search (⌘K)") on the
IconButton component used in this file so users see the shortcut on hover; keep
the existing props (size, aria-label, onClick => setOpen(true), className) and
only add the title string (you can include both Mac and Win hints if desired).
In `@packages/chronicle/src/lib/source.ts`:
- Around line 115-118: toLink currently forces PageNavLink.title by doing
String(p.name) which will stringify ReactNode names to "[object Object]";
instead guard and produce a proper string title: if p.name is a string use it,
otherwise resolve the page by URL from source.getPages() (or the pages array)
and use extractFrontmatter(match).title as the fallback (and finally fallback to
an empty string); update the toLink mapping (referencing toLink, pages,
PageNavLink.title, Item.name, source.getPages(), and extractFrontmatter) to
implement this lookup and type-guard so non-string ReactNode names do not
produce "[object Object]".
In `@packages/chronicle/src/themes/default/Layout.tsx`:
- Around line 47-50: Breadcrumbs is always fed the docs tree so on API routes
getBreadcrumbItems can't find the path; update the Layout component to use the
isApiRoute flag (detected at isApiRoute) to either hide the Breadcrumbs or pass
the API tree instead of the docs tree: when isApiRoute is true, supply the API
page tree (the API tree variable) to the Breadcrumbs call (or skip rendering
Breadcrumbs entirely on API routes), ensuring getBreadcrumbItems receives the
correct tree for the slug computed by useMemo.
In `@packages/chronicle/src/themes/default/Page.tsx`:
- Around line 7-15: The theme no longer renders the table of contents because
the Toc component is not wired into the default layout; reintegrate Toc into the
layout so pages render a TOC (or document its removal). Update the default
theme’s Layout component (where Breadcrumbs was added) to import and render the
Toc component alongside or within the same container used for Breadcrumbs
(reference Toc and Breadcrumbs components and the Layout and Page/ThemePageProps
rendering flow), pass the page or page.toc data as needed, and ensure any
CSS/classes (styles.toc or styles.sidebar) are applied so the TOC appears
without breaking existing layout.
---
Nitpick comments:
In `@packages/chronicle/src/components/ui/client-theme-switcher.tsx`:
- Around line 19-34: When isClient is false the component returns null causing a
layout shift on mount; instead render a fixed-size placeholder that matches the
IconButton's outer dimensions so space is reserved (for example render the same
IconButton structure with visibility:hidden or a sized wrapper) inside the
ClientThemeSwitcher (use the existing isClient check and IconButton/size props
to mirror size), and either align the outer IconButton's size with the incoming
size prop or document why IconButton uses size={3} if you intentionally keep the
inner icon size independent.
In `@packages/chronicle/src/themes/default/Layout.module.css`:
- Around line 103-124: Update the theme-unaware hardcoded shadow and the
near-no-op backdrop filter: replace the rgba(...) shadow used in the .card rule
with a theme token (e.g., var(--rs-shadow-*, or an appropriate existing shadow
variable) so the elevation responds to dark/light themes and accessibility
settings, and for .subNav either increase the blur radius (e.g., to a
perceptible value) or remove the backdrop-filter property to avoid unnecessary
GPU work—change the rules in the .card and .subNav selectors accordingly.
In `@packages/chronicle/src/themes/default/Layout.tsx`:
- Around line 121-150: The sub-nav should be omitted when there's no navigation
to display; update the Layout component to only render the <nav
className={styles.subNav}> block when there is meaningful content (e.g. prev or
next exist, or Breadcrumbs would render). Compute a boolean like hasNav =
Boolean(prev || next || (slug && tree && tree.length)) and wrap the existing nav
(including the IconButton group and <Breadcrumbs slug={slug} tree={tree} />) in
a conditional render using that flag so the empty 48px sub-nav is not shown on
pages with no page data.
In `@packages/chronicle/src/themes/default/Page.module.css`:
- Around line 33-45: Remove the redundant margin-top: 0 from the .content h1
rule (or replace it with a comment/intent if you explicitly need to override the
:first-child reset) to avoid unintentionally removing top spacing for non-first
h1s; and change the .content p rule's line-height: 171.429% to a design token or
a unitless value (e.g., var(--rs-line-height-regular) or 1.71) so it matches the
file's styling conventions and improves readability.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 26555773-af54-4944-8cd9-4f4fc0f16ecd
📒 Files selected for processing (10)
packages/chronicle/src/components/ui/client-theme-switcher.tsxpackages/chronicle/src/components/ui/search.module.csspackages/chronicle/src/components/ui/search.tsxpackages/chronicle/src/lib/source.tspackages/chronicle/src/server/api/page.tspackages/chronicle/src/server/entry-server.tsxpackages/chronicle/src/themes/default/Layout.module.csspackages/chronicle/src/themes/default/Layout.tsxpackages/chronicle/src/themes/default/Page.module.csspackages/chronicle/src/themes/default/Page.tsx
💤 Files with no reviewable changes (1)
- packages/chronicle/src/components/ui/search.module.css
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/chronicle/src/server/entry-server.tsx
| const toLink = (p: (typeof pages)[number]): PageNavLink => ({ | ||
| url: p.url, | ||
| title: String(p.name ?? '') | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the shape of items returned by fumadocs-core/page-tree flattenTree in v16.6.15.
fd -t f 'package.json' node_modules/fumadocs-core 2>/dev/null | head -n 2
rg -nP --type=ts -C2 'export\s+(declare\s+)?(function|const)\s+flattenTree' node_modules/fumadocs-core 2>/dev/null
rg -nP --type=ts -C3 'name\s*:' node_modules/fumadocs-core/dist/page-tree 2>/dev/null | head -n 40Repository: raystack/chronicle
Length of output: 44
🏁 Script executed:
# First, let's look at the actual source.ts file around lines 115-118
cat -n packages/chronicle/src/lib/source.ts | sed -n '100,130p'Repository: raystack/chronicle
Length of output: 1318
🏁 Script executed:
# Check the package.json for fumadocs-core dependency
cat packages/chronicle/package.json | jq '.dependencies, .devDependencies' 2>/dev/null || cat packages/chronicle/package.json | grep -A 20 '"dependencies"'Repository: raystack/chronicle
Length of output: 1633
🏁 Script executed:
# Look for type definitions and imports related to PageNavLink, PageTree
rg -n 'PageNavLink|PageTree' packages/chronicle/src/lib/source.ts -B 3 -A 3Repository: raystack/chronicle
Length of output: 1076
🏁 Script executed:
# Check for fumadocs-core imports and usage
rg -n 'from.*fumadocs-core' packages/chronicle/src/lib/source.tsRepository: raystack/chronicle
Length of output: 296
🏁 Script executed:
# Find and read the types file
find packages/chronicle/src -name 'types.ts' -o -name 'types.d.ts' -o -name 'index.d.ts' | head -5Repository: raystack/chronicle
Length of output: 44
🏁 Script executed:
# Check the types directory or types file
cat packages/chronicle/src/types.ts 2>/dev/null || cat packages/chronicle/src/types/index.ts 2>/dev/null || find packages/chronicle/src -type f -name '*.ts' | xargs grep -l 'PageNavLink\|PageNav' | head -3Repository: raystack/chronicle
Length of output: 136
🏁 Script executed:
# Search for PageNavLink definition
rg -n 'PageNavLink' packages/chronicle/src -B 2 -A 2Repository: raystack/chronicle
Length of output: 2457
🏁 Script executed:
# Search for PageNav definition as well
rg -n 'interface PageNav|type PageNav' packages/chronicle/src -B 2 -A 5Repository: raystack/chronicle
Length of output: 811
🏁 Script executed:
# List the types directory structure
ls -la packages/chronicle/src/types/Repository: raystack/chronicle
Length of output: 470
🏁 Script executed:
# Check what Node type looks like when imported from fumadocs-core
rg -n 'Root|Node|Folder' packages/chronicle/src/lib/source.ts -A 3 | head -30Repository: raystack/chronicle
Length of output: 1293
🏁 Script executed:
# Search for any usage of page.name or how Node type items are accessed
rg -n '\.name' packages/chronicle/src/lib/source.ts -B 2 -A 2Repository: raystack/chronicle
Length of output: 210
🏁 Script executed:
# Let's check the fumadocs-core documentation or type stubs in the project
find packages/chronicle -name '*.d.ts' -o -name '*.ts' | xargs grep -l 'fumadocs' | xargs grep -l 'name.*ReactNode\|ReactNode.*name' 2>/dev/null | head -5Repository: raystack/chronicle
Length of output: 44
🏁 Script executed:
# Look for any comments or usage patterns around page names
rg -n 'p\.name|pages\[.*\]\.name' packages/chronicle/src -B 3 -A 3Repository: raystack/chronicle
Length of output: 1804
🌐 Web query:
fumadocs-core 16.6.15 PageTree Item name type ReactNode
💡 Result:
In fumadocs-core version 16.6.15, the PageTree.Item interface defines the "name" property as type ReactNode (import { ReactNode } from 'react'). This represents the display name of the page/navigation item, which can be a string, JSX element, or other React renderable content for flexible rendering in UI components like sidebars. The full Item interface is: interface Item { $id?: string; $ref?: { file: string }; type: 'page'; name: ReactNode; url: string; external?: boolean; description?: ReactNode; icon?: ReactNode; } This structure is consistent across fumadocs-core v16.x releases, including 16.6.15, as shown in official documentation and API references. PageTree nodes (Root, Folder, Separator) also use ReactNode for name and similar properties to support rich content.
Citations:
- 1: https://www.fumadocs.dev/docs/headless/page-tree
- 2: https://www.mintlify.com/fuma-nama/fumadocs/api/core/page-tree
- 3: https://www.mintlify.com/fuma-nama/fumadocs/concepts/page-tree
🌐 Web query:
fumadocs-core flattenTree return type name field documentation
💡 Result:
The flattenTree function from fumadocs-core/page-tree flattens a page tree into an array of page nodes. Its signature is function flattenTree(nodes: Node[]): Item[]. The return type is Item[], where Item is the interface for page nodes with fields like $id?: string, $ref?: { file: string }, type: 'page', name: ReactNode, url: string, external?: boolean, description?: ReactNode, and icon?: ReactNode. The name field in Item is of type ReactNode and represents the display name of the page.
Citations:
String(p.name) will stringify ReactNode names to "[object Object]".
In fumadocs-core 16.6.15, Item.name is typed as ReactNode, which can be a string, JSX element, or other React renderable content. When a page's name is a React element (e.g., decorated with an icon or custom formatting), String(...) produces "[object Object]" and that value flows through to PageNavLink.title.
The PageNavLink interface expects title: string. This is a latent bug that will surface whenever page names are anything other than simple strings.
Prefer deriving the title from the page's frontmatter title (matching what extractFrontmatter does) or guard for string-only names.
🛠 Proposed fix
- const toLink = (p: (typeof pages)[number]): PageNavLink => ({
- url: p.url,
- title: String(p.name ?? '')
- });
+ const toLink = (p: (typeof pages)[number]): PageNavLink => ({
+ url: p.url,
+ title: typeof p.name === 'string' ? p.name : (p.name == null ? '' : String(p.name))
+ });Or, for full fidelity, resolve the title from source.getPages() by URL and use extractFrontmatter(match).title.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const toLink = (p: (typeof pages)[number]): PageNavLink => ({ | |
| url: p.url, | |
| title: String(p.name ?? '') | |
| }); | |
| const toLink = (p: (typeof pages)[number]): PageNavLink => ({ | |
| url: p.url, | |
| title: typeof p.name === 'string' ? p.name : (p.name == null ? '' : String(p.name)) | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/chronicle/src/lib/source.ts` around lines 115 - 118, toLink
currently forces PageNavLink.title by doing String(p.name) which will stringify
ReactNode names to "[object Object]"; instead guard and produce a proper string
title: if p.name is a string use it, otherwise resolve the page by URL from
source.getPages() (or the pages array) and use extractFrontmatter(match).title
as the fallback (and finally fallback to an empty string); update the toLink
mapping (referencing toLink, pages, PageNavLink.title, Item.name,
source.getPages(), and extractFrontmatter) to implement this lookup and
type-guard so non-string ReactNode names do not produce "[object Object]".
| export function Page({ page }: ThemePageProps) { | ||
| return ( | ||
| <Flex className={styles.page}> | ||
| <article className={styles.article} data-article-content> | ||
| <Breadcrumbs slug={page.slug} tree={tree} /> | ||
| <div className={styles.content}>{page.content}</div> | ||
| </article> | ||
| <Toc items={page.toc} /> | ||
| </Flex> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether Toc is still rendered anywhere in the default theme.
rg -nP --type=ts --type=tsx -C2 '\b(Toc|toc)\b' packages/chronicle/src/themes/default
rg -nP -C2 "from\\s+['\"]\\S*Toc" packages/chronicle/srcRepository: raystack/chronicle
Length of output: 928
🏁 Script executed:
cat -n packages/chronicle/src/themes/default/index.tsRepository: raystack/chronicle
Length of output: 360
🏁 Script executed:
cat -n packages/chronicle/src/themes/default/Layout.tsx | head -80Repository: raystack/chronicle
Length of output: 3304
🏁 Script executed:
rg -n "Toc" packages/chronicle/src --type=ts --type=tsx | grep -E "(render|<Toc|import.*Toc)" | head -20Repository: raystack/chronicle
Length of output: 89
🏁 Script executed:
# Check if there are any parent components that might use the theme's Toc export
rg -n "themes.*Toc|theme\.Toc" packages/chronicle/src --type=ts --type=tsxRepository: raystack/chronicle
Length of output: 89
🏁 Script executed:
rg -n "<Toc" packages/chronicle/src | head -20Repository: raystack/chronicle
Length of output: 153
🏁 Script executed:
rg -n "Toc" packages/chronicle/src/themes/default/Toc.tsx -A 5 | head -40Repository: raystack/chronicle
Length of output: 684
TOC is not rendered anywhere in the theme and should be integrated into the layout.
The Toc component exists but is not part of defaultTheme and is not instantiated anywhere. If TOC was previously displayed on pages, this represents a regression. The PR mentions verifying "no regressions in TOC"—either wire Toc back into Layout.tsx (similar to how Breadcrumbs was integrated), or update the PR scope to explicitly document that TOC display has been removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/chronicle/src/themes/default/Page.tsx` around lines 7 - 15, The
theme no longer renders the table of contents because the Toc component is not
wired into the default layout; reintegrate Toc into the layout so pages render a
TOC (or document its removal). Update the default theme’s Layout component
(where Breadcrumbs was added) to import and render the Toc component alongside
or within the same container used for Breadcrumbs (reference Toc and Breadcrumbs
components and the Layout and Page/ThemePageProps rendering flow), pass the page
or page.toc data as needed, and ensure any CSS/classes (styles.toc or
styles.sidebar) are applied so the TOC appears without breaking existing layout.
TOC renders as short horizontal markers floating right-center, each width scaled from the heading text length (base + per char, clamped). Hovering the aside fades the markers out and reveals a panel with the full heading list, active state highlighted. Shadows switch to the Apsara --rs-shadow-soft token. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- page-context: coerce prev/next from API response to null so the state always matches the Page type instead of carrying undefined. - source.getPageNav: guard PageNavLink.title to string-only page names so ReactNode names don't stringify to "[object Object]". - Layout tabs: compute per-entry active state from api.basePath instead of the shared isApiRoute boolean, so only the matching API tab highlights when multiple APIs are configured. - Layout: skip breadcrumbs on API routes since the docs tree doesn't include API paths. - Search: add title="Search (⌘K)" so the keyboard shortcut stays discoverable after the kbd hint was removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
Test plan