Refactor/separate docs components pages#29
Conversation
Simplify docs layout and slug page structure, align components chrome with wider max width and tighter padding, and extend Next config as needed. Made-with: Cursor
… to fix hydration mismatch
|
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 46 minutes and 1 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe application's navigation and layout architecture is restructured to consolidate the separate docs and components sections. Docs routes now redirect to components. A unified horizontal navigation header ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/www/app/templates/layout.tsx (1)
62-83:⚠️ Potential issue | 🟡 MinorExpose the active section in the templates nav.
The active link here is only styled visually. Adding
aria-current="page"keeps this header consistent with the docs/components layouts and gives screen readers the current-page cue.Suggested fix
<Link key={href} href={href} + aria-current={isActive ? "page" : undefined} className={cn( "relative flex items-center gap-1.5 px-3 py-1.5 rounded-lg text-sm font-medium transition-colors",As per coding guidelines, "Include standard HTML / ARIA attributes where applicable for accessibility."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/app/templates/layout.tsx` around lines 62 - 83, The nav Link currently only indicates active state visually; update the Link element to expose the active section to assistive tech by adding an ARIA attribute—set aria-current="page" when isActive (e.g., aria-current={isActive ? "page" : undefined} on the Link). Locate the Link component block that renders the motion.span with layoutId="templatesNavPill" and Icon/label children and add the conditional aria-current attribute without changing the visual behavior or existing motion.span/layoutId.
🧹 Nitpick comments (2)
apps/www/app/docs/[slug]/page.tsx (1)
4-14: Drop one of the two/docs/:slugredirect layers.
apps/www/next.config.tsalready redirects/docs/:slugto/components/:slugbefore this route is resolved, so keeping this page means extra route/build surface and two redirect definitions to maintain. If the config redirect is the intended permanent behavior, I’d remove this route file entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/app/docs/`[slug]/page.tsx around lines 4 - 14, This page defines a second redirect layer for /docs/:slug (functions generateStaticParams, dynamicParams and the DocSlugRedirect component calling redirect) which duplicates the redirect already in apps/www/next.config.ts; remove this route file (apps/www/app/docs/[slug]/page.tsx) so only the config-based redirect remains, or alternatively remove the config redirect and keep this file — but do not keep both; delete the file containing generateStaticParams/dynamicParams/DocSlugRedirect to drop the duplicate redirect.apps/www/app/components/layout.tsx (1)
37-133: Consider extracting the shared section header into one component.This header markup now exists in the components, docs, and templates layouts with only small variations, so nav/theme/accessibility changes will keep drifting between files. Centralizing it would make this refactor easier to maintain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/www/app/components/layout.tsx` around lines 37 - 133, The topHeader JSX is duplicated across layouts — extract it into a single component (e.g., SharedTopHeader or TopHeader) that receives the needed props (isDark, pathname, isOverview, isMobileMenuOpen, setIsMobileMenuOpen) and renders the same structure using SECTION_NAV, motion, ThemeToggle, ArrowLeft, Menu, X, etc.; replace the local topHeader variable with a call to this new component and export it for reuse in components, docs, and templates so future nav/theme/accessibility changes are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/app/components/layout.tsx`:
- Around line 117-128: The mobile menu toggle button is missing accessible
attributes; update the button rendered in the component (the element using
setIsMobileMenuOpen, isMobileMenuOpen, Menu and X) to include a descriptive
aria-label (e.g., "Toggle component list"), aria-expanded={isMobileMenuOpen} to
reflect state, and aria-controls pointing to the sidebar's id (add an id like
"component-sidebar" to the sidebar container if not present) so assistive tech
knows the relationship; apply the same attributes to the other toggle instance
referenced in the file (the block near lines 177-187).
In `@apps/www/app/components/page.tsx`:
- Around line 197-202: The filter count element rendered in motion.p
(key={`${search}-${activeCategory}`}) isn't announced to assistive tech; make it
a live region by adding ARIA attributes: set aria-live="polite",
aria-atomic="true" and role="status" on that motion.p (or wrap the count in an
element with those attributes) so screen readers reliably announce updates when
search or activeCategory changes.
In `@apps/www/contexts/theme-context.tsx`:
- Around line 36-42: The storage event handler onStorage updates themeCache and
calls onStoreChange but doesn't update the DOM theme attribute; modify onStorage
to, after validating s === "light" || s === "dark", set
document.documentElement.dataset.theme = s in addition to assigning themeCache
and calling onStoreChange so the receiving tab's CSS (selectors using
[data-theme]) updates immediately; ensure you reference STORAGE_KEY and the
onStorage handler (and keep existing themeCache and onStoreChange behavior).
---
Outside diff comments:
In `@apps/www/app/templates/layout.tsx`:
- Around line 62-83: The nav Link currently only indicates active state
visually; update the Link element to expose the active section to assistive tech
by adding an ARIA attribute—set aria-current="page" when isActive (e.g.,
aria-current={isActive ? "page" : undefined} on the Link). Locate the Link
component block that renders the motion.span with layoutId="templatesNavPill"
and Icon/label children and add the conditional aria-current attribute without
changing the visual behavior or existing motion.span/layoutId.
---
Nitpick comments:
In `@apps/www/app/components/layout.tsx`:
- Around line 37-133: The topHeader JSX is duplicated across layouts — extract
it into a single component (e.g., SharedTopHeader or TopHeader) that receives
the needed props (isDark, pathname, isOverview, isMobileMenuOpen,
setIsMobileMenuOpen) and renders the same structure using SECTION_NAV, motion,
ThemeToggle, ArrowLeft, Menu, X, etc.; replace the local topHeader variable with
a call to this new component and export it for reuse in components, docs, and
templates so future nav/theme/accessibility changes are made in one place.
In `@apps/www/app/docs/`[slug]/page.tsx:
- Around line 4-14: This page defines a second redirect layer for /docs/:slug
(functions generateStaticParams, dynamicParams and the DocSlugRedirect component
calling redirect) which duplicates the redirect already in
apps/www/next.config.ts; remove this route file
(apps/www/app/docs/[slug]/page.tsx) so only the config-based redirect remains,
or alternatively remove the config redirect and keep this file — but do not keep
both; delete the file containing
generateStaticParams/dynamicParams/DocSlugRedirect to drop the duplicate
redirect.
🪄 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: 9a7c493b-6871-4047-91c3-62a89970aab0
📒 Files selected for processing (7)
apps/www/app/components/layout.tsxapps/www/app/components/page.tsxapps/www/app/docs/[slug]/page.tsxapps/www/app/docs/layout.tsxapps/www/app/templates/layout.tsxapps/www/contexts/theme-context.tsxapps/www/next.config.ts
| const onStorage = (e: StorageEvent) => { | ||
| if (e.key !== STORAGE_KEY) return; | ||
| try { | ||
| const s = localStorage.getItem(STORAGE_KEY) as Theme | null; | ||
| if (s === "light" || s === "dark") { | ||
| themeCache = s; | ||
| onStoreChange(); |
There was a problem hiding this comment.
Sync data-theme inside the storage listener.
The cross-tab path updates themeCache and re-renders subscribers, but it never updates document.documentElement.dataset.theme in the receiving tab. Any CSS driven by [data-theme] stays stale until the user toggles locally or reloads.
Suggested fix
const s = localStorage.getItem(STORAGE_KEY) as Theme | null;
if (s === "light" || s === "dark") {
themeCache = s;
- onStoreChange();
+ document.documentElement.dataset.theme = s;
+ emit();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/www/contexts/theme-context.tsx` around lines 36 - 42, The storage event
handler onStorage updates themeCache and calls onStoreChange but doesn't update
the DOM theme attribute; modify onStorage to, after validating s === "light" ||
s === "dark", set document.documentElement.dataset.theme = s in addition to
assigning themeCache and calling onStoreChange so the receiving tab's CSS
(selectors using [data-theme]) updates immediately; ensure you reference
STORAGE_KEY and the onStorage handler (and keep existing themeCache and
onStoreChange behavior).
1 participant
PR Title
fix(www): resolve theme hydration mismatch in app layoutType of Change
Description
Fixed a hydration mismatch caused by theme initialization diverging between server and client renders.
The previous theme state logic could render
"dark"on the server but"light"on initial client hydration (fromlocalStorage), which caused React to regenerate the tree on the client.What changed
apps/www/contexts/theme-context.tsxto use a hydration-safe theme synchronization approach withuseSyncExternalStore.data-theme) and subscribers.This removes the hydration warning and keeps theme behavior consistent.
Testing
cd apps/www && pnpm lintcd apps/www && pnpm build/componentsrender path.Checklist (for New Components)
If adding a new component, please ensure you've completed the following:
registry/{component-name}.tsxwith the source code.registry/config.tsspecifying dependencies and Tailwind configuration.pnpm build:registryto regenerateregistry.json.apps/www/app/page.tsx.motion(Motion.dev).classNameprop (cnutility).Related Issues
Fixes hydration mismatch on
/componentscaused by SSR/client theme state divergence.Summary by CodeRabbit
Release Notes
New Features
Improvements