Open
Conversation
- Sidebar header: plain title (single content) or Select dropdown (multi-content) - VersionSwitcher pinned to sidebar footer, hidden if no versions - ChapterNav: remove numbering, Figma-matching typography tokens - Add Departure Mono (self-hosted) and Hanuman (Google Fonts) - Font CSS variables: --paper-font-mono, --paper-font-body - Nav items: proper padding, border-radius, accent active state - Section labels: uppercase mono, secondary color, rs-space-8 gap Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use --rs-letter-spacing-mini for title - Share .title class between plain title and Select.Value Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Navbar: Apsara IconButton arrows, chevron breadcrumb separators, settings icon - Navbar: sticky at top:0 on scroll, blur bg, 0.5px border - Content: full width with max-width 1024px, content-box sizing - Content: padding-right for TOC space, padding-top for initial offset - Layout: height 100vh on content for proper scroll container - Remove search button from navbar (keyboard-only) - Add scroll content to versioned example Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Click settings icon to show eye (reader mode), sun (theme), cross (close) - Click cross to collapse back to settings icon - No functionality wired yet, UI only Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Toggle dark/light via sun/moon icon in settings panel - Enable system theme for paper theme (remove forcedTheme: light) - Hydration-safe: only render after client mount Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…paragraph line-height - Breadcrumb: use --paper-font-mono, medium weight - Content: switch from Georgia to --paper-font-body (Hanuman) - Paragraph: line-height 2 - Main: width 90% Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- ReaderModeContext: shared state between Layout and Page - Eye icon toggles reader mode via context - Layout hides sidebar when reader mode active - Page hides ReadingProgress and removes right padding - Content centers in reader mode Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add description field to contentEntrySchema - Pass through ContentRoot → LandingEntry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Header: large mono title + description side by side, border bottom - Cards: dotted grid bg, folder icon, edge labels (FIG_001, [ label ]) - Theme-aware fonts via --paper-font-mono/body with fallbacks - Card hover: icon scale + opacity + bg shift - Add 4 content dirs to versioned example with descriptions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 (3)
📝 WalkthroughWalkthroughThis PR redesigns the paper documentation theme and extends the Chronicle system with reading time metadata tracking. Updates include: switching the default theme to paper in example configurations, adding description fields to content sections and landing entries, integrating the remark-reading-time plugin for MDX, redesigning the landing page with updated card layouts and folder icons, implementing a reader mode toggle with theme switching, and refactoring the paper theme's layout, navigation, and page styling to use sticky positioning, design tokens, and responsive patterns. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 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. Review rate limit: 0/1 reviews remaining, refill in 35 minutes and 37 seconds.Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/pages/LandingPage.module.css`:
- Around line 27-51: The fixed-width rules (the selector with "width: 385px" and
the .card rule with "width: 360px") cause horizontal overflow on small screens;
add responsive overrides using media queries (e.g., `@media` (max-width: 600px)
and/or `@media` (max-width: 400px)) to set those widths to 100% or max-width: 100%
and remove rigid spacing: for the selector with width: 385px change to width:
auto / max-width: 100% and allow word-wrap; for .card set width: 100%, flex: 1 1
auto (or remove flex-shrink), and reduce gap/margin in the .grid (or change gap
to a smaller var) inside the same media queries so header text and cards
collapse cleanly without horizontal scrolling.
In `@packages/chronicle/src/pages/LandingPage.tsx`:
- Around line 26-38: The decorative image labels and icon inside the card are
being included in the link’s accessible name; update the JSX so purely visual
elements are removed from AT by adding aria-hidden="true" (or
role="presentation") to the spans using classNames cardImageLabel,
cardImageLabelTop, cardImageLabelRight and to the FolderIcon (cardIcon), leaving
the semantic label/description spans (cardLabel, cardDescription) intact so
screen readers still read the meaningful content; ensure you apply the attribute
on the exact elements shown in the LandingPage component to prevent them from
contributing to the link's accessible name.
In `@packages/chronicle/src/themes/paper/Page.module.css`:
- Around line 1-13: The .main rule currently uses box-sizing: content-box so the
padding-right (the TOC gutter) is added on top of width: 90%, causing overflow;
change .main to use box-sizing: border-box (or alternatively adjust width with
calc to subtract var(--rs-space-17)) so padding is included within the 90%
width, and keep .readerMode's padding-right: 0 behavior for that modifier;
update the CSS in Page.module.css targeting the .main and .readerMode selectors
accordingly.
In `@packages/chronicle/src/themes/paper/Page.tsx`:
- Around line 93-117: The settings open/close icon-only buttons lack accessible
names; add aria-labels (or aria-labelledby) to the IconButton that renders
AdjustmentsHorizontalIcon (when settingsOpen is false) and to the IconButton
that renders XMarkIcon (when settingsOpen is true) so screen readers announce
them (e.g., "Open settings" / "Close settings"); keep existing aria-labels on
the reader-mode IconButton (toggleReaderMode) and the theme toggle
(setTheme/resolvedTheme/isClient) unchanged.
- Around line 54-74: The current code wraps IconButton (native <button>) inside
RouterLink (<a>), creating nested interactive elements; update the navigation
markup so the link is the interactive control instead of wrapping a button: for
the prev and next cases render RouterLink (prev.url / next.url) as the clickable
element and style it to look like the IconButton (using the same size/aria-label
and placing ArrowLeftIcon / ArrowRightIcon inside), or alternatively keep
IconButton but remove RouterLink and perform navigation in its onClick (e.g.,
history.push) so you no longer nest <a> and <button>; adjust both branches where
prev and next are handled (references: IconButton, RouterLink, ArrowLeftIcon,
ArrowRightIcon, prev, next).
🪄 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: e471985a-de25-424b-a42b-caeb7a353d1f
⛔ Files ignored due to path filters (1)
packages/chronicle/src/themes/paper/fonts/DepartureMono-Regular.woff2is excluded by!**/*.woff2
📒 Files selected for processing (17)
examples/basic/chronicle.yamlexamples/versioned/chronicle.yamlexamples/versioned/content/design/index.mdxexamples/versioned/content/docs/index.mdxexamples/versioned/content/ops/index.mdxpackages/chronicle/src/lib/config.tspackages/chronicle/src/pages/LandingPage.module.csspackages/chronicle/src/pages/LandingPage.tsxpackages/chronicle/src/themes/paper/ChapterNav.module.csspackages/chronicle/src/themes/paper/ChapterNav.tsxpackages/chronicle/src/themes/paper/Layout.module.csspackages/chronicle/src/themes/paper/Layout.tsxpackages/chronicle/src/themes/paper/Page.module.csspackages/chronicle/src/themes/paper/Page.tsxpackages/chronicle/src/themes/paper/ReaderModeContext.tsxpackages/chronicle/src/themes/registry.tspackages/chronicle/src/types/config.ts
rsbh
changed the title
- font-weight: 400 → --rs-font-weight-regular - font-size: 10px → --rs-font-size-micro - font-size: 16px → --rs-font-size-large - gap: 10px → --rs-space-3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove nested <a>/<button> — use RouterLink directly for nav arrows - Add aria-labels to settings open/close buttons - Add aria-hidden to decorative card image - Fix content-box overflow with calc() max-width - Add mobile breakpoints for landing page (768px) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Render frontmatter title + description centered above content - Add separator line between header and content - Move header outside data-article-content (exclude from TOC) - Remove overflow-y/height from content (use natural body scroll) - Fix TOC heading click to update URL hash via history.pushState Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add remark-reading-time plugin with valueToExport for MDX module export - Pass _readingTime from module through loadPageModule → frontmatter - Display reading time in paper theme article header - Reading time style: Departure Mono, tertiary color, 12px - Content padding-top: --rs-space-15 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/chronicle/src/themes/paper/Page.tsx (1)
30-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard the unmatched-path case before computing
next.If
findIndexreturns-1, Line 40 currently exposespages[0]as the forward link. Please returnnullnavigation when the pathname is missing, or add the samecurrentIndex >= 0guard to thenextbranch.Suggested patch
return { prev: currentIndex > 0 ? pages[currentIndex - 1] : null, - next: currentIndex < pages.length - 1 ? pages[currentIndex + 1] : null, + next: + currentIndex >= 0 && currentIndex < pages.length - 1 + ? pages[currentIndex + 1] + : null, crumbs: breadcrumbItems.map(item => ({ label: item.name, href: item.url ?? pathname,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/themes/paper/Page.tsx` around lines 30 - 40, The prev/next computation uses currentIndex = pages.findIndex(...) but doesn't guard the case currentIndex === -1, causing pages[0] to be returned as next for unmatched paths; update the return object in the useMemo block (the prev/next calculation around currentIndex and pages) to return null for both prev and next when currentIndex < 0 (or at minimum add currentIndex >= 0 to the next branch), so that when pathname is not found you set prev: null and next: null instead of indexing pages.
🧹 Nitpick comments (1)
packages/chronicle/src/themes/paper/ReaderModeContext.tsx (1)
10-13: ⚡ Quick winFail fast when
useReaderModeis used outside the provider.Current defaults make missing provider wiring silently degrade at runtime. Prefer a nullable context and explicit error in the hook.
Suggested change
-interface ReaderModeContextValue { +interface ReaderModeContextValue { readerMode: boolean; toggleReaderMode: () => void; } -const ReaderModeContext = createContext<ReaderModeContextValue>({ - readerMode: false, - toggleReaderMode: () => {}, -}); +const ReaderModeContext = createContext<ReaderModeContextValue | null>(null); @@ export function useReaderMode() { - return useContext(ReaderModeContext); + const ctx = useContext(ReaderModeContext); + if (!ctx) { + throw new Error('useReaderMode must be used within ReaderModeProvider'); + } + return ctx; }Also applies to: 26-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/themes/paper/ReaderModeContext.tsx` around lines 10 - 13, The context currently uses a non-null default which masks missing provider wiring; change createContext<ReaderModeContextValue | null>(null) for ReaderModeContext so the value can be null, then update the consumer hook (useReaderMode) to read ReaderModeContext and throw a clear error if the value is null (e.g., "useReaderMode must be used within ReaderModeProvider"), and ensure any Provider (ReaderModeProvider) still supplies a proper ReaderModeContextValue; also update any related types/usages that assumed a non-null context.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/versioned/content/design/index.mdx`:
- Around line 7-9: Remove the in-body H1 titled "# Design System" from the
document so the frontmatter-rendered title isn't duplicated; keep the remaining
body content ("Design system documentation placeholder.") and any other sections
intact so the layout uses the single title from the page frontmatter rather than
an extra heading in the markdown.
In `@examples/versioned/content/ops/index.mdx`:
- Around line 7-9: Remove the in-body H1 heading that duplicates the frontmatter
title by deleting the line "# Ops Guide" in the file index.mdx (the visible page
title is now rendered from frontmatter in the paper layout), leaving only the
placeholder content "Operations documentation placeholder." so the page shows a
single title from the frontmatter.
In `@packages/chronicle/src/lib/source.ts`:
- Around line 243-244: The current return logic treats minutes falsily (so 0
becomes missing); in the expression using mod.readingTime?.minutes, change the
check to a nullish test so zero is preserved: compute minutes =
mod.readingTime?.minutes and set _readingTime to minutes == null ? undefined :
Math.max(1, Math.round(minutes)); update the return in the function that builds
the object (the line referencing mod.default, mod.toc and _readingTime)
accordingly.
---
Outside diff comments:
In `@packages/chronicle/src/themes/paper/Page.tsx`:
- Around line 30-40: The prev/next computation uses currentIndex =
pages.findIndex(...) but doesn't guard the case currentIndex === -1, causing
pages[0] to be returned as next for unmatched paths; update the return object in
the useMemo block (the prev/next calculation around currentIndex and pages) to
return null for both prev and next when currentIndex < 0 (or at minimum add
currentIndex >= 0 to the next branch), so that when pathname is not found you
set prev: null and next: null instead of indexing pages.
---
Nitpick comments:
In `@packages/chronicle/src/themes/paper/ReaderModeContext.tsx`:
- Around line 10-13: The context currently uses a non-null default which masks
missing provider wiring; change createContext<ReaderModeContextValue |
null>(null) for ReaderModeContext so the value can be null, then update the
consumer hook (useReaderMode) to read ReaderModeContext and throw a clear error
if the value is null (e.g., "useReaderMode must be used within
ReaderModeProvider"), and ensure any Provider (ReaderModeProvider) still
supplies a proper ReaderModeContextValue; also update any related types/usages
that assumed a non-null context.
🪄 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: db604702-c8e7-474c-8cd6-453382d17412
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpackages/chronicle/src/themes/paper/fonts/DepartureMono-Regular.woff2is excluded by!**/*.woff2
📒 Files selected for processing (23)
examples/basic/chronicle.yamlexamples/versioned/chronicle.yamlexamples/versioned/content/design/index.mdxexamples/versioned/content/docs/index.mdxexamples/versioned/content/ops/index.mdxpackage.jsonpackages/chronicle/src/lib/config.tspackages/chronicle/src/lib/source.tspackages/chronicle/src/pages/LandingPage.module.csspackages/chronicle/src/pages/LandingPage.tsxpackages/chronicle/src/server/entry-server.tsxpackages/chronicle/src/server/vite-config.tspackages/chronicle/src/themes/paper/ChapterNav.module.csspackages/chronicle/src/themes/paper/ChapterNav.tsxpackages/chronicle/src/themes/paper/Layout.module.csspackages/chronicle/src/themes/paper/Layout.tsxpackages/chronicle/src/themes/paper/Page.module.csspackages/chronicle/src/themes/paper/Page.tsxpackages/chronicle/src/themes/paper/ReaderModeContext.tsxpackages/chronicle/src/themes/paper/ReadingProgress.tsxpackages/chronicle/src/themes/registry.tspackages/chronicle/src/types/config.tspackages/chronicle/src/types/content.ts
…heck - Remove in-body H1 from example MDX (frontmatter title renders it) - Use nullish check for readingTime minutes (0 normalizes to 1) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
descriptionfield to content entry schemaTest plan
🤖 Generated with Claude Code