Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| function stripHtml(str: string): string { | ||
| return str.replace(/<[^>]*>/g, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughLarge refactor of the Eclipse app: removed many fumadocs/shadcn config and legacy notebook/layout modules, replaced sidebar/TOC/search/layout primitives with new implementations, rewrote global styles and MDX content, and pruned several dependencies and scripts. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/eclipse/src/components/toc.tsx (1)
83-103:⚠️ Potential issue | 🟡 MinorGuard the no-match path in
calc().If
activecontains ids that are not rendered in this TOC,upperstays atNumber.MAX_VALUEand the thumb gets garbage CSS values. Returning[0, 0]when nothing matches avoids the jump.Proposed fix
function calc(container: HTMLElement, active: string[]): TocThumbType { if (active.length === 0 || container.clientHeight === 0) { return [0, 0]; } let upper = Number.MAX_VALUE, lower = 0; + let matched = false; for (const item of active) { const element = container.querySelector<HTMLElement>(`a[href="#${item}"]`); if (!element) continue; + matched = true; const styles = getComputedStyle(element); upper = Math.min(upper, element.offsetTop + parseFloat(styles.paddingTop)); lower = Math.max( lower, element.offsetTop + element.clientHeight - parseFloat(styles.paddingBottom), ); } + + if (!matched) { + return [0, 0]; + } return [upper, lower - upper]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/src/components/toc.tsx` around lines 83 - 103, In calc(container: HTMLElement, active: string[]): TocThumbType ensure the case where none of the active ids match rendered TOC anchors is handled: after iterating active and computing upper/lower (the variables inside calc), check if upper is still Number.MAX_VALUE (meaning no element matched) and return [0, 0] instead of continuing to compute and return garbage values; update the function calc to return early in that no-match path so the thumb CSS values remain valid.
♻️ Duplicate comments (1)
apps/eclipse/src/components/search.tsx (1)
107-109:⚠️ Potential issue | 🟡 MinorIncomplete HTML sanitization — XSS vector remains.
The regex
/<[^>]*>/gdoesn't handle malformed tags like<scr<script>ipt>. After one pass, this becomes<script>. CodeQL correctly flagged this. Sinceresult.contentcomes from the search API which indexes your own content, the risk is lower, but defense-in-depth matters.🔒 Iterative sanitization or DOM-based approach
function stripHtml(str: string): string { - return str.replace(/<[^>]*>/g, ''); + // Iteratively strip until no more tags remain + let result = str; + let previous: string; + do { + previous = result; + result = result.replace(/<[^>]*>/g, ''); + } while (result !== previous); + return result; }Alternatively, use a DOM-based approach:
function stripHtml(str: string): string { if (typeof document === 'undefined') return str.replace(/<[^>]*>/g, ''); const doc = new DOMParser().parseFromString(str, 'text/html'); return doc.body.textContent ?? ''; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/src/components/search.tsx` around lines 107 - 109, The stripHtml function uses a single-pass regex which allows crafted input like "<scr<script>ipt>" to bypass filtering; update stripHtml to robustly remove HTML by using a DOM-based sanitizer in the browser (use DOMParser to parse to 'text/html' and return body.textContent) and provide a safe server-side fallback (either run iterative stripping until the string no longer changes or use a proper HTML parser/sanitizer library such as htmlparser2 or sanitize-html) so that malformed nested tags cannot reconstitute dangerous markup; ensure the function (stripHtml) returns the sanitized plain text in both environments and handles the typeof document === 'undefined' case by using the fallback.
🧹 Nitpick comments (7)
apps/eclipse/content/design-system/index.mdx (2)
54-57: CSS import order matters—consider adding a note.The
@import "tailwindcss"must come before Eclipse styles for proper cascade ordering. This is implied by the code order but could trip up users who rearrange imports.A brief inline comment in the code block could help:
/* Import Tailwind first, then Eclipse styles */ `@import` "tailwindcss"; `@import` "@prisma/eclipse/styles/globals.css";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/content/design-system/index.mdx` around lines 54 - 57, Add a short inline comment above the CSS imports in the code block to explicitly state that "@import 'tailwindcss';" must come before the Eclipse styles import ("@import '@prisma/eclipse/styles/globals.css';") to preserve cascade ordering; update the MDX snippet near the existing import lines to include that single-line explanatory comment so future editors don't accidentally reorder them.
20-22: Consider specifying minimum versions more precisely.The prerequisites list
tailwindcssv4 and@tailwindcss/postcssv4, but Tailwind v4 is relatively new. It might help users to know the exact minimum version tested (e.g.,4.0.0or4.1.0) to avoid compatibility surprises with early v4 releases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/content/design-system/index.mdx` around lines 20 - 22, Update the prerequisites to specify explicit minimum tested versions instead of loose "v4" for clarity: replace the entries for `tailwindcss` and `@tailwindcss/postcss` (currently listed as "v4") with a precise minimum like "tailwindcss >= 4.0.0" and "@tailwindcss/postcss >= 4.0.0" (or the exact patch you tested, e.g., "4.1.0") and adjust the surrounding phrasing to indicate these are the minimum tested versions to avoid ambiguity.apps/eclipse/src/components/sidebar.tsx (2)
217-219:ReactDOM.flushSyncin animation handlers can cause performance issues.
flushSyncforces synchronous re-renders, bypassing React's batching. While it ensures the hidden state is set immediately after animation ends, it can cause jank. The animation end is already async, so the sync flush may not be necessary.Consider testing without
flushSync—React 18+ batches state updates by default, and the visual result should be the same:-onAnimationEnd={() => { - if (!open) ReactDOM.flushSync(() => setOverlayHidden(true)); -}} +onAnimationEnd={() => { + if (!open) setOverlayHidden(true); +}}Also applies to: 226-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/src/components/sidebar.tsx` around lines 217 - 219, The animation end handlers use ReactDOM.flushSync which forces synchronous re-renders and can cause jank; replace the flushSync call in the onAnimationEnd handlers with a normal state update (call setOverlayHidden(true) directly) and verify visuals—if you need to prioritize responsiveness consider using startTransition around less critical updates; update both occurrences that call ReactDOM.flushSync (the onAnimationEnd handlers that reference setOverlayHidden) to avoid flushSync and rely on React 18+ batching instead.
343-353: Complex click handling on folder links could confuse users.Clicking the icon toggles the folder, clicking the text navigates and conditionally toggles. This dual behavior on a single link element can be unexpected. Users might accidentally toggle when they meant to navigate.
The
[data-icon]selector approach is clever but somewhat fragile—it depends on the SVG havingdata-iconand not being wrapped differently.Consider separating the toggle button from the link more explicitly, similar to
SidebarFolderTrigger, to make the interaction clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/src/components/sidebar.tsx` around lines 343 - 353, The current onClick handler on the folder link (using e.target.matches('[data-icon], [data-icon] *') and toggling via setOpen) mixes navigation and toggle behavior and relies on the fragile data-icon selector; change this to separate concerns by extracting the icon into an explicit toggle control (like SidebarFolderTrigger) with its own onClick that stops propagation and calls setOpen(!open), and leave the link's onClick to only handle navigation (or to setOpen(active ? !open : true) without icon-based detection); update references in this component to remove the data-icon selector and wire the new toggle button handler so clicking the icon only toggles and clicking the link only navigates.apps/eclipse/src/components/page.tsx (2)
131-138: Footer navigation lookup uses exact pathname match, which may miss trailing slashes or query params.The
findIndexcomparisonitem.url === pathnameis strict. Ifpathnamehas a trailing slash anditem.urldoesn't (or vice versa), navigation won't be found.🔗 Normalize paths before comparison
const { previous, next } = useMemo(() => { - const idx = footerList.findIndex((item) => item.url === pathname); + const normalizedPathname = pathname.replace(/\/$/, ''); + const idx = footerList.findIndex((item) => item.url.replace(/\/$/, '') === normalizedPathname); if (idx === -1) return {}; return { previous: footerList[idx - 1], next: footerList[idx + 1], }; }, [footerList, pathname]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/src/components/page.tsx` around lines 131 - 138, The footer lookup fails on mismatched path formats; inside the useMemo that computes {previous, next} (the findIndex using footerList and pathname), normalize both item.url and pathname before comparing (e.g., strip query/hash and trailing slashes, treat root "/" consistently) and use the normalized values in findIndex; implement a small helper (normalizePath) and replace the strict equality check item.url === pathname with normalizePath(item.url) === normalizePath(pathname) in the useMemo that computes previous and next.
143-143: Footer has inconsistent padding class compared to other page sections.
DocsFooterusesp-6(padding all sides) whileDocsHeaderandDocsBodyusepx-6 ... py-6. This creates visual inconsistency on mobile whereDocsFootergets top padding fromp-6but also hasmd:py-12.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/src/components/page.tsx` at line 143, DocsFooter's footer element uses p-6 which applies uniform padding and causes extra top padding on mobile; change the class on the footer element (the <footer className="p-6 md:pl-[18.5rem] md:pr-12 md:py-12 ..."> in the DocsFooter/Docs page component) from p-6 to px-6 py-6 so it matches DocsHeader and DocsBody and keeps consistent horizontal and vertical padding across breakpoints.apps/eclipse/src/components/search.tsx (1)
28-31: Body scroll lock cleanup may leave stale state if component unmounts unexpectedly.The cleanup function restores
overflowto an empty string, which works if no other code modifiesdocument.body.style.overflow. However, if another modal or overlay sets overflow while this dialog is open, the cleanup will clobber that value.🛡️ Safer scroll lock that preserves previous value
useEffect(() => { + const previousOverflow = document.body.style.overflow; document.body.style.overflow = 'hidden'; - return () => { document.body.style.overflow = ''; }; + return () => { document.body.style.overflow = previousOverflow; }; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/src/components/search.tsx` around lines 28 - 31, Store the previous body overflow when the effect runs and restore it only if we still own the lock: in the useEffect that currently sets document.body.style.overflow = 'hidden' (in apps/eclipse/src/components/search.tsx), capture const prevOverflow = document.body.style.overflow before setting it to 'hidden', and in the cleanup check if document.body.style.overflow === 'hidden' (i.e., still our lock) then restore document.body.style.overflow = prevOverflow; this preserves other modals' changes and avoids clobbering their value.
🤖 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/eclipse/src/app/global.css`:
- Around line 52-54: The .font-mono-settings rule uses the wrong CSS variable
name (--font-mono--settings) so the font-feature settings won't be applied;
change the variable reference inside the .font-mono-settings declaration to the
correct token (--font-mono-settings) so font-feature-settings:
var(--font-mono-settings) uses the intended value; update any other occurrences
of --font-mono--settings to --font-mono-settings to keep names consistent.
- Around line 132-134: The global rule targets every li and forces list-disc,
which removes numbering from ordered lists; change the selector so only
unordered lists get the disc style (e.g., replace the global li selector with a
selector targeting ul li or add a utility-class for unordered lists) and keep
the `@apply` mx-6 list-outside on li elements that need the spacing; update the
selector that contains "@apply mx-6 list-disc list-outside;" (the li rule) to
restrict list-disc to unordered lists only so ol items keep their numbers.
- Line 4: The Stylelint failure is caused by the standard SCSS rule
at-rule-no-unknown flagging Tailwind v4 at-rules like `@theme` (and Tailwind uses
`@import/`@keyframes in its scope); update .stylelintrc.json to configure the
at-rule-no-unknown rule to ignore Tailwind-specific at-rules (add the Tailwind
at-rules array or disable at-rule-no-unknown and enable
stylelint-config-tailwindcss/at-rule exceptions) so `@theme`, `@import`, `@keyframes`
used by Tailwind v4 no longer trigger errors.
In `@apps/eclipse/src/components/layout.tsx`:
- Around line 12-18: DocsLayoutProps declares sidebar.banner but the
implementation builds its own searchBanner and ignores that prop; fix by wiring
the prop through: in the component that creates searchBanner (reference
searchBanner) read props.sidebar?.banner (from DocsLayoutProps) and either
replace searchBanner with that node when provided or merge them (e.g., render
both nodes or wrap them in a fragment) so the passed banner is used;
alternatively, if you prefer to remove the unused API, delete sidebar.banner
from DocsLayoutProps to keep the interface in sync with the implementation.
- Around line 43-54: layout.tsx currently only checks the layout prop
sidebarEnabled before rendering SidebarContent and SidebarDrawer, so per-page
disables via SidebarEnabledFromPageProvider/SidebarEnabledSync are ignored;
update the rendering gate to consume the SidebarEnabledContext (or use the
existing SidebarEnabledGate) and use the context's effective value instead of
the raw sidebarEnabled prop when deciding to render SidebarContent and
SidebarDrawer (keep SidebarEnabledFromPageProvider in place to provide the
context to children and ensure SidebarEnabledSync can update pageEnabled).
In `@apps/eclipse/src/components/page.tsx`:
- Around line 39-43: SidebarEnabledSync currently calls setEnabled during render
which violates React rules; change it to call setEnabled inside a useEffect
hook: import useEffect from React, then inside SidebarEnabledSync use
useEffect(() => { if (!enabled) setEnabled(false); }, [enabled, setEnabled]);
keep the component returning null as before and mention
SidebarEnabledSetterContext and the parent SidebarEnabledFromPageProvider so
reviewers can locate the related state provider.
- Around line 81-92: DocsHeader currently destructures className and ...props
but never uses them, so custom classes and attributes passed to DocsHeader are
ignored; update the function to merge the incoming className with the existing
header class string (using your project's utility like clsx or a template
literal) and spread ...props onto the <header> element so attributes like id,
aria-*, or event handlers are preserved; locate the DocsHeader component and
modify the <header> element to include the merged className and {...props}.
In `@apps/eclipse/src/components/search.tsx`:
- Around line 144-146: The KBD label is hardcoded to "CMD+K"; change it to
detect platform (mac vs others) and render the appropriate modifier string
(e.g., "⌘K" or "Ctrl+K") where the <kbd> element is rendered in the Search
component (the kbd element shown in search.tsx); add a small helper (e.g., isMac
using navigator.platform or userAgent guard for SSR) and use that boolean to
choose the displayed text so Windows/Linux users see "Ctrl+K" while mac users
see the Command symbol.
- Around line 47-53: Wrap the dialog container div (the element with
onClick={(e) => e.stopPropagation()} and onKeyDown handling Escape/onClose) with
proper ARIA attributes and a focus trap: add role="dialog", aria-modal="true"
and aria-labelledby pointing to the dialog title element (create an id on the
title) so screen readers recognize it, ensure the container has tabIndex={-1}
and programmatically focus it (or the first interactive element) when mounted,
and implement keyboard focus trapping (handle Tab and Shift+Tab to keep focus
inside the dialog) and restore focus to the previously focused element when
onClose is called.
In `@apps/eclipse/src/components/sidebar.tsx`:
- Around line 205-206: The current render directly calls setOverlayHidden(false)
and setDrawerHidden(false) when open is true, which is an anti-pattern; move
this logic into a useEffect that runs when open changes (e.g., useEffect(() => {
if (open) { if (overlayHidden) setOverlayHidden(false); if (drawerHidden)
setDrawerHidden(false); } }, [open, overlayHidden, drawerHidden])), referencing
the state setters setOverlayHidden and setDrawerHidden and the state values
overlayHidden and drawerHidden so updates only occur inside the effect and only
when necessary.
- Line 375: The SidebarItem component declares an external prop but never uses
it; update the SidebarItem function signature and implementation to either
remove the external prop or apply appropriate external-link behavior: when
external is true, render the anchor with target="_blank" and rel="noopener
noreferrer" and optionally show an external-link icon next to children; ensure
any callers passing external remain compatible or remove the prop type from
callers if you choose to delete it. Reference: SidebarItem (function) and the
external prop in its props type.
- Around line 167-181: The hover timeout referenced by timerRef (used in the
onPointerLeave handler) can fire after unmount; add a cleanup to clear the
timeout on component unmount: implement a useEffect cleanup that calls
window.clearTimeout(timerRef.current) (and optionally sets timerRef.current = 0)
so any pending timer from onPointerLeave is cancelled; ensure this ties to the
same timerRef used alongside onPointerEnter/onPointerLeave and does not rely on
event handlers to clear it.
---
Outside diff comments:
In `@apps/eclipse/src/components/toc.tsx`:
- Around line 83-103: In calc(container: HTMLElement, active: string[]):
TocThumbType ensure the case where none of the active ids match rendered TOC
anchors is handled: after iterating active and computing upper/lower (the
variables inside calc), check if upper is still Number.MAX_VALUE (meaning no
element matched) and return [0, 0] instead of continuing to compute and return
garbage values; update the function calc to return early in that no-match path
so the thumb CSS values remain valid.
---
Duplicate comments:
In `@apps/eclipse/src/components/search.tsx`:
- Around line 107-109: The stripHtml function uses a single-pass regex which
allows crafted input like "<scr<script>ipt>" to bypass filtering; update
stripHtml to robustly remove HTML by using a DOM-based sanitizer in the browser
(use DOMParser to parse to 'text/html' and return body.textContent) and provide
a safe server-side fallback (either run iterative stripping until the string no
longer changes or use a proper HTML parser/sanitizer library such as htmlparser2
or sanitize-html) so that malformed nested tags cannot reconstitute dangerous
markup; ensure the function (stripHtml) returns the sanitized plain text in both
environments and handles the typeof document === 'undefined' case by using the
fallback.
---
Nitpick comments:
In `@apps/eclipse/content/design-system/index.mdx`:
- Around line 54-57: Add a short inline comment above the CSS imports in the
code block to explicitly state that "@import 'tailwindcss';" must come before
the Eclipse styles import ("@import '@prisma/eclipse/styles/globals.css';") to
preserve cascade ordering; update the MDX snippet near the existing import lines
to include that single-line explanatory comment so future editors don't
accidentally reorder them.
- Around line 20-22: Update the prerequisites to specify explicit minimum tested
versions instead of loose "v4" for clarity: replace the entries for
`tailwindcss` and `@tailwindcss/postcss` (currently listed as "v4") with a
precise minimum like "tailwindcss >= 4.0.0" and "@tailwindcss/postcss >= 4.0.0"
(or the exact patch you tested, e.g., "4.1.0") and adjust the surrounding
phrasing to indicate these are the minimum tested versions to avoid ambiguity.
In `@apps/eclipse/src/components/page.tsx`:
- Around line 131-138: The footer lookup fails on mismatched path formats;
inside the useMemo that computes {previous, next} (the findIndex using
footerList and pathname), normalize both item.url and pathname before comparing
(e.g., strip query/hash and trailing slashes, treat root "/" consistently) and
use the normalized values in findIndex; implement a small helper (normalizePath)
and replace the strict equality check item.url === pathname with
normalizePath(item.url) === normalizePath(pathname) in the useMemo that computes
previous and next.
- Line 143: DocsFooter's footer element uses p-6 which applies uniform padding
and causes extra top padding on mobile; change the class on the footer element
(the <footer className="p-6 md:pl-[18.5rem] md:pr-12 md:py-12 ..."> in the
DocsFooter/Docs page component) from p-6 to px-6 py-6 so it matches DocsHeader
and DocsBody and keeps consistent horizontal and vertical padding across
breakpoints.
In `@apps/eclipse/src/components/search.tsx`:
- Around line 28-31: Store the previous body overflow when the effect runs and
restore it only if we still own the lock: in the useEffect that currently sets
document.body.style.overflow = 'hidden' (in
apps/eclipse/src/components/search.tsx), capture const prevOverflow =
document.body.style.overflow before setting it to 'hidden', and in the cleanup
check if document.body.style.overflow === 'hidden' (i.e., still our lock) then
restore document.body.style.overflow = prevOverflow; this preserves other
modals' changes and avoids clobbering their value.
In `@apps/eclipse/src/components/sidebar.tsx`:
- Around line 217-219: The animation end handlers use ReactDOM.flushSync which
forces synchronous re-renders and can cause jank; replace the flushSync call in
the onAnimationEnd handlers with a normal state update (call
setOverlayHidden(true) directly) and verify visuals—if you need to prioritize
responsiveness consider using startTransition around less critical updates;
update both occurrences that call ReactDOM.flushSync (the onAnimationEnd
handlers that reference setOverlayHidden) to avoid flushSync and rely on React
18+ batching instead.
- Around line 343-353: The current onClick handler on the folder link (using
e.target.matches('[data-icon], [data-icon] *') and toggling via setOpen) mixes
navigation and toggle behavior and relies on the fragile data-icon selector;
change this to separate concerns by extracting the icon into an explicit toggle
control (like SidebarFolderTrigger) with its own onClick that stops propagation
and calls setOpen(!open), and leave the link's onClick to only handle navigation
(or to setOpen(active ? !open : true) without icon-based detection); update
references in this component to remove the data-icon selector and wire the new
toggle button handler so clicking the icon only toggles and clicking the link
only navigates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 40dddb08-795d-4e6b-bcdb-b0ada8420f72
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
apps/eclipse/cli.jsonapps/eclipse/components.jsonapps/eclipse/content/design-system/index.mdxapps/eclipse/next.config.mjsapps/eclipse/package.jsonapps/eclipse/src/app/(design-system)/[[...slug]]/page.tsxapps/eclipse/src/app/(design-system)/layout.tsxapps/eclipse/src/app/api/search/route.tsapps/eclipse/src/app/global.cssapps/eclipse/src/app/layout.tsxapps/eclipse/src/components/layout.tsxapps/eclipse/src/components/layout/language-toggle.tsxapps/eclipse/src/components/layout/link-item.tsxapps/eclipse/src/components/layout/notebook/client.tsxapps/eclipse/src/components/layout/notebook/index.tsxapps/eclipse/src/components/layout/notebook/page/client.tsxapps/eclipse/src/components/layout/notebook/page/index.tsxapps/eclipse/src/components/layout/notebook/sidebar.tsxapps/eclipse/src/components/layout/search-toggle.tsxapps/eclipse/src/components/layout/shared.tsxapps/eclipse/src/components/layout/sidebar/base.tsxapps/eclipse/src/components/layout/sidebar/link-item.tsxapps/eclipse/src/components/layout/sidebar/page-tree.tsxapps/eclipse/src/components/layout/sidebar/tabs/dropdown.tsxapps/eclipse/src/components/layout/sidebar/tabs/index.tsxapps/eclipse/src/components/layout/theme-toggle.tsxapps/eclipse/src/components/nav.tsxapps/eclipse/src/components/page-actions.tsxapps/eclipse/src/components/page.tsxapps/eclipse/src/components/provider.tsxapps/eclipse/src/components/search.tsxapps/eclipse/src/components/sidebar.tsxapps/eclipse/src/components/toc.tsxapps/eclipse/src/components/toc/clerk.tsxapps/eclipse/src/components/toc/default.tsxapps/eclipse/src/components/ui/button.tsxapps/eclipse/src/components/ui/collapsible.tsxapps/eclipse/src/components/ui/popover.tsxapps/eclipse/src/components/ui/scroll-area.tsxapps/eclipse/src/lib/layout.shared.tsxapps/eclipse/src/lib/urls.ts
💤 Files with no reviewable changes (27)
- apps/eclipse/next.config.mjs
- apps/eclipse/cli.json
- apps/eclipse/components.json
- apps/eclipse/src/lib/urls.ts
- apps/eclipse/src/components/ui/button.tsx
- apps/eclipse/src/components/layout/sidebar/tabs/index.tsx
- apps/eclipse/src/components/layout/notebook/sidebar.tsx
- apps/eclipse/src/components/toc/default.tsx
- apps/eclipse/src/components/layout/language-toggle.tsx
- apps/eclipse/src/components/ui/scroll-area.tsx
- apps/eclipse/src/lib/layout.shared.tsx
- apps/eclipse/src/components/layout/theme-toggle.tsx
- apps/eclipse/src/components/layout/link-item.tsx
- apps/eclipse/src/components/layout/shared.tsx
- apps/eclipse/src/components/layout/sidebar/tabs/dropdown.tsx
- apps/eclipse/src/components/layout/sidebar/link-item.tsx
- apps/eclipse/src/components/layout/notebook/index.tsx
- apps/eclipse/src/components/ui/collapsible.tsx
- apps/eclipse/src/components/layout/notebook/page/client.tsx
- apps/eclipse/src/components/layout/sidebar/page-tree.tsx
- apps/eclipse/src/components/layout/sidebar/base.tsx
- apps/eclipse/src/components/ui/popover.tsx
- apps/eclipse/src/components/layout/search-toggle.tsx
- apps/eclipse/src/components/layout/notebook/page/index.tsx
- apps/eclipse/src/components/layout/notebook/client.tsx
- apps/eclipse/src/components/page-actions.tsx
- apps/eclipse/src/components/toc/clerk.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/eclipse/src/components/search.tsx (1)
50-59:⚠️ Potential issue | 🟠 MajorPlease finish the modal focus management.
Most of the dialog semantics are in place, but Tab/Shift+Tab can still escape into the page behind the modal, and the clickable
ESCchip is not a keyboard-accessible control. Please trap focus while the dialog is open and make the close affordance a real button.Also applies to: 69-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/src/components/search.tsx` around lines 50 - 59, The modal currently uses a plain div and inline onKeyDown but lacks focus trapping and a semantic close control; update the Search component to make the "ESC" affordance a real <button> that calls onClose and is keyboard-focusable, add a ref (e.g., modalRef) to the dialog root and on mount save document.activeElement and move focus into the modal (preferably to the search input), and implement a focus trap in a useEffect that listens for Tab/Shift+Tab to loop focus among focusable elements inside modalRef (querySelectorAll for anchors, inputs, buttons, [tabindex]:not([tabindex="-1"])), restoring the previously focused element on unmount; keep the existing Escape handling but ensure it also triggers onClose from the same effect.
🤖 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/eclipse/src/app/global.css`:
- Around line 101-102: The `@apply` directive in apps/eclipse/src/app/global.css
is missing a trailing semicolon (currently `@apply mt-6` before the closing
brace); update that CSS rule by adding the semicolon so it reads `@apply mt-6;`
to ensure PostCSS/Tailwind parsing is valid and avoids errors in stricter
environments.
---
Duplicate comments:
In `@apps/eclipse/src/components/search.tsx`:
- Around line 50-59: The modal currently uses a plain div and inline onKeyDown
but lacks focus trapping and a semantic close control; update the Search
component to make the "ESC" affordance a real <button> that calls onClose and is
keyboard-focusable, add a ref (e.g., modalRef) to the dialog root and on mount
save document.activeElement and move focus into the modal (preferably to the
search input), and implement a focus trap in a useEffect that listens for
Tab/Shift+Tab to loop focus among focusable elements inside modalRef
(querySelectorAll for anchors, inputs, buttons,
[tabindex]:not([tabindex="-1"])), restoring the previously focused element on
unmount; keep the existing Escape handling but ensure it also triggers onClose
from the same effect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3d73381a-cdb1-4c94-a261-cc080f9eacbe
📒 Files selected for processing (6)
apps/eclipse/src/app/global.cssapps/eclipse/src/components/layout.tsxapps/eclipse/src/components/page.tsxapps/eclipse/src/components/search.tsxapps/eclipse/src/components/sidebar.tsxpackages/eclipse/src/styles/globals.css
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/eclipse/src/components/sidebar.tsx
- apps/eclipse/src/components/page.tsx
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.
Redesigns the Eclipse design system app to use its own simplified component architecture while maintaining fumadocs integration for content, search, navigation, and heading
tracking.
What changed
Architecture — Replaced the previous fumadocs-ui layout system (~3,800 lines across layout/notebook/, layout/sidebar/, toc/, ui/) with a flat component structure (~950 lines
across 6 files):
Fumadocs integration preserved:
Removed:
Other changes:
Known issue
@prisma/eclipse CodeBlock component uses fd-* CSS tokens from fumadocs-ui. These need to be replaced with Eclipse design tokens in a follow-up PR to the eclipse package.
Summary by CodeRabbit