-
Notifications
You must be signed in to change notification settings - Fork 264
Docs homepage redesign #1718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Docs homepage redesign #1718
Conversation
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More Details
Vulnerabilities [next:14.2.21]
Name | Severity | Source | Fixed version | CVSS score | CVSS exploitability score | Has public exploit | Has CISA KEV exploit |
---|---|---|---|---|---|---|---|
CVE-2025-29927 | GHSA-f82v-jwr5-mffw | 14.2.25 | 9.1 | 3.9 | true | false | |
CVE-2025-32421 | GHSA-qpjv-v59x-3qc4 | 14.2.24 | 3.7 | 2.2 | false | false | |
CVE-2025-48068 | GHSA-3h52-269p-cp9r | 14.2.30 | 2.3 | - | false | false |
To ignore this finding as an exception, reply to this conversation with #wiz_ignore reason
If you'd like to ignore this finding in all future scans, add an exception in the .wiz file (learn more) or create an Ignore Rule (learn more).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More Details
Vulnerabilities [elliptic:6.5.4]
Name | Severity | Source | Fixed version | CVSS score | CVSS exploitability score | Has public exploit | Has CISA KEV exploit |
---|---|---|---|---|---|---|---|
CVE-2024-42459 | GHSA-f7q4-pwc6-w24p | 6.5.7 | 5.3 | 3.9 | false | false | |
CVE-2024-42460 | GHSA-977x-g7h5-7qgw | 6.5.7 | 5.3 | 3.9 | false | false | |
CVE-2024-42461 | GHSA-49q7-c7j4-3p7m | 6.5.7 | 9.1 | 3.9 | false | false | |
CVE-2024-48948 | GHSA-fc9h-whq2-v747 | 6.6.0 | 4.8 | 2.2 | false | false | |
CVE-2024-48949 | GHSA-434g-2637-qmqr | 6.5.6 | 9.1 | 3.9 | false | false | |
GHSA-vjh7-7g9h-fjfh | GHSA-vjh7-7g9h-fjfh | 6.6.1 | 9.0 | - | false | false |
To ignore this finding as an exception, reply to this conversation with #wiz_ignore reason
If you'd like to ignore this finding in all future scans, add an exception in the .wiz file (learn more) or create an Ignore Rule (learn more).
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a new responsive CustomHeader component and wires it via theme.config.tsx, replacing prior logo/search/navbar config. Introduces a LoadingBar component that listens to Next.js route events and is rendered globally in pages/_app.tsx. Enhances AskAIButton with size/width/id props. Updates Search components: centers DocSearch overlay, adds hover-based active selection, and propagates Algolia queryID; exports AlgoliaHit type. Adds @heroicons/react dependency. Adds CLAUDE.md repository guide. Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Router as Next.js Router
participant LoadingBar as LoadingBar
Note over LoadingBar: Subscribes to routeChangeStart/Complete/Error
User->>Router: Navigate (link, search, etc.)
Router-->>LoadingBar: routeChangeStart
activate LoadingBar
LoadingBar->>LoadingBar: show bar, progress -> 0% then 70%
Router-->>LoadingBar: routeChangeComplete | routeChangeError
LoadingBar->>LoadingBar: progress -> 100%, hide after delay
deactivate LoadingBar
sequenceDiagram
autonumber
participant User
participant Header as CustomHeader
participant Router as Next.js Router
participant Theme as Theme Provider
Note over Header: Renders in navbar via theme.config.tsx
User->>Header: Click nav link
Header->>Router: router.push(prefetched route)
Router-->>Header: routeChangeStart/Complete (for dimming state)
User->>Header: Toggle theme
Header->>Theme: setTheme('light'|'dark')
Theme-->>Header: updated theme
sequenceDiagram
autonumber
participant User
participant Search as Search (DocSearch)
participant Algolia as Algolia API
participant App as App Context
User->>Search: Type query
Search->>Algolia: fetch hits
Algolia-->>Search: hits + optional queryID
Search->>App: setQueryID(queryID)
User->>Search: Hover/Focus result
Search->>Router: navigate to selected slug
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/AskAIButton.tsx (1)
36-49
: Add type="button" and address static default ID in AskAIButton
- components/AskAIButton.tsx—at the
<button>
declaration (lines 36–49):
• Addtype="button"
to prevent accidental form submissions.
• Theid
prop defaults to'custom-ask-ai-button'
. We found no code querying this ID, but a static default can collide if multiple instances render. Consider:
– Removing the default so callers must supply a uniqueid
.
– Generating a unique ID per instance (e.g., via React’suseId()
).Apply:
- <button - id={id} + <button + type="button" + id={id} className={`${baseClasses} ${sizeClasses} ${widthClasses}`}
🧹 Nitpick comments (14)
package.json (1)
31-31
: Remove unused @heroicons/react dependencyI couldn’t find any imports of
@heroicons/react
in the codebase, but I did find one import of@remixicon/react
:
- components/AskAIButton.tsx:1
import { RiSparkling2Fill } from '@remixicon/react';Unless you plan to add Heroicons, you can remove
@heroicons/react
frompackage.json
(and your lockfile) to avoid shipping unused code—and keep only the one icon library in use.components/AskAIButton.tsx (1)
39-45
: Prefer theme tokens/classes over inline styles for consistency and themingInline brand color and layout styles make theming and dark-mode tweaks harder. Consider replacing with design tokens or Tailwind/Nextra classes (e.g., using a CSS var like var(--primary-600) and utility classes for layout).
Example:
- style={{ - backgroundColor: '#FF0420', - color: 'white', - display: 'flex', - alignItems: 'center', - gap: '4px' - }} + style={{ backgroundColor: 'var(--primary-600)' }} + className={`${baseClasses} ${sizeClasses} ${widthClasses} nx-text-white nx-flex nx-items-center nx-gap-1`}pages/_app.tsx (1)
35-36
: LoadingBar and CustomHeader mobile overlay share z-index:50, causing stacking conflictsOur search shows:
- components/LoadingBar/index.tsx: line 46 uses
nx-z-50
- components/CustomHeader/index.tsx: line 188 (mobile menu overlay) also uses
nx-z-50
With both at the same z-index, the overlay can obscure the bar (or vice versa, depending on render order). To ensure the loading bar always sits above interactive elements:
• Bump the LoadingBar’s z-index above 50
components/LoadingBar/index.tsx ──────────────────────────────────────────────────────────── - <div className="nx-fixed nx-top-0 nx-left-0 nx-right-0 nx-z-50 nx-h-1 …"> + <div className="nx-fixed nx-top-0 nx-left-0 nx-right-0 nx-z-[60] nx-h-1 …">• (Alternatively) Lower the CustomHeader overlay’s z-index below 50
components/CustomHeader/index.tsx ──────────────────────────────────────────────────────────── - className="… nx-z-50 …" + className="… nx-z-40 …"Please update one of these to resolve the overlap.
components/LoadingBar/index.tsx (2)
34-41
: Defensive cleanup for timersGiven timers are nullable, guard clears in cleanup.
- clearTimeout(progressTimer); - clearTimeout(finishTimer); + if (progressTimer) clearTimeout(progressTimer); + if (finishTimer) clearTimeout(finishTimer);
46-51
: Add ARIA attributes for accessibilityExpose progress semantics for assistive tech.
- <div className="nx-fixed nx-top-0 nx-left-0 nx-right-0 nx-z-50 nx-h-1 nx-bg-transparent"> + <div className="nx-fixed nx-top-0 nx-left-0 nx-right-0 nx-z-50 nx-h-1 nx-bg-transparent" role="progressbar" aria-valuemin={0} aria-valuemax={100} aria-valuenow={progress}>theme.config.tsx (1)
69-115
: Gate Kapa widget script on a valid website ID to avoid broken initializationRight now the script renders when the feature flag is on, even if NEXT_PUBLIC_KAPA_WEBSITE_ID is missing (empty string is passed). Safer to render only when both the flag is enabled and the env var is present.
Apply this diff to guard the script:
head: () => { const { asPath, defaultLocale, locale } = useRouter(); const { frontMatter } = useConfig(); const url = 'https://docs.optimism.io' + (defaultLocale === locale ? asPath : `/${locale}${asPath}`); - const enableDocsAIWidget = useFeature('enable_docs_ai_widget').on; + const enableDocsAIWidget = useFeature('enable_docs_ai_widget').on; + const websiteId = process.env.NEXT_PUBLIC_KAPA_WEBSITE_ID; + const shouldLoadKapa = enableDocsAIWidget && Boolean(websiteId); return ( <> <meta property='og:url' content={url} /> @@ - {enableDocsAIWidget && ( + {shouldLoadKapa && ( <script async src='https://widget.kapa.ai/kapa-widget.bundle.js' - data-website-id={process.env.NEXT_PUBLIC_KAPA_WEBSITE_ID || ''} + data-website-id={websiteId} data-project-name='OP Labs' data-project-color='#FF0420' data-modal-title='Optimism Docs Assistant ✨' data-project-logo='https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcR8nhCmw7cu6jVQI01JFtMAV5tkTNLJXMSAOg&s' data-modal-example-questions='What is the OP Stack?, How do I get started with Supersim?, How do I create a SuperERC20 token?, How do I get faucet funds?'components/Search/index.tsx (3)
12-18
: Fail fast if Algolia env vars are not setUsing empty strings for Algolia credentials can lead to confusing runtime failures. Consider validating configuration once and disabling search gracefully when misconfigured.
Example guard:
-const client = algoliasearch( - process.env.NEXT_PUBLIC_ALGOLIA_APPLICATION_ID || '', - process.env.NEXT_PUBLIC_ALGOLIA_SEARCH_API_KEY || '' -); +const appId = process.env.NEXT_PUBLIC_ALGOLIA_APPLICATION_ID; +const apiKey = process.env.NEXT_PUBLIC_ALGOLIA_SEARCH_API_KEY; +if (!appId || !apiKey) { + // Optional: console.warn('Algolia env vars missing: search disabled.'); +} +const client = algoliasearch(appId || 'missing', apiKey || 'missing'); -const index = client.initIndex(process.env.NEXT_PUBLIC_ALGOLIA_INDEX_NAME || 'docs'); +const INDEX_NAME = process.env.NEXT_PUBLIC_ALGOLIA_INDEX_NAME || 'docs'; +const index = client.initIndex(INDEX_NAME);
44-49
: Debounce queries, avoid races, and type the Algolia responseCurrently every keystroke triggers a request with no debounce and no race cancellation, which can reorder results. Also, you can leverage generics to type hits instead of a custom cast.
Apply this diff to debounce and type the result:
useEffect(() => { - async function fetchData() { - const hits: AlgoliaHits = await index.search(search, { - clickAnalytics: true - }); - - setQueryID(hits.queryID); - - const mappedHits = hits?.hits.map((hit) => ({ - id: hit.objectID, - route: hit.slug, - children: ( - <> - <HighlightMatches value={hit.title} match={search} /> - <div className='excerpt nx-mt-1 nx-text-sm nx-leading-[1.35rem] nx-text-gray-600 dark:nx-text-gray-400 contrast-more:dark:nx-text-gray-50'> - <HighlightMatches value={hit.description} match={search} /> - </div> - </> - ) - })); - setResults(mappedHits); - } - - fetchData(); - }, [search, directories]); + if (!search) { + setResults([]); + setQueryID?.(undefined as unknown as string); // optional: clear queryID + return; + } + let cancelled = false; + const t = setTimeout(async () => { + const res = await index.search<AlgoliaHit>(search, { clickAnalytics: true }); + if (cancelled) return; + setQueryID?.(res.queryID); + const mappedHits = res.hits.map((hit) => ({ + id: hit.objectID, + route: hit.slug, + children: ( + <> + <HighlightMatches value={hit.title} match={search} /> + <div className='excerpt nx-mt-1 nx-text-sm nx-leading-[1.35rem] nx-text-gray-600 dark:nx-text-gray-400 contrast-more:dark:nx-text-gray-50'> + <HighlightMatches value={hit.description} match={search} /> + </div> + </> + ) + })); + setResults(mappedHits); + }, 150); + return () => { + cancelled = true; + clearTimeout(t); + }; + }, [search]);Also applies to: 50-67
70-77
: Avoid duplicating centering/width styles already applied in DocSearchDocSearch internally centers and sets width via inline styles now. The overlayClassName duplicates left/translate and width, which is harmless but redundant.
Apply this diff to keep concerns in DocSearch:
- overlayClassName='nx-min-h-[100px] nx-w-[600px] nx-max-w-[calc(100vw-2rem)] nx-left-1/2 -nx-translate-x-1/2' + overlayClassName='nx-min-h-[100px]'components/Search/docsearch.tsx (3)
94-101
: Use the configured index name and avoid O(n) position lookup; guard when queryID is missingHardcoding index: 'docs' can desync analytics if the index name is overridden via env. Also, computing positions with results.indexOf(result) is unnecessary.
Apply this diff:
- aa.default('clickedObjectIDsAfterSearch', { - index: 'docs', - eventName: 'Search Option Clicked', - queryID: queryID, - objectIDs: [result.id], - positions: [results.indexOf(result) + 1] - }); + if (queryID) { + aa.default('clickedObjectIDsAfterSearch', { + index: ALGOLIA_INDEX, + eventName: 'Search Option Clicked', + queryID, + objectIDs: [result.id], + positions: [i + 1] + }); + }Add this near the top of the file:
const ALGOLIA_INDEX = process.env.NEXT_PUBLIC_ALGOLIA_INDEX_NAME || 'docs';
89-106
: Stabilize callbacks to reduce re-renders and dependency churnfinishSearch is recreated on every render and then pulled into handleKeyDown’s dependency list, which recreates handleKeyDown too. Wrap finishSearch in useCallback and remove it from handleKeyDown deps.
Suggested changes:
- const finishSearch = (i: number) => { + const finishSearch = useCallback((i: number) => { input.current?.blur(); @@ - setShow(false); - }; + setShow(false); + }, [results, queryID, setObjectID, onChange]); const handleKeyDown = useCallback( function <T>(e: KeyboardEvent<T>) { @@ } }, - [active, results, router, finishSearch, handleActive, composition] + [active, results, router, handleActive, composition] );Also applies to: 112-157
321-321
: Optional: lazy-load the Algolia logo imageLow-priority improvement to avoid blocking the overlay paint.
Apply this diff:
- <img src='/logos/algolia-logo-blue.png' style={{ width: '60px' }} alt='Algolia logo' /> + <img src='/logos/algolia-logo-blue.png' loading='lazy' style={{ width: '60px' }} alt='Algolia logo' />components/CustomHeader/index.tsx (2)
160-183
: Add an aria-label to the nav for better accessibilityMinor improvement for screen readers.
Apply this diff:
- <nav className='nx-border-t nx-border-b nx-border-gray-200 dark:nx-border-neutral-800'> + <nav aria-label='Primary' className='nx-border-t nx-border-b nx-border-gray-200 dark:nx-border-neutral-800'>
90-109
: Deduplicate the inline SVG logoThe same large inline SVG appears twice (desktop and mobile). Extracting it into a shared Logo component will reduce duplication and keep branding changes in one place.
Example:
- Create components/Logo.tsx:
import React from 'react'; export function Logo({ className }: { className?: string }) { return ( <svg width='194' height='32' viewBox='0 0 194 32' fill='none' xmlns='http://www.w3.org/2000/svg' className={className}> <path d='M7.038 24.23C6.072 24.23 ...' fill='#FF0420' /> <path d='M134.081 24C133.928 24 ...' fill='currentColor' /> </svg> ); }
- Replace both usages:
- <svg ...>...</svg> + <Logo className='nx-h-6 sm:nx-h-8 nx-w-auto' />and
- <svg ...>...</svg> + <Logo className='nx-h-6 nx-w-auto' />Also applies to: 201-218
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
CLAUDE.md
(1 hunks)components/AskAIButton.tsx
(3 hunks)components/CustomHeader/index.tsx
(1 hunks)components/LoadingBar/index.tsx
(1 hunks)components/Search/docsearch.tsx
(11 hunks)components/Search/index.tsx
(4 hunks)package.json
(1 hunks)pages/_app.tsx
(2 hunks)theme.config.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/CustomHeader/index.tsx (1)
components/Search/index.tsx (1)
Search
(31-79)
🪛 LanguageTool
CLAUDE.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...this repository. ## Repository Overview This is the Optimism Documentation websi...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...nteroperability features. ## Tech Stack - Framework: Next.js 14.2.21 with React ...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ork**: Next.js 14.2.21 with React 18.2.0 - Documentation Engine: Nextra 2.13.2 (d...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ion Engine**: Nextra 2.13.2 (docs theme) - Language: TypeScript - **Package Manag...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ... (docs theme) - Language: TypeScript - Package Manager: pnpm (required - do n...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...pnpm (required - do not use npm or yarn) - Content Format: MDX (Markdown with Rea...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...**: MDX (Markdown with React components) - Deployment: Netlify ## Essential Comm...
(QB_NEW_EN)
[grammar] ~54-~54: There might be a mistake here.
Context: ...sable React components for documentation - public/
: Static assets, images, and tutorial fi...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...tatic assets, images, and tutorial files - utils/
: Utility scripts for linting, validatio...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...linting, validation, and build processes - providers/
: React context providers for global sta...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ...portant Patterns ### MDX Page Structure All documentation pages use MDX format w...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...ntent here... ``` ### Component Imports Use the configured path alias for compon...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...tName' ``` ### Adding New Documentation 1. Create MDX file in appropriate pages/
...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...ng New Documentation 1. Create MDX file in appropriate pages/
subdirectory 2. In...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...ectory when possible ### Spell Checking - Custom dictionary maintained in `words.t...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...cking - Custom dictionary maintained in words.txt
- Add technical terms using `pnpm spellche...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ...words.txt
- Add technical terms using pnpm spellcheck:fix
- Spell checking runs automatically in the...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...ly in the lint pipeline ## Git Workflow - Pre-push hook: Automatically runs `pnp...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ... validation on push ## Special Features - Kapa.ai Widget: AI assistant integrate...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ...ant integrated for documentation queries - Algolia Search: Full-text search acros...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ...*: Full-text search across documentation - Feelback: User feedback collection sys...
(QB_NEW_EN)
[grammar] ~100-~100: There might be a mistake here.
Context: ...lback**: User feedback collection system - Growth Book: A/B testing framework for...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...Adding a New Page 1. Create .mdx
file in appropriate pages/
directory 2. Add f...
(QB_NEW_EN)
[style] ~109-~109: The double modal “needed Run” is nonstandard (only accepted in certain dialects). Consider “to be Run”.
Context: ...nd import React components as needed 4. Run pnpm dev
to preview 5. Run `pnpm lint...
(NEEDS_FIXED)
[grammar] ~112-~112: There might be a mistake here.
Context: ...fore committing ### Updating Components - Components are in components/
director...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ...s that use them ### Working with Images - Place images in public/img/
directory ...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...X files - Optimize images before adding to repository ## Notes - The repository u...
(QB_NEW_EN)
[grammar] ~122-~122: There might be a mistake here.
Context: ...es before adding to repository ## Notes - The repository uses automated quality ch...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - docs-optimism
- GitHub Check: Header rules - docs-optimism
- GitHub Check: Pages changed - docs-optimism
🔇 Additional comments (8)
components/AskAIButton.tsx (2)
11-27
: SSR-guarded rendering looks goodDeferring render until mount and gating on the GrowthBook flag avoids hydration mismatch and keeps the widget behind the flag.
53-54
: All AskAIButton imports correctly use named exportsVerified across
.ts
and.tsx
files that every occurrence ofAskAIButton
is imported with the named syntax (import { AskAIButton } from ...
) and no default imports remain. No further changes needed.CLAUDE.md (1)
93-96
: Husky hook configuration confirmedThe
.husky/pre-push
script indeed runspnpm fix
and auto-commits any resulting changes, and there is no pre-commit hook defined. The documentation in CLAUDE.md (lines 93–96) accurately describes this behavior.pages/_app.tsx (1)
9-9
: Global LoadingBar import looks fineImport path and alias usage are consistent with the repo’s conventions.
theme.config.tsx (1)
16-18
: Navbar migration to a dedicated component looks goodSwitching to navbar: { component: CustomHeader } centralizes header concerns and removes scattered config. This aligns with the redesign goal.
components/Search/docsearch.tsx (2)
71-77
: Keyboard shortcuts handler is solidGlobal '/' and Ctrl/Cmd+K focus behavior with proper cleanup is implemented correctly.
Also applies to: 83-86
12-13
: Algolia Insights initialization confirmedInitialization is present in
pages/_app.tsx
(line 28:aa.default('init', { … })
), ensuring that subsequentaa
calls (e.g., incomponents/Search/docsearch.tsx
) will function correctly. No further changes required.components/CustomHeader/index.tsx (1)
11-18
: Overall header composition is clean and responsiveSolid separation of concerns (desktop vs mobile regions), route prefetching, and theme toggle wiring.
pnpm dev # Start development server at localhost:3001 | ||
pnpm build # Create production build | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect dev server port
The script "dev" in package.json is next dev
without a -p
flag, which defaults to port 3000, not 3001. Update the docs or the script for consistency.
Apply one of:
- pnpm dev # Start development server at localhost:3001
+ pnpm dev # Start development server at http://localhost:3000
or change package.json script:
- "dev": "next dev",
+ "dev": "next dev -p 3001",
📝 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.
pnpm dev # Start development server at localhost:3001 | |
pnpm build # Create production build | |
``` | |
pnpm dev # Start development server at http://localhost:3000 | |
pnpm build # Create production build |
🤖 Prompt for AI Agents
In CLAUDE.md around lines 20 to 22, the docs state "pnpm dev" starts the dev
server at localhost:3001 but the package.json script uses "next dev" which
defaults to port 3000; fix by making them consistent: either update CLAUDE.md to
say the dev server runs on localhost:3000, or modify the package.json "dev"
script to "next dev -p 3001" (or equivalent environment config) so the script
actually starts on 3001.
```bash | ||
pnpm spellcheck:lint # Check spelling | ||
pnpm spellcheck:fix # Add words to dictionary (words.txt) | ||
pnpm lint:eslint # Run ESLint | ||
pnpm lint:breadcrumbs # Validate breadcrumb structure | ||
pnpm lint:redirects # Check redirect configuration | ||
pnpm lint:metadata # Validate page metadata | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Listed per-tool lint scripts don’t exist
The docs advertise pnpm lint:eslint
, lint:breadcrumbs
, lint:redirects
, lint:metadata
, but package.json doesn’t define them. Either document the actual commands or add script aliases.
Option A (fix docs):
- pnpm lint:eslint # Run ESLint
- pnpm lint:breadcrumbs # Validate breadcrumb structure
- pnpm lint:redirects # Check redirect configuration
- pnpm lint:metadata # Validate page metadata
+ pnpm lint # ESLint + spellcheck + breadcrumbs + redirects + metadata
+ pnpm check-breadcrumbs # Validate breadcrumb structure
+ pnpm check-redirects # Check redirect configuration
+ pnpm validate-metadata # Validate page metadata
Option B (add scripts to package.json):
"scripts": {
+ "lint:eslint": "eslint . --ext mdx",
+ "lint:breadcrumbs": "NODE_NO_WARNINGS=1 node --loader ts-node/esm utils/breadcrumbs.ts",
+ "lint:redirects": "NODE_NO_WARNINGS=1 node --loader ts-node/esm utils/redirects.ts",
+ "lint:metadata": "NODE_NO_WARNINGS=1 node --loader ts-node/esm utils/metadata-validator.ts",
Check which path you prefer and keep docs aligned:
🏁 Script executed:
#!/bin/bash
# Show available scripts
cat package.json | jq '.scripts'
Length of output: 1376
Sync CLI docs with package.json scripts
The commands listed in CLAUDE.md (lines 31–38) reference scripts that don’t exist in package.json. Choose one of the following to restore alignment:
Option A – Update CLAUDE.md:
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -31,7 +31,11 @@
pnpm spellcheck:lint # Check spelling
pnpm spellcheck:fix # Add words to dictionary (words.txt)
- pnpm lint:eslint # Run ESLint
- pnpm lint:breadcrumbs # Validate breadcrumb structure
- pnpm lint:redirects # Check redirect configuration
- pnpm lint:metadata # Validate page metadata
+ pnpm lint # Run ESLint, spellcheck, breadcrumbs, redirects, metadata, link-checker
+ pnpm check-breadcrumbs # Validate breadcrumb structure
+ pnpm check-redirects # Check redirect configuration
+ pnpm validate-metadata # Validate page metadata
+ pnpm link-checker # Check for broken links
Option B – Add missing aliases to package.json scripts:
--- a/package.json
+++ b/package.json
@@ "scripts": {
+ "lint:eslint": "eslint . --ext mdx --max-warnings 0",
+ "lint:breadcrumbs": "NODE_NO_WARNINGS=1 node --loader ts-node/esm utils/breadcrumbs.ts",
+ "lint:redirects": "NODE_NO_WARNINGS=1 node --loader ts-node/esm utils/redirects.ts",
+ "lint:metadata": "CHANGED_FILES=$(git diff --name-only --cached) NODE_NO_WARNINGS=1 node --loader ts-node/esm utils/metadata-validator.ts",
Let me know which approach you prefer so the docs and scripts stay in sync.
📝 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.
```bash | |
pnpm spellcheck:lint # Check spelling | |
pnpm spellcheck:fix # Add words to dictionary (words.txt) | |
pnpm lint:eslint # Run ESLint | |
pnpm lint:breadcrumbs # Validate breadcrumb structure | |
pnpm lint:redirects # Check redirect configuration | |
pnpm lint:metadata # Validate page metadata | |
``` | |
pnpm spellcheck:lint # Check spelling | |
pnpm spellcheck:fix # Add words to dictionary (words.txt) | |
pnpm lint # Run ESLint, spellcheck, breadcrumbs, redirects, metadata, link-checker | |
pnpm check-breadcrumbs # Validate breadcrumb structure | |
pnpm check-redirects # Check redirect configuration | |
pnpm validate-metadata # Validate page metadata | |
pnpm link-checker # Check for broken links |
``` | ||
pages/ # All documentation content (MDX files) | ||
├── app-developers/ # Application developer guides | ||
├── operators/ # Node & chain operator documentation | ||
├── stack/ # OP Stack protocol documentation | ||
├── superchain/ # Superchain network documentation | ||
├── interop/ # Interoperability documentation | ||
└── connect/ # Contributing guides and resources | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a language to the fenced code block (markdownlint MD040)
Specify a language (e.g., text) for the directory tree block to satisfy linters and improve readability.
-```
+```text
pages/ # All documentation content (MDX files)
├── app-developers/ # Application developer guides
├── operators/ # Node & chain operator documentation
├── stack/ # OP Stack protocol documentation
├── superchain/ # Superchain network documentation
├── interop/ # Interoperability documentation
└── connect/ # Contributing guides and resources
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In CLAUDE.md around lines 43 to 52, the fenced code block showing the directory
tree is missing a language specifier which triggers markdownlint MD040; update
the opening fence to include a language (e.g., change to
text) so the
block becomes a labeled code block, ensuring the opening and closing backticks
remain matched and no other content is altered.
</details>
<!-- fingerprinting:phantom:poseidon:chinchilla -->
<!-- This is an auto-generated comment by CodeRabbit -->
useEffect(() => { | ||
const handleEscape = (e: KeyboardEvent) => { | ||
if (e.key === 'Escape') { | ||
setIsMobileMenuOpen(false); | ||
} | ||
}; | ||
|
||
if (isMobileMenuOpen) { | ||
document.addEventListener('keydown', handleEscape); | ||
document.body.style.overflow = 'hidden'; | ||
return () => { | ||
document.removeEventListener('keydown', handleEscape); | ||
document.body.style.overflow = ''; | ||
}; | ||
} else { | ||
document.body.style.overflow = ''; | ||
} | ||
}, [isMobileMenuOpen]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Body scroll lock can get stuck when resizing from mobile to desktop
If the mobile menu is open and the viewport crosses the desktop breakpoint, the overlay unmounts but isMobileMenuOpen remains true, leaving body.style.overflow = 'hidden'.
Add a resize-aware effect to auto-close the mobile menu and ensure overflow resets:
useEffect(() => {
const handleEscape = (e: KeyboardEvent) => {
if (e.key === 'Escape') {
setIsMobileMenuOpen(false);
}
};
@@
}, [isMobileMenuOpen]);
+
+ // Close mobile menu when switching to desktop to avoid sticky scroll lock
+ useEffect(() => {
+ if (isDesktop && isMobileMenuOpen) {
+ setIsMobileMenuOpen(false);
+ document.body.style.overflow = '';
+ }
+ }, [isDesktop, isMobileMenuOpen]);
📝 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.
useEffect(() => { | |
const handleEscape = (e: KeyboardEvent) => { | |
if (e.key === 'Escape') { | |
setIsMobileMenuOpen(false); | |
} | |
}; | |
if (isMobileMenuOpen) { | |
document.addEventListener('keydown', handleEscape); | |
document.body.style.overflow = 'hidden'; | |
return () => { | |
document.removeEventListener('keydown', handleEscape); | |
document.body.style.overflow = ''; | |
}; | |
} else { | |
document.body.style.overflow = ''; | |
} | |
}, [isMobileMenuOpen]); | |
useEffect(() => { | |
const handleEscape = (e: KeyboardEvent) => { | |
if (e.key === 'Escape') { | |
setIsMobileMenuOpen(false); | |
} | |
}; | |
if (isMobileMenuOpen) { | |
document.addEventListener('keydown', handleEscape); | |
document.body.style.overflow = 'hidden'; | |
return () => { | |
document.removeEventListener('keydown', handleEscape); | |
document.body.style.overflow = ''; | |
}; | |
} else { | |
document.body.style.overflow = ''; | |
} | |
}, [isMobileMenuOpen]); | |
// Close mobile menu when switching to desktop to avoid sticky scroll lock | |
useEffect(() => { | |
if (isDesktop && isMobileMenuOpen) { | |
setIsMobileMenuOpen(false); | |
document.body.style.overflow = ''; | |
} | |
}, [isDesktop, isMobileMenuOpen]); |
{/* Search Bar - hidden on mobile, shown on larger screens */} | ||
{isDesktop && ( | ||
<div className='nx-flex nx-mx-auto nx-relative' style={{ width: '400px' }}> | ||
<Search className='nx-w-full' directories={[]} /> | ||
</div> | ||
)} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid passing a new [] to Search on every render (causes redundant fetches)
You pass directories={[]} which is a new array per render. Combined with the effect dependency in Search, this can retrigger searches unnecessarily.
Short-term mitigation here:
-import { useTheme } from 'nextra-theme-docs';
+import { useTheme } from 'nextra-theme-docs';
+import { useMemo } from 'react';
@@
- {isDesktop && (
+ {isDesktop && (
<div className='nx-flex nx-mx-auto nx-relative' style={{ width: '400px' }}>
- <Search className='nx-w-full' directories={[]} />
+ {/* Stable empty directories to avoid effect churn */}
+ <Search className='nx-w-full' directories={useMemo(() => [], [])} />
</div>
)}
@@
- {!isDesktop && (
+ {!isDesktop && (
<div className='nx-px-4 nx-pb-4 nx-border-b nx-border-gray-200 dark:nx-border-neutral-800'>
- <Search className='nx-w-full' directories={[]} />
+ <Search className='nx-w-full' directories={useMemo(() => [], [])} />
</div>
)}
Longer-term: remove the directories prop from Search entirely, since the implementation now pulls exclusively from Algolia.
Also applies to: 153-158
🤖 Prompt for AI Agents
In components/CustomHeader/index.tsx around lines 110-116 (and the other
occurrence at 153-158), you're passing directories={[]} which creates a new
array on every render and causes the Search component's effect to retrigger;
either remove the directories prop entirely (preferred, since Search now reads
from Algolia) or replace directories={[]} with a stable reference (e.g., define
a top-level const EMPTY_DIRECTORIES: string[] = [] outside the component and
pass directories={EMPTY_DIRECTORIES}) so the prop identity does not change
between renders.
let progressTimer: NodeJS.Timeout; | ||
let finishTimer: NodeJS.Timeout; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timer types may be incompatible in the browser
Using NodeJS.Timeout can cause TS type errors in browser contexts. Prefer ReturnType for cross-env compatibility.
- let progressTimer: NodeJS.Timeout;
- let finishTimer: NodeJS.Timeout;
+ let progressTimer: ReturnType<typeof setTimeout> | null = null;
+ let finishTimer: ReturnType<typeof setTimeout> | null = null;
🤖 Prompt for AI Agents
In components/LoadingBar/index.tsx around lines 10 to 12, the timers are typed
as NodeJS.Timeout which can cause TypeScript incompatibilities in browser
builds; change the type to ReturnType<typeof setTimeout> (or use number for
setTimeout in DOM libs) for cross-environment compatibility, e.g., update both
progressTimer and finishTimer declarations to use ReturnType<typeof setTimeout>
and ensure any clearTimeout calls accept that type.
const handleComplete = () => { | ||
setProgress(100); | ||
finishTimer = setTimeout(() => { | ||
setLoading(false); | ||
setProgress(0); | ||
}, 300); | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition: clear the pending progress timer on completion
If a route completes before the 200ms timer fires, progress can jump back to 70% after completion. Clear the progress timer in handleComplete to avoid flicker.
- const handleComplete = () => {
- setProgress(100);
- finishTimer = setTimeout(() => {
- setLoading(false);
- setProgress(0);
- }, 300);
- };
+ const handleComplete = () => {
+ if (progressTimer) {
+ clearTimeout(progressTimer);
+ progressTimer = null;
+ }
+ setProgress(100);
+ finishTimer = setTimeout(() => {
+ setLoading(false);
+ setProgress(0);
+ finishTimer = null;
+ }, 300);
+ };
Also consider cancelling any pending finishTimer when a new navigation starts:
const handleStart = () => {
setLoading(true);
setProgress(0);
-
+ if (finishTimer) {
+ clearTimeout(finishTimer);
+ finishTimer = null;
+ }
📝 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 handleComplete = () => { | |
setProgress(100); | |
finishTimer = setTimeout(() => { | |
setLoading(false); | |
setProgress(0); | |
}, 300); | |
}; | |
const handleComplete = () => { | |
if (progressTimer) { | |
clearTimeout(progressTimer); | |
progressTimer = null; | |
} | |
setProgress(100); | |
finishTimer = setTimeout(() => { | |
setLoading(false); | |
setProgress(0); | |
finishTimer = null; | |
}, 300); | |
}; | |
const handleStart = () => { | |
setLoading(true); | |
setProgress(0); | |
if (finishTimer) { | |
clearTimeout(finishTimer); | |
finishTimer = null; | |
} | |
// …rest of your start logic (e.g. setting progressTimer) | |
}; |
🤖 Prompt for AI Agents
In components/LoadingBar/index.tsx around lines 22-29, handleComplete sets a
finishTimer but does not clear any existing timers, which can let a previously
scheduled timeout fire after completion and reset progress (causing flicker);
modify handleComplete to clear the pending finishTimer (if set) before setting a
new one, and also clear/cancel any existing finishTimer when a new navigation
starts so no stale timeout can run later. Ensure finishTimer is stored in a
scope that both the navigation-start handler and handleComplete can access and
that you check for a non-null timer before calling clearTimeout, then null it
after clearing.
export function Search({ | ||
className, | ||
directories, | ||
directories | ||
}: Readonly<{ | ||
className?: string; | ||
directories: NormalItem[]; | ||
}>): ReactElement { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
directories prop is unused but drives effect re-renders; remove it or memoize at call sites
The effect depends on directories, but the prop is not used in this implementation. In CustomHeader it’s always passed as a new [] literal, which will retrigger the effect on every render, causing redundant network requests.
Two options:
- Preferred: remove directories from props and dependencies.
-export function Search({
- className,
- directories
-}: Readonly<{
- className?: string;
- directories: NormalItem[];
-}>): ReactElement {
+export function Search({
+ className
+}: Readonly<{
+ className?: string;
+}>): ReactElement {
@@
- }, [search, directories]);
+ }, [search]);
- Minimal mitigation at call sites: pass a memoized empty array to avoid changing references every render (see comment on CustomHeader).
Also applies to: 68-69
🤖 Prompt for AI Agents
In components/Search/index.tsx around lines 31 to 37 (and note related lines
68-69), the directories prop is declared and included in the effect dependencies
but never used, causing unnecessary effect re-renders when callers pass a new
array literal; remove directories from the component props and from the effect
dependency list, update the component's type signature to no longer require
directories, and update all call sites (e.g., CustomHeader) to stop passing
directories or, if you cannot change callers immediately, memoize the array at
the call site (useMemo or a shared constant) so its reference is stable.
@jvmi7 I can't see the redesign, the homepage is still the same. |
https://www.figma.com/design/KI7tPCi9211oDU4KgtYCFX/Docs?node-id=16342-4890&m=dev