refactor(docs): Replace Sphinx based home page with redesigned typescript home page.#56
Open
AVMatthews wants to merge 8 commits intoy-scope:mainfrom
Open
refactor(docs): Replace Sphinx based home page with redesigned typescript home page.#56AVMatthews wants to merge 8 commits intoy-scope:mainfrom
AVMatthews wants to merge 8 commits intoy-scope:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded a Next.js-based documentation homepage (layout, pages, components, styles, tooling), expanded the docs build pipeline to build dev docs and the homepage separately and merge outputs, and documented a git submodule clone step in the README. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 27
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/app/content.ts`:
- Line 144: Define a concrete item contract and replace the loose Array<any> to
restore type-safety: create an Item type/interface (e.g., Item with required
fields like title:string and common optional fields such as href?:string,
description?:string, external?:boolean, img?:string, imgStyle?:string), then
change the Category type to use items: Item[] instead of Array<any>; also update
any nearby usages to accept Category[] where the list of categories is used.
Ensure the unique symbols Category, items, Item, external, and imgStyle are used
so callers and imports can be updated accordingly.
In `@docs/app/layout.tsx`:
- Line 40: The ThemeProvider import is located after the exported constants
(metadata, viewport); move the line importing ThemeProvider so all imports are
grouped at the top of the file in layout.tsx—place the import alongside the
other imports above the exports (e.g., above the metadata and viewport
declarations) to follow conventional ordering and improve readability; ensure
you only relocate the import statement for ThemeProvider and do not change its
identifier.
In `@docs/app/page.tsx`:
- Around line 134-137: The current items.map callback uses the array index (i)
as the React key when rendering <IconLink> which can break state when items
reorder; update the map to use a stable unique identifier from the item (e.g.,
use items.map((it) => <IconLink key={it.href} ... />) or fallback to it.id or
another unique property on the item) so replace the key={i} with key={it.href}
(or key={it.id} / other stable unique field) inside the items.map that renders
IconLink.
- Line 125: The Category component currently types its items prop as Array<any>;
define a concrete interface (e.g., interface CategoryItem { id: string; title:
string; href?: string; icon?: React.ReactNode; /* add fields used in rendering
*/ }) and replace the prop type in the Category declaration (Category({title,
items}: {title: string; items: Array<any>}) => ...) with items: CategoryItem[]
(or readonly CategoryItem[]), updating any usages and imports to match the new
structure for full type safety.
- Line 78: The prop declaration imgStyle?: any should be changed to a specific
style type to preserve TypeScript checks; replace the any with an appropriate
type such as React.CSSProperties (or a framework-specific type like ImageStyle
if applicable) and import the type if necessary, updating the props/interface
that declares imgStyle to use that type so consumers get proper autocomplete and
type safety for CSS properties.
- Around line 22-32: Extract the duplicated GITHUB_ICON_PATH constant into a new
shared module (e.g., export const GITHUB_ICON_PATH from a file in
docs/app/shared or docs/lib) and replace the inline string in both page.tsx and
Navbar.tsx with an import of that constant; update references to the symbol
GITHUB_ICON_PATH in page.tsx and in the Navbar component (where it's currently
duplicated around the 202 and 326 usages) to use the imported value, and ensure
the new module exports the exact same string so SVGs render identically.
In `@docs/app/shared/ThemeProvider.tsx`:
- Around line 40-49: The ThemeProvider currently calls
useState(getInitialTheme()) which returns "light" during SSR, causing a flash
before client theme is applied; to fix, move initial theme sync out of React
render by adding a small blocking script in the app head that reads
localStorage/ui-theme or prefers-color-scheme and sets
document.body.setAttribute("data-bs-theme", ...) before hydration, and keep
ThemeProvider's getInitialTheme and useEffect as fallback (or add
suppressHydrationWarning on the body if you opt to allow mismatch); update
references in ThemeProvider, getInitialTheme, and the useEffect that sets
document.body and localStorage to ensure consistent behavior.
In `@docs/components/sections/ContactSection.tsx`:
- Around line 184-187: Delete the commented-out iframe block inside the
ContactSection component (the commented lines containing the iframe with
className "w-full h-[230px] border-0" and the MailList src URL) so no
dead/commented code remains; if the iframe approach should be preserved for
future reference, create a short issue or add a one-line comment referencing
that issue number instead of leaving the full iframe commented in
ContactSection.tsx.
- Around line 27-56: Replace the inline style props on the icon components with
Tailwind utility classes: remove style={{width: "24px", height: "24px"}} and add
a className such as "w-6 h-6" (or equivalent sizing) to each of DiscordIcon,
SlackIcon, and ZulipIcon so they use Tailwind for dimensions while keeping the
existing alt and src attributes unchanged.
- Line 85: Remove the unused ref by deleting the containerRef declaration (const
containerRef = useRef<HTMLDivElement>(null)) and removing any JSX ref usage
(e.g., ref={containerRef}) inside the ContactSection component; also drop the
useRef import if it becomes unused. This cleans up the ContactSection component
where containerRef is created and attached but never used.
In `@docs/components/sections/Footer.tsx`:
- Line 88: The footer currently hard-codes "2015-2026" which will become stale;
update the Footer component to compute the end year at render time by using
JavaScript Date (e.g., const currentYear = new Date().getFullYear()) and render
either a single year or a range (e.g., startYear === currentYear ?
`${startYear}` : `${startYear}-${currentYear}`) instead of the literal "2026";
make this change inside the Footer (exported Footer component / JSX where the
copyright string is composed).
- Around line 40-65: The anchor tags in Footer.tsx that render icon-only links
(the anchors containing SiLinkedin, SiYoutube, and Mail) lack accessible names;
add an accessible label to each link by adding an aria-label (or include
visually hidden text) describing the destination (e.g., aria-label="Yscope on
LinkedIn", aria-label="Yscope on YouTube", aria-label="Email Yscope") to the
three anchors so screen readers can announce their purpose; update the anchors
where SiLinkedin, SiYoutube, and Mail are used.
In `@docs/components/sections/Navbar.tsx`:
- Line 176: The Navbar component has inconsistent Dev Docs links: the desktop
link (href="/dev-guide/") and the mobile link (href="/dev-docs") should match;
update the href in the Navbar component (both the desktop anchor/render where
href is "/dev-guide/" and the mobile/menu render where href is "/dev-docs") to
use the same canonical path (pick one, e.g. "/dev-docs") so both Desktop and
Mobile buttons point to the identical route.
- Around line 276-280: The mapped link elements render duplicate key props —
remove the redundant key on the inner anchor and keep the key on the outer
wrapper (SheetClose) so React receives a single key per list item; update the
JSX in Navbar.tsx by deleting the key={link.href} from the <a> element (leaving
key={link.href} on SheetClose) to eliminate the duplicate key warning.
- Around line 70-83: The useEffect fetch in Navbar.tsx can update state after
unmount; add an AbortController: create controller inside useEffect, pass
controller.signal to fetch("https://api.github.com/repos/y-scope/clp"), and in
the catch ignore abort errors (check error.name === 'AbortError') so
setGithubStars is not called for aborted requests; return a cleanup function
that calls controller.abort() to cancel the fetch on unmount. Ensure the
existing response.ok/data handling remains but gate setGithubStars behind
non-aborted completion.
- Around line 60-64: The timeout set by projectDocsTimeoutRef in
handleProjectDocsLeave is never cleared on unmount (or when a new timeout is
created); add a React useEffect cleanup that clears the timeout via
clearTimeout(projectDocsTimeoutRef.current) and sets
projectDocsTimeoutRef.current = null on component unmount, and also ensure any
existing timeout is cleared before assigning a new one in handleProjectDocsLeave
to avoid stray state updates to setProjectDocsOpen and potential memory leaks
(PROJECT_DOCS_MENU_CLOSE_DELAY_MS, projectDocsTimeoutRef,
handleProjectDocsLeave, setProjectDocsOpen).
In `@docs/components/sections/ZohoSignupForm.tsx`:
- Around line 83-92: The onLoad handler currently treats any iframe load during
submission (in the onLoad callback) as a successful submit by calling
setSuccessVisible(true), clearing inputs (setEmail, setFirstName) and resetting
submitting; add a brief inline comment above this logic in the onLoad handler
explaining that iframe load detection is fragile for cross-origin forms because
the iframe may load on error pages or redirects, and that the message listener
is used as a secondary/authoritative check for success/failure—reference the
onLoad handler, submitting state, setSuccessVisible, and the existing message
listener so future maintainers understand the limitation and why both checks
exist.
- Around line 62-66: The handleMessage listener currently accepts any
postMessage and can be spoofed; update handleMessage to first validate ev.origin
against the expected Zoho iframe origin (or a configured allowed-origin value)
before inspecting ev.data, and only call setSuccessVisible(true) when origin
matches and ev.data is the expected success payload; also ensure the
window.addEventListener("message", handleMessage) registration is paired with a
removal in the component cleanup so the origin check is effective and no stale
listeners remain.
- Around line 134-142: The email input lacks the HTML email type which enables
native validation and mobile keyboards; update the input element used with state
variable email (and setEmail) in ZohoSignupForm to include type="email" (i.e.,
add the type attribute on the <input ... /> that has id "EMBED_FORM_EMAIL_LABEL"
and name "CONTACT_EMAIL") so browsers validate format and show the correct
keyboard on mobile.
In `@docs/lib/utils.ts`:
- Around line 15-17: The cn helper currently uses unsafe casts and an eslint
suppression; remove the eslint-disable comment and the (any) casts and simply
return twMerge(clsx(inputs)) typed as string if necessary. Update the function
named cn to call twMerge(clsx(inputs)) directly (no casting), relying on the
existing types from clsx and tailwind-merge, and only add an explicit return
type annotation for cn if you need to ensure the function signature is
string-returning. Ensure references to twMerge, clsx, and the inputs parameter
are used exactly as in the current helper.
In `@docs/next-sitemap.config.mjs`:
- Around line 1-7: The JSDoc type for the sitemap configuration is wrong:
replace the current Next.js type with next-sitemap's IConfig by updating the
JSDoc to reference import('next-sitemap').IConfig (e.g., change /** `@type`
{next.NextConfig} */ to /** `@type` {import('next-sitemap').IConfig} */) so the
config object named config (which contains changefreq, generateRobotsTxt,
transform, etc.) is validated against the correct next-sitemap v4 IConfig type;
ensure any leftover incorrect import/comment for next is removed or updated
accordingly.
In `@docs/next.config.ts`:
- Around line 3-19: The config sets output: "export" while also defining async
rewrites(), which conflicts because rewrites require a server runtime; remove
the rewrites() block or switch the export mode off (remove or change output:
"export") so rewrites can run — locate the output: "export" setting and the
async rewrites() function in the file and either delete/disable rewrites() or
change output to a non-static/server-capable mode.
- Around line 5-7: The config uses experimental.turboPack (camelCase) which is
incorrect for Next.js — update the key to the documented lowercase form
experimental.turbopack (or remove/replace it if Next.js 16 deprecated this
switch); locate the experimental.turboPack entry in next.config.ts and change it
to experimental.turbopack (verify against the official Next.js 16 docs and
adjust if the API expects a different structure or removal).
In `@docs/package.json`:
- Line 26: The next dependency is at major version 16 while eslint-config-next
is at major version 15; update the eslint-config-next entry to use the same
major (e.g., change "eslint-config-next" to ^16.x to match "next": "^16.0.7") so
both packages share the same major version and avoid incompatible lint/parser
behavior.
- Line 15: The lint script "lint:check:js" currently targets only "app" so new
TS/TSX files under components/ and config files aren’t checked; update the
"lint:check:js" package script to broaden ESLint targets (e.g., include
components and config and/or a project-wide glob) and add --ext
.js,.jsx,.ts,.tsx so ESLint runs on JS/TS/TSX files (adjust the "lint:check:js"
value to something like running eslint across app components config or a src/**
pattern with the proper --ext flag).
In `@README.md`:
- Around line 19-21: The fenced code block containing the submodule command (```
... git submodule update --init --recursive ... ```) needs blank lines before
and after it to satisfy markdownlint MD031; edit the README.md list item to
ensure there is an empty line immediately above the opening triple backticks and
an empty line immediately below the closing triple backticks so the fenced block
is separated from surrounding list text.
In `@taskfiles/docs.yaml`:
- Around line 72-80: The script block that runs the Next.js build uses fragile
shell steps (cd docs, npm install, npm run build, rsync) but does not fail fast;
add robust error handling by enabling a fail-fast shell (e.g., set -euo
pipefail) at the start of the block or explicitly check the exit status after
critical commands (check the result of cd docs, npm install, and npm run build)
and abort with a clear error if any step fails; ensure commands referenced (cd
docs, npm install --no-audit --no-fund, npm run build, rsync -a docs/out/
"{{.G_NEXT_OUTPUT_DIR}}/") run only after verifying previous steps succeeded so
the task stops on failure instead of continuing silently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: be7626bf-3175-41cf-b634-7b51e640e108
⛔ Files ignored due to path filters (27)
docs/app/favicon.icois excluded by!**/*.icodocs/package-lock.jsonis excluded by!**/package-lock.jsondocs/public/assets/images/api-server.svgis excluded by!**/*.svgdocs/public/assets/images/benchmarks.svgis excluded by!**/*.svgdocs/public/assets/images/blog.svgis excluded by!**/*.svgdocs/public/assets/images/clp-logo.pngis excluded by!**/*.pngdocs/public/assets/images/datasets.svgis excluded by!**/*.svgdocs/public/assets/images/discord.svgis excluded by!**/*.svgdocs/public/assets/images/docker-compose_icon.svgis excluded by!**/*.svgdocs/public/assets/images/json.svgis excluded by!**/*.svgdocs/public/assets/images/kubernetes.svgis excluded by!**/*.svgdocs/public/assets/images/log-ingestor.svgis excluded by!**/*.svgdocs/public/assets/images/log-viewer_icon.svgis excluded by!**/*.svgdocs/public/assets/images/mcp_dark.svgis excluded by!**/*.svgdocs/public/assets/images/mcp_light.svgis excluded by!**/*.svgdocs/public/assets/images/presto_dark.svgis excluded by!**/*.svgdocs/public/assets/images/presto_light.svgis excluded by!**/*.svgdocs/public/assets/images/publications.svgis excluded by!**/*.svgdocs/public/assets/images/python.svgis excluded by!**/*.svgdocs/public/assets/images/s3.svgis excluded by!**/*.svgdocs/public/assets/images/schema-file-syntax.svgis excluded by!**/*.svgdocs/public/assets/images/scripts.svgis excluded by!**/*.svgdocs/public/assets/images/single-node.svgis excluded by!**/*.svgdocs/public/assets/images/slack.svgis excluded by!**/*.svgdocs/public/assets/images/text.svgis excluded by!**/*.svgdocs/public/assets/images/zulip.svgis excluded by!**/*.svgdocs/public/bootstrap.bundle.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (28)
README.mddocs/.gitignoredocs/.prettyierrc.yamldocs/.stylelintrc.jsondocs/app/assets/css/colors.cssdocs/app/assets/scss/styles.scssdocs/app/content.tsdocs/app/layout.tsxdocs/app/page.tsxdocs/app/shared/ThemeProvider.tsxdocs/components/sections/ContactSection.tsxdocs/components/sections/Footer.tsxdocs/components/sections/Navbar.tsxdocs/components/sections/ZohoSignupForm.tsxdocs/components/ui/button.tsxdocs/components/ui/card.tsxdocs/components/ui/sheet.tsxdocs/conf.pydocs/eslint.config.mjsdocs/index.rstdocs/lib/utils.tsdocs/next-sitemap.config.mjsdocs/next.config.tsdocs/package.jsondocs/postcss.config.mjsdocs/tailwind.config.cjsdocs/tsconfig.jsontaskfiles/docs.yaml
Member
kirkrodrigues
left a comment
kirkrodrigues
left a comment
There was a problem hiding this comment.
Partial review based on what I've noticed so far:
- Can you address CodeRabbit's feedback first before requesting a review?
- There's a scrollbar when I view the homepage, both in Chrome and Firefox.
- Some of the links seem to link to pages on docs.yscope.com rather than within the local build.
- The Dev Docs button doesn't seem to work if I click it on the outer margin.
- The website sign-up form doesn't seem to have any validation.
- Can we keep the dev docs source and homepage source separate? It seems messy to mix them together.
|
|
||
| - |- | ||
| # Build Next.js homepage | ||
| rm -rf docs/out docs/.next |
Member
There was a problem hiding this comment.
Can we instead put all generated output in the build directory instead of within the source tree?
Author
There was a problem hiding this comment.
I tried a couple methods to make the happen, but I couldn't get it to work correctly. If this is important to you let me know and I could keep working on it.
Author
What has a scroll bar? I opened it in Firefox and Chrome, but I don’t see a scroll bar.
By the website signup form doesn’t have any validation, do you mean the newsletter sign up form? Do you just want me to add validation, that the email is in email form, etc. I do believe that they do some verification on there end before adding to the contact lists.
Would moving the dev docs folder out of the docs folder be enough. Sphinx tries to generate a home page and I had force it to use our home page and not its home page which might not work if I try to spectate fully. What would be your ideal file layout? I can see how close I can get it to that. |
Member
There's a horizontal scrollbar.
Sorry, yeah, the newsletter sign-up form. I think we would want at least basic validation---the fields aren't empty, the email address is actually an email, right?
Currently, you have the |
There was a problem hiding this comment.
Actionable comments posted: 27
♻️ Duplicate comments (3)
docs/homepage/components/sections/ZohoSignupForm.tsx (2)
111-146:⚠️ Potential issue | 🟠 MajorOnly mark signup success after a trusted Zoho success signal.
ev.origin.includes("zoho.com")still accepts spoofable origins likehttps://zoho.com.evil.example, and the iframeonLoadmarks any response as success anyway. Use exact allowed origins and avoid treating generic iframe loads as successful submissions.🔒 Proposed fix direction
+ const trustedMessageOrigins = new Set([ + "https://ma.zoho.com", + "https://zcsub-cmpzourl.maillist-manage.com", + ]); + // If the Zoho endpoint returns to the hidden iframe we can show a success useEffect(() => { const handleMessage = (ev: MessageEvent) => { - if (false === ev.origin.includes("zoho.com")) { + if (false === trustedMessageOrigins.has(ev.origin)) { return; } if ("string" === typeof ev.data && ev.data.includes("Thank you")) { setSuccessVisible(true); + setEmail(""); + setFirstName(""); + setSubmitting(false); } }; @@ onLoad={() => { - // If we submitted and the iframe finished loading, assume submission completed. - // -> Currently treats any iframe load during submission as a success if (submitting) { - setSuccessVisible(true); - - // clear controlled inputs after successful submission - setEmail(""); - setFirstName(""); setSubmitting(false); } }}/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/components/sections/ZohoSignupForm.tsx` around lines 111 - 146, The postMessage handler and iframe onLoad are marking successes insecurely: change the origin check in handleMessage to compare ev.origin against a small whitelist of exact allowed origins (e.g., "https://zoho.com" and any exact subdomains you trust) instead of using includes, and only call setSuccessVisible(true) when the message payload explicitly indicates a Zoho success (check ev.data structure/value); remove success logic from the iframe onLoad handler (or gate it behind a verified flag set only by a trusted postMessage) so that generic iframe loads do not trigger success, updating references in this component (handleMessage, window.addEventListener, iframe onLoad, setSuccessVisible, submitting, and any iframeName usage) accordingly.
193-204:⚠️ Potential issue | 🟡 MinorAdd native email input validation.
The controlled email field still lacks
type="email", so mobile keyboards and browser-level email validation are not enabled.💡 Proposed fix
<input className={"text-input w-100"} id={"EMBED_FORM_EMAIL_LABEL"} name={"CONTACT_EMAIL"} placeholder={"Email"} + type={"email"} value={email} onChange={(e) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/components/sections/ZohoSignupForm.tsx` around lines 193 - 204, The email input in ZohoSignupForm.tsx is missing native email validation; update the input element (the one with id "EMBED_FORM_EMAIL_LABEL" and props value={email}, onChange={(e) => { setEmail(e.target.value); if (emailError) setEmailError(null); }}) to include type="email" (and optionally inputMode="email" for better mobile keyboards) so browser-level validation and mobile email keyboards are enabled.taskfiles/docs.yaml (1)
83-92: 🛠️ Refactor suggestion | 🟠 MajorRedundant
--include='*/index.html'and two-step copy can collapse into onersyncinvocation.
--include='*/index.html'is a no-op here because there's no catch-all--exclude='*'after it — everything is already included by default, so only--exclude='/index.html'has any effect. Also, the two sequentialrsynccalls can be a single command using a filter rule that excludes the dev-docs rootindex.htmlwhile overlaying the homepage output on top, as previously suggested by@kirkrodrigues.♻️ Proposed consolidation
# Merge the homepage and dev docs - |- + set -euo pipefail rm -rf "{{.OUTPUT_DIR}}" mkdir -p "{{.OUTPUT_DIR}}" - - # Copy dev docs - rsync -a --exclude='/index.html' --include='*/index.html' "{{.G_DEV_DOCS_OUTPUT_DIR}}/" "{{.OUTPUT_DIR}}/" - - # Copy the homepage - rsync -a "{{.G_HOMEPAGE_OUTPUT_DIR}}/" "{{.OUTPUT_DIR}}/" + rsync -a --exclude='/index.html' "{{.G_DEV_DOCS_OUTPUT_DIR}}/" "{{.OUTPUT_DIR}}/" + rsync -a "{{.G_HOMEPAGE_OUTPUT_DIR}}/" "{{.OUTPUT_DIR}}/"(Note: trailing whitespace on line 90 after
"{{.OUTPUT_DIR}}/"— the blank separator line has a stray space — also gets cleaned up above.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@taskfiles/docs.yaml` around lines 83 - 92, The two-step rsync and redundant --include rule should be collapsed: remove the no-op --include='*/index.html' and replace the two sequential rsync calls with a single rsync invocation that copies from G_DEV_DOCS_OUTPUT_DIR to OUTPUT_DIR while excluding the dev-docs root index.html (use an appropriate --filter/--exclude rule) and then overlays G_HOMEPAGE_OUTPUT_DIR into OUTPUT_DIR in one pass or as a subsequent rsync that only adds/overwrites files (use the same OUTPUT_DIR target and ensure no trailing whitespace after the rsync lines); reference the rsync invocations and the variables G_DEV_DOCS_OUTPUT_DIR, G_HOMEPAGE_OUTPUT_DIR and OUTPUT_DIR to locate the block to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/homepage/.prettyierrc.yaml`:
- Around line 1-6: Rename the configuration file `.prettyierrc.yaml` to
`.prettierrc.yaml` (fix the missing 't') so Prettier can load the settings; keep
the contents (tabWidth, useTabs, singleQuote, quoteProps, printWidth, endOfLine)
unchanged and ensure the new filename is committed/renamed in the repo.
In `@docs/homepage/app/assets/scss/styles.scss`:
- Around line 398-426: There are two conflicting `.container` blocks: one
applies Tailwind's container utility via `@apply container mx-auto` and the
other manually sets paddings that override Tailwind’s container padding; pick
one approach and remove the other to avoid dual sources of truth—either remove
the manual padding rules (the second `.container` block and its media queries)
and set `container.padding` in tailwind.config.cjs to 2.5rem/5rem/10rem, or keep
the manual padding and change the first block to only `@apply mx-auto` (remove
`@apply container`) so paddings are controlled solely by the SCSS; update or
remove the `.container` definitions accordingly and keep `.container-fluid`
untouched.
- Around line 27-29: The global rule "html, body { overflow-x: hidden; }" hides
the real cause of the horizontal scrollbar and should be removed; instead find
the offending element (commonly a full-bleed row, a container using negative
margins, or a wide SVG/iframe) and constrain it there—inspect elements that
render wider than 100vw and fix them by adding max-width: 100%; overflow-x:
auto/hidden on that specific selector (e.g., the .full-bleed, .container--wide,
or the SVG/iframe selector), or remove/adjust negative margins so the element no
longer exceeds the viewport; replace the global overflow rule with targeted
fixes and verify sticky positioning and anchor scrolling behave correctly.
- Around line 3-19: Replace fragile relative node_modules imports in styles.scss
by relying on Sass/Next.js include paths: remove the "../../../node_modules/..."
prefixes in each `@import` (e.g., the imports referencing
"tw-animate-css/dist/tw-animate.css" and "bootstrap/scss/functions",
"bootstrap/scss/variables", etc.) and update project Sass configuration (Sass
loadPaths / includePaths or Next.js sassOptions.includePaths) to include
"node_modules" so the imports resolve by module name; ensure all occurrences in
styles.scss use module-style imports like "bootstrap/..." and
"tw-animate-css/..." so moving the file won’t break resolution.
In `@docs/homepage/app/layout.tsx`:
- Line 56: Replace the hardcoded GA4 ID in the GoogleAnalytics component with an
environment-driven value: read process.env.NEXT_PUBLIC_GA_ID and only render
<GoogleAnalytics gaId={...}/> when process.env.NODE_ENV === "production" and
NEXT_PUBLIC_GA_ID is set (to avoid analytics in dev/preview). Update the code
that renders GoogleAnalytics in layout.tsx (the GoogleAnalytics component
invocation) to use the env var and short-circuit rendering when the variable is
falsy or NODE_ENV !== "production".
- Around line 58-67: Replace the raw Google Fonts <link> tags in layout.tsx with
a next/font/google import for ABeeZee: import ABeeZee from 'next/font/google',
instantiate it with the same subsets/display and a CSS variable (e.g., variable:
'--font-abeezee'), remove the three <link> entries shown in the diff, and add
the new font's CSS variable to the root/html/className usage alongside
Geist/Geist_Mono so ABeeZee is self-hosted, preloaded, and exposed as a CSS
variable like the other fonts.
- Around line 55-57: The GoogleAnalytics component is incorrectly placed between
<html> and <head>; move the GoogleAnalytics element so it is rendered as the
last child of the <html> element (i.e., after the closing </body>) to comply
with Next.js `@next/third-parties/google` guidance and avoid hydration/script
relocation issues—locate the GoogleAnalytics usage in layout.tsx (the
GoogleAnalytics component invocation) and relocate it to the end of the HTML
tree (after the body contents) so it becomes the final child of <html>.
In `@docs/homepage/app/page.tsx`:
- Around line 49-77: The IconLink component currently expects an isExternal prop
but getCategories() supplies external, causing external links to open in the
same tab; update the call-sites that render IconLink (where you pass category
props from getCategories()) to pass isExternal={category.external} or change the
IconLink prop name to external and update its signature and usages accordingly
so the external flag is wired correctly (refer to IconLink and getCategories).
- Around line 188-242: The inner wrapper using Bootstrap's "row" (the div with
className "row gap-4 px-4 align-items-center justify-content-center" that
contains the Button components) should be changed to a plain flex wrapper to
avoid row negative margins: remove "row" and replace it with the Bootstrap
utility "d-flex" (optionally add "flex-row" or "flex-wrap" if needed) so the
className becomes e.g. "d-flex gap-4 px-4 align-items-center
justify-content-center"; keep the surrounding "col my-4" and the Button elements
(Quickstart and Current Release) unchanged.
In `@docs/homepage/app/shared/ThemeProvider.tsx`:
- Around line 17-34: getInitialTheme currently accesses localStorage.getItem and
window.matchMedia during initial render which can throw; update getInitialTheme
to guard these calls: keep the existing typeof window checks, wrap
localStorage.getItem("ui-theme") in a try/catch and only use the value if it
equals "dark" or "light", and check that window.matchMedia exists (typeof
window.matchMedia === "function") before calling .matches; ensure any caught
errors are ignored and the function falls back to the default return ("light")
so initial render cannot throw.
In `@docs/homepage/components/icons/GithubIcon.tsx`:
- Line 16: The component prop typing for GithubIcon overlaps
SVGProps<SVGSVGElement> by re-declaring height, width and fill as numbers,
excluding valid string values; update the signature to Omit the three props from
SVGProps and then re-add them with broader types (e.g., height?: string |
number; width?: string | number; fill?: string) so the component accepts both
string and number dimensions and avoids redundancy — adjust the GithubIcon
parameter type and ensure usages of props and ...props remain unchanged.
- Around line 26-37: The SVG path for the GithubIcon is built via concatenated
string literals which hurts readability; update the d prop in GithubIcon (the
JSX attribute that currently sets d={ "..."+ "..."} ) to use a single string
literal (one quoted string or a template literal) containing the entire path
data, keeping the exact path content unchanged so the icon renders identically.
In `@docs/homepage/components/sections/Footer.tsx`:
- Around line 60-68: Remove the unnecessary target and rel attributes from the
mailto anchor in Footer.tsx: locate the <a> element with
href="mailto:info@yscope.com" (aria-label "Contact YScope via email", className
"nav-link-btn-alt", children include the Mail component) and delete the
target={"_blank"} and rel={"noopener noreferrer"} props so the mailto URL is
handled natively by the browser/mail client.
- Around line 21-31: In the Footer component replace the bare <a
className="logo" href="/"> element with Next's client-side Link: import Link
from "next/link" and render the logo using <Link href="/"> while preserving
className="logo", the inline style object, and inner text "YScope" (or, if using
Next 12/older semantics, wrap the existing <a> inside <Link>); this ensures
client-side navigation and respects any future basePath.
- Around line 17-85: The .row element inside the Footer component is missing a
.container parent which breaks Bootstrap grid margins; update the JSX in
Footer.tsx so the trailing <div className={"row"}> (the block that contains the
copyright <p> / text-center paragraph) is wrapped inside a sibling <div
className={"container"}> just before that .row and after the existing closing
container, ensuring the .row has a direct .container parent to restore correct
layout.
In `@docs/homepage/components/sections/Navbar.tsx`:
- Around line 157-174: projectDocsLinks mapping currently renders an <a> that
wraps a Button, creating nested interactive elements; change this so only one
interactive element is used by either (A) rendering the Button as an anchor (use
Button's asChild/elementType/href prop offered by your UI library) and move the
onClick/e.stopPropagation to that Button-as-anchor, or (B) remove the <a> and
style the Button to act as the link (set href/navigation on Button and apply
className "text-decoration-none text-sm"); update the same pattern in the other
occurrence (the block around lines 271-288) and ensure stopPropagation remains
applied to the single interactive element to prevent parent clicks.
- Around line 137-152: The Project Docs dropdown only opens on mouse hover;
update the trigger handling so keyboard users can open/close and see state: add
focus and onClick handlers to the Button alongside the existing
onMouseEnter/onMouseLeave (e.g., call existing
handleProjectDocsEnter/handleProjectDocsLeave or new toggle handlers), ensure
projectDocsOpen is toggled on Enter/Space and on click, and add proper ARIA
attributes on the Button (aria-haspopup="menu" and
aria-expanded={projectDocsOpen}) and keyboard navigation considerations for the
dropdown items so the Menu/links are reachable via keyboard. Use the existing
Button and ChevronDown symbols and preserve the rotation logic tied to
projectDocsOpen.
In `@docs/homepage/components/sections/ZohoSignupForm.tsx`:
- Around line 52-53: Replace the unary negation in the email validity check with
the repository's explicit boolean-comparison style: change the conditional that
uses !isEmailValid to use an explicit comparison against false (e.g., false ==
isEmailValid) while keeping the rest of the condition (MIN_EMAIL_LENGTH >
emailVal.length) unchanged; update the if statement that references
isEmailValid, MIN_EMAIL_LENGTH, and emailVal in ZohoSignupForm.tsx accordingly.
In `@docs/homepage/components/ui/button.tsx`:
- Line 32: The themeToggle variant in the Button component is missing the
theme-toggle-btn class, so update the variant definition for themeToggle
(symbol: themeToggle) to include "theme-toggle-btn" alongside the existing
"rounded-md d-inline-flex align-items-center" so the component applies the
.theme-toggle-btn SCSS styles automatically; follow the same pattern used by the
github variant (symbol: btn-github) to ensure theme-aware colours, borders, and
hover states are applied without relying on callers to pass the className prop.
In `@docs/homepage/components/ui/sheet.tsx`:
- Around line 146-148: The `mobile-menu-sheet` token is placed after the caller
`className` inside the cn(...) call so it wins via twMerge; adjust the order in
the Sheet component's class composition: if `mobile-menu-sheet` must be
immutable, hard-code it at the start of the class list passed to cn(...);
otherwise move `"mobile-menu-sheet"` before the `className` argument so a
caller-supplied `className` can override it. Locate the cn(...) invocation that
currently lists `className, "mobile-menu-sheet"` and reorder accordingly.
In `@docs/homepage/eslint.config.mjs`:
- Around line 3-19: The StylisticConfigArray entry was commented out in the
EslintConfig which silently disables stylistic rules; update the EslintConfig to
either re-enable StylisticConfigArray after fixing the ESM/CJS loader cycle or,
if the cycle can't be resolved immediately, add an explicit TODO with a tracking
issue ID so this workaround isn't forgotten (reference the commented
StylisticConfigArray and the EslintConfig array in the file and include the
TODO(`#issue`) and brief note about the ESM/CJS loader cycle).
In `@docs/homepage/next-sitemap.config.mjs`:
- Around line 13-20: The transform function currently re-specifies top-level
defaults (changefreq and priority) unnecessarily; either remove the transform
entirely so next-sitemap uses its defaults (which already set lastmod) or
simplify the transform function (the transform function named transform that
returns { loc, lastmod, changefreq, priority }) to only set lastmod (and loc) —
e.g., keep loc and set lastmod to new Date().toISOString() and remove
changefreq/priority from the returned object.
- Around line 1-5: Replace the inert backticked comments with valid directives:
either remove them or change the top comments to a proper block disable (e.g.,
/* eslint-disable `@typescript-eslint/require-await`,
`@typescript-eslint/no-unsafe-assignment`, sort-keys */) or per-line //
eslint-disable-next-line `@typescript-eslint/require-await` etc., and fix the
JSDoc by removing backticks so it reads /** `@type`
{import('next-sitemap').IConfig} */ (referencing the `@type` JSDoc and the
eslint-disable directives in this file).
In `@docs/homepage/next.config.ts`:
- Around line 1-9: Replace the CommonJS untyped config with a native ESM
TypeScript export: remove the top "// `@ts-nocheck`", add an import type for
NextConfig from "next", type the nextConfig object (const nextConfig: NextConfig
= { ... }), drop the empty turbopack: {} property, and export the config with
"export default nextConfig" instead of module.exports; target the nextConfig
symbol and the turbopack property when making the changes.
In `@docs/homepage/package.json`:
- Line 11: The package.json "start" script currently runs "next start", but
next.config.ts sets output:"export" (static export), and "next start" is
unsupported for static exported sites; update the "start" script in package.json
to invoke a static file server that serves the export output directory (the
folder produced by next export, e.g., "out"), e.g., use a simple static server
CLI (serve/http-server or similar) or a hosting-specific serve command so the
built static site is served correctly instead of using next start.
In `@docs/homepage/tailwind.config.cjs`:
- Around line 8-23: Tailwind v4 ignores theme.container options (center,
padding, screens) in tailwind.config.cjs, so remove those keys from
theme.container and migrate the container rules into your main CSS entry (your
styles.scss) using the `@utility` container directive; implement margin-inline:
auto, padding-inline values (1rem default, 2rem at xl breakpoint, 4rem at 2xl),
and set max-widths corresponding to the screen breakpoints (540px, 720px, 960px,
1140px, 1420px) with matching media queries so the container behavior is
restored.
In `@taskfiles/docs.yaml`:
- Around line 74-81: The shell step currently uses "cd docs/homepage || exit 0"
which masks cd failures and can let rsync copy wrong/stale content; change that
to fail fast by either enabling strict mode (e.g., set -euo pipefail at the top
of the script block) or replacing the short-circuit with a non-zero exit (e.g.,
"cd docs/homepage || exit 1") so that the subsequent npm install/npm run build
and rsync -a docs/homepage/out/ only run when the directory change succeeds;
update the shell command that contains "cd docs/homepage || exit 0" (and ensure
mkdir -p "{{.G_HOMEPAGE_OUTPUT_DIR}}" / rsync are preserved) accordingly.
---
Duplicate comments:
In `@docs/homepage/components/sections/ZohoSignupForm.tsx`:
- Around line 111-146: The postMessage handler and iframe onLoad are marking
successes insecurely: change the origin check in handleMessage to compare
ev.origin against a small whitelist of exact allowed origins (e.g.,
"https://zoho.com" and any exact subdomains you trust) instead of using
includes, and only call setSuccessVisible(true) when the message payload
explicitly indicates a Zoho success (check ev.data structure/value); remove
success logic from the iframe onLoad handler (or gate it behind a verified flag
set only by a trusted postMessage) so that generic iframe loads do not trigger
success, updating references in this component (handleMessage,
window.addEventListener, iframe onLoad, setSuccessVisible, submitting, and any
iframeName usage) accordingly.
- Around line 193-204: The email input in ZohoSignupForm.tsx is missing native
email validation; update the input element (the one with id
"EMBED_FORM_EMAIL_LABEL" and props value={email}, onChange={(e) => {
setEmail(e.target.value); if (emailError) setEmailError(null); }}) to include
type="email" (and optionally inputMode="email" for better mobile keyboards) so
browser-level validation and mobile email keyboards are enabled.
In `@taskfiles/docs.yaml`:
- Around line 83-92: The two-step rsync and redundant --include rule should be
collapsed: remove the no-op --include='*/index.html' and replace the two
sequential rsync calls with a single rsync invocation that copies from
G_DEV_DOCS_OUTPUT_DIR to OUTPUT_DIR while excluding the dev-docs root index.html
(use an appropriate --filter/--exclude rule) and then overlays
G_HOMEPAGE_OUTPUT_DIR into OUTPUT_DIR in one pass or as a subsequent rsync that
only adds/overwrites files (use the same OUTPUT_DIR target and ensure no
trailing whitespace after the rsync lines); reference the rsync invocations and
the variables G_DEV_DOCS_OUTPUT_DIR, G_HOMEPAGE_OUTPUT_DIR and OUTPUT_DIR to
locate the block to change.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 27fead02-7925-471f-9a92-cf7759092afb
⛔ Files ignored due to path filters (28)
docs/dev-docs/_static/favicon.icois excluded by!**/*.icodocs/homepage/app/favicon.icois excluded by!**/*.icodocs/homepage/package-lock.jsonis excluded by!**/package-lock.jsondocs/homepage/public/assets/images/api-server.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/benchmarks.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/blog.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/clp-logo.pngis excluded by!**/*.pngdocs/homepage/public/assets/images/datasets.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/discord.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/docker-compose_icon.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/json.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/kubernetes.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/log-ingestor.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/log-viewer_icon.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/mcp_dark.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/mcp_light.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/presto_dark.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/presto_light.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/publications.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/python.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/s3.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/schema-file-syntax.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/scripts.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/single-node.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/slack.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/text.svgis excluded by!**/*.svgdocs/homepage/public/assets/images/zulip.svgis excluded by!**/*.svgdocs/homepage/public/bootstrap.bundle.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (38)
README.mddocs/dev-docs/_static/clp-versions.jsondocs/dev-docs/_static/custom.cssdocs/dev-docs/conf.pydocs/dev-docs/dev-guide/contrib-guides-cpp.mddocs/dev-docs/dev-guide/contrib-guides-general.mddocs/dev-docs/dev-guide/contrib-guides-overview.mddocs/dev-docs/dev-guide/contrib-guides-taskfiles.mddocs/dev-docs/dev-guide/index.mddocs/dev-docs/dev-guide/misc-deploying.mddocs/dev-docs/index.mddocs/dev-docs/index.rstdocs/homepage/.gitignoredocs/homepage/.prettyierrc.yamldocs/homepage/.stylelintrc.jsondocs/homepage/app/assets/css/colors.cssdocs/homepage/app/assets/scss/styles.scssdocs/homepage/app/content.tsdocs/homepage/app/layout.tsxdocs/homepage/app/page.tsxdocs/homepage/app/shared/ThemeProvider.tsxdocs/homepage/components/icons/GithubIcon.tsxdocs/homepage/components/sections/ContactSection.tsxdocs/homepage/components/sections/Footer.tsxdocs/homepage/components/sections/Navbar.tsxdocs/homepage/components/sections/ZohoSignupForm.tsxdocs/homepage/components/ui/button.tsxdocs/homepage/components/ui/card.tsxdocs/homepage/components/ui/sheet.tsxdocs/homepage/eslint.config.mjsdocs/homepage/lib/utils.tsdocs/homepage/next-sitemap.config.mjsdocs/homepage/next.config.tsdocs/homepage/package.jsondocs/homepage/postcss.config.mjsdocs/homepage/tailwind.config.cjsdocs/homepage/tsconfig.jsontaskfiles/docs.yaml
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (6)
docs/homepage/eslint.config.mjs (1)
3-16:⚠️ Potential issue | 🟡 MinorTrack the disabled stylistic rules before this becomes permanent.
StylisticConfigArrayis still commented out without a TODO or issue reference. Please either re-enable it or add an explicit tracking comment near Lines 3 and 16.#!/bin/bash # Description: Verify whether the disabled StylisticConfigArray workaround has a tracking TODO. # Expected after fix: the disabled StylisticConfigArray lines include a TODO with an issue reference, # or the import/spread is re-enabled. rg -n -C2 'StylisticConfigArray|TODO\(#' docs/homepage/eslint.config.mjs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/eslint.config.mjs` around lines 3 - 16, The commented-out StylisticConfigArray import and spread are left disabled without tracking; either re-enable them (uncomment the import "StylisticConfigArray" and the spread "...StylisticConfigArray" in EslintConfig) or add an explicit TODO comment with an issue/reference immediately above both the import line and the array spread mentioning the reason and ticket (e.g., TODO(`#1234`): temporary disablement) so rg searches for 'StylisticConfigArray' or 'TODO(#' will find the tracking note.docs/homepage/next-sitemap.config.mjs (1)
1-1:⚠️ Potential issue | 🟡 MinorFix the remaining JSDoc
@typeannotation.Line 1 still wraps
@typein backticks, so editors and type checkers treat it as plain comment text rather than a JSDoc type annotation.🛠️ Proposed fix
-/** `@type` {import('next-sitemap').IConfig} */ +/** `@type` {import("next-sitemap").IConfig} */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/next-sitemap.config.mjs` at line 1, The top-of-file JSDoc currently has the `@type` tag wrapped in backticks (the line that reads `/** `@type` {import('next-sitemap').IConfig} */`), which prevents editors/type-checkers from recognizing it; fix this by removing the surrounding backticks so the JSDoc contains a proper `@type` tag and keep the import('next-sitemap').IConfig reference unchanged.docs/homepage/app/shared/ThemeProvider.tsx (1)
26-37:⚠️ Potential issue | 🔴 CriticalDuplicate
localStorage.getItem— the unguarded read at lines 26–29 defeats the try/catch fix and can throw during initial render.Lines 26–29 perform an unguarded
localStorage.getItem("ui-theme")before the try/catch block added at lines 30–37. When storage access throws (Safari Private Browsing, sandboxed iframes, browsers with cookies/storage disabled), the error propagates out ofgetInitialTheme()during initial render and crashes the tree before the ThemeProvider effect can run. The second block is also dead code — if lines 26–29 don't throw and return a valid value, they already returned; if they return nothing (null or unknown string), lines 31–34 repeat the same check under the guard.The original intent (per past review feedback) was to replace the unguarded read with the guarded one, not append it.
🐛 Proposed fix
- const stored = localStorage.getItem("ui-theme"); - if ("dark" === stored || "light" === stored) { - return stored; - } try { const stored = localStorage.getItem("ui-theme"); if ("dark" === stored || "light" === stored) { return stored; } } catch { // Ignore storage access failures. }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/app/shared/ThemeProvider.tsx` around lines 26 - 37, getInitialTheme currently reads localStorage unguarded then repeats the same read inside a try/catch, which can throw during initial render; remove the first unguarded call and only perform the guarded read inside getInitialTheme (the try/catch block), returning the stored value if it's "dark" or "light" and otherwise falling back to the default — update the function in ThemeProvider.tsx (getInitialTheme) to only use the guarded localStorage.getItem("ui-theme") and keep the catch that ignores storage access failures.docs/homepage/package.json (1)
11-11:⚠️ Potential issue | 🔴 Critical
next serve outis not a valid Next.js CLI command.The Next.js CLI does not expose a
servesubcommand (onlydev,build,start,lint,info,telemetry). Invokingnpm startin this package will therefore fail. Withoutput: "export"set innext.config.ts,next startisn't supported either, so a static file server is needed.🐛 Proposed fix
- "start": "next serve out", + "start": "npx serve out",Or add
serveas a devDependency and drop thenpx.Does Next.js 16 CLI support a "next serve" subcommand?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/package.json` at line 11, The "start" script in package.json currently uses an invalid Next.js CLI command ("next serve out"); update the package.json "start" script to use a static file server since next.config.ts uses output: "export". Replace the script value with a valid static-server invocation (for example "npx serve -s out" or "npx http-server out") OR add "serve" as a devDependency and set the script to "serve -s out"; ensure the script name "start" is updated and tested to serve the exported "out" directory.docs/homepage/components/sections/Navbar.tsx (1)
162-195:⚠️ Potential issue | 🟠 Major
onBlurcloses the dropdown when focus moves between items inside it.
onBlur={handleProjectDocsLeave}fires every time focus leaves any element in the container, including when the user tabs from the trigger button into the dropdown links — collapsing the menu the user was just about to use. Checke.relatedTargetagainste.currentTargetbefore starting the close timer so intra-container focus movement is preserved.♿ Proposed fix
- onBlur={handleProjectDocsLeave} + onBlur={(e) => { + if (null === e.relatedTarget || + false === e.currentTarget.contains(e.relatedTarget as Node)) { + handleProjectDocsLeave(); + } + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/components/sections/Navbar.tsx` around lines 162 - 195, The onBlur handler (handleProjectDocsLeave) currently closes the dropdown whenever any child loses focus, collapsing the menu during intra-container focus moves; update handleProjectDocsLeave to inspect the blur event's relatedTarget (or use projectDocsContainerRef.current.contains(relatedTarget)) and bail out (do not start the close timer) if relatedTarget is still inside the projectDocs container, otherwise proceed with the existing timeout/close logic; ensure any timeout clear logic (as used in onFocus) is preserved and referenced in handleProjectDocsLeave so focus moves between the Button and dropdown links don't close the menu.taskfiles/docs.yaml (1)
74-81:⚠️ Potential issue | 🟠 MajorBuild steps still run even when
npm installfails — addset -euo pipefail.Although
cd docs/homepage || exit 1now fails fast on a missing directory, the rest of the block runs with the default shell behaviour: ifnpm install --no-audit --no-fundfails (e.g., network outage or lockfile mismatch),npm run buildwill still execute and thersyncwill happily sync a staledocs/homepage/out/, producing a green task with incorrect artifacts. Enable strict mode so any failure aborts the step.🛡️ Proposed fix
# Build the homepage - |- + set -euo pipefail rm -rf docs/homepage/out docs/homepage/.next cd docs/homepage || exit 1 npm install --no-audit --no-fund npm run build cd - mkdir -p "{{.G_HOMEPAGE_OUTPUT_DIR}}" rsync -a docs/homepage/out/ "{{.G_HOMEPAGE_OUTPUT_DIR}}/"The same hardening is warranted for the merge step at lines 84–92.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@taskfiles/docs.yaml` around lines 74 - 81, Add strict shell error handling by inserting set -euo pipefail at the start of the shell command block that begins with rm -rf docs/homepage/out docs/homepage/.next and ends with rsync -a docs/homepage/out/ "{{.G_HOMEPAGE_OUTPUT_DIR}}/"; also add the same set -euo pipefail at the start of the separate merge step block (the block that creates the target dir and runs rsync to merge outputs) so any failing command (e.g., npm install or npm run build) will abort the step immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/homepage/app/content.ts`:
- Around line 20-140: Several internal hrefs in the items arrays are missing
trailing slashes (e.g. "/clp/main/user-docs/guides-docker-compose-deployment",
"/clp/main/user-docs/guides-k8s-deployment",
"/clp/main/user-docs/guides-using-log-ingestor", etc.) which is inconsistent
with Next.js trailingSlash: true; update every internal href (strings starting
with "/") in the items arrays in this file to ensure they end with a "/" (append
one if missing) while leaving external links (isExternal true or starting with
"http") unchanged—search the objects under the section titles like "Deploy",
"Log Input", "Analyze & View", "Resources", and "References" and normalize their
href properties.
In `@docs/homepage/app/layout.tsx`:
- Around line 6-11: Reformat the inconsistent named-import block so its
indentation matches the other imports: align the specifiers for "ABeeZee as
abeezeeFont", "Geist as geistFont", and "Geist_Mono as geistMonoFont" inside the
braces to the same indentation style used by the other import blocks in this
file (and keep the import of Script from "next/script" unchanged); update the
import statement that declares abeezeeFont, geistFont, and geistMonoFont to use
the consistent multi-line indentation so ESLint/Prettier won't flag it.
In `@docs/homepage/app/page.tsx`:
- Around line 126-130: The code uses the array index as a React key for the
IconLink list (key={i}), which can cause reconciliation bugs if items becomes
dynamic; update the mapping to use a stable identifier from each item such as
key={it.href} or key={it.label} (or a dedicated id property) when rendering
items.map in the IconLink component so each child has a consistent, unique key
instead of the index.
- Around line 86-90: Replace the raw <img> usage for the homepage icons with
Next.js's Image component (import Image from "next/image") to restore Next.js
asset handling and width/height linting: use Image with src={imgSrc},
alt={imgAlt}, className="homepage_icon", and supply explicit width/height or use
layout/fill as appropriate; if using static export keep unoptimized={true}. If
these are inline SVGs or you intentionally must keep <img>, add a short comment
next to the <img> referencing why next/image was not used (e.g., "SVG inline,
using <img> intentionally — explains constraints") and ensure variables imgAlt,
imgSrc, imgStyle and the className "homepage_icon" are preserved.
In `@docs/homepage/components/icons/GithubIcon.tsx`:
- Around line 3-16: Change the runtime import of SVGProps to a type-only import
to avoid preserving a runtime dependency; replace the current import line with
"import type { SVGProps } from 'react';" and keep the rest of the GithubIcon
component signature and props type (Omit<SVGProps<SVGSVGElement>, ...> & {...})
unchanged so only the import statement is updated.
In `@docs/homepage/components/sections/Navbar.tsx`:
- Around line 315-316: The "Project Docs" UI is rendered as a non-functional
<Button> inside the Navbar component which is announced as an actionable control
but has no onClick/href/keyboard behavior; either replace that Button element
that renders the "Project Docs" label with a semantic heading or paragraph
(e.g., an <h3> or <p>) preserving the existing styling, or make it a real
interactive control by wiring an onClick/href and keyboard handlers plus
appropriate ARIA attributes (e.g., role="button" and onKeyDown handling) so the
Button becomes functional; update the JSX where the Button with text "Project
Docs" is rendered in the Navbar component accordingly.
- Around line 254-268: The markup nests an <svg> around the GithubIcon component
which itself returns an <svg>, producing invalid/incorrect rendering; remove the
outer <svg> wrapper and render <GithubIcon /> directly, passing the intended
attributes (fill="currentColor", height={20}, width={20}, viewBox="0 0 24 24",
xmlns if needed) to the GithubIcon call, and apply the same change in the mobile
menu instance (the other occurrence near the mobile menu rendering); keep the
surrounding link and the githubStars/formatNumber logic unchanged.
- Around line 83-98: Replace the negation usage when toggling project docs with
an explicit comparison: in handleProjectDocsKeyDown change "const next = !v;" to
"const next = false === v;" and make the identical change in the corresponding
onClick handler (the handler that calls setProjectDocsOpen with (v) => { const
next = !v; ... }) so both places use "const next = false === v;" to compute the
toggled value before focusing the first item.
In `@docs/homepage/components/sections/ZohoSignupForm.tsx`:
- Around line 18-34: Remove the misplaced duplicate JSDoc block that precedes
the MIN_EMAIL_LENGTH and MIN_NAME_LENGTH constant declarations; keep the correct
JSDoc immediately above the ZohoSignupForm component and delete the earlier,
identical comment so only the ZohoSignupForm JSDoc (the block that documents the
component) remains directly above the ZohoSignupForm definition.
- Line 99: The long inline type assertion and long className exceed
max-line-length; define a reusable type alias (e.g., type SetupSFType = (formIx:
string, trackCode: string, autoOptIn: boolean, theme: string) => void) and use
it to cast setupSF once (const setupSFTyped = setupSF as SetupSFType) then call
setupSFTyped(FORM_IX, "ZCFORMVIEW", false, "light"); likewise extract the long
input className into a named constant (e.g., const inputClass = ['cls1',
'very-long-class', 'another-class'].join(' ')) or build it across multiple
concatenated strings and use that variable in the JSX to keep lines under the
limit; update references to setupSF and the input element accordingly.
- Around line 80-107: The current useEffect returns early if an existing Zoho
script is found which prevents calling setupSF for this component instance;
update the useEffect so that if an existing script tag is present you still
check for the global setupSF and call it with FORM_IX, "ZCFORMVIEW", false,
"light" (same call made in s.onload), and keep the s.onload logic unchanged so
new script loads still call setupSF; reference the existing variables and
functions: useEffect, existing, s.onload, setupSF, and FORM_IX to locate and
implement the change.
In `@docs/homepage/components/ui/button.tsx`:
- Line 31: Remove the unused variant key 'tab' (the line "tab: \"\"") from the
button variants definition (e.g., the object that defines buttonVariants/button
styles) since it provides no styling and is not referenced elsewhere;
alternatively, if you intend to keep it for future use, replace it with a short
explanatory comment next to the 'tab' key describing its intended purpose so
reviewers know it's intentional.
In `@taskfiles/docs.yaml`:
- Around line 84-92: The rsync invocation "rsync -a --exclude='/index.html'
--include='*/index.html' \"{{.G_DEV_DOCS_OUTPUT_DIR}}/\" \"{{.OUTPUT_DIR}}/\""
contains a redundant --include='*/index.html' that matches nothing given the
anchored --exclude and default rsync behavior; either remove the --include flag
entirely or replace it with a brief comment explaining the intended behavior if
you actually meant to selectively include nested index.html files, and if you
need more complex inclusion/exclusion (e.g., 404.html) consolidate into a single
rsync using a --filter file or a combined set of --exclude/--include rules so
the intent is explicit and maintainable.
---
Duplicate comments:
In `@docs/homepage/app/shared/ThemeProvider.tsx`:
- Around line 26-37: getInitialTheme currently reads localStorage unguarded then
repeats the same read inside a try/catch, which can throw during initial render;
remove the first unguarded call and only perform the guarded read inside
getInitialTheme (the try/catch block), returning the stored value if it's "dark"
or "light" and otherwise falling back to the default — update the function in
ThemeProvider.tsx (getInitialTheme) to only use the guarded
localStorage.getItem("ui-theme") and keep the catch that ignores storage access
failures.
In `@docs/homepage/components/sections/Navbar.tsx`:
- Around line 162-195: The onBlur handler (handleProjectDocsLeave) currently
closes the dropdown whenever any child loses focus, collapsing the menu during
intra-container focus moves; update handleProjectDocsLeave to inspect the blur
event's relatedTarget (or use
projectDocsContainerRef.current.contains(relatedTarget)) and bail out (do not
start the close timer) if relatedTarget is still inside the projectDocs
container, otherwise proceed with the existing timeout/close logic; ensure any
timeout clear logic (as used in onFocus) is preserved and referenced in
handleProjectDocsLeave so focus moves between the Button and dropdown links
don't close the menu.
In `@docs/homepage/eslint.config.mjs`:
- Around line 3-16: The commented-out StylisticConfigArray import and spread are
left disabled without tracking; either re-enable them (uncomment the import
"StylisticConfigArray" and the spread "...StylisticConfigArray" in EslintConfig)
or add an explicit TODO comment with an issue/reference immediately above both
the import line and the array spread mentioning the reason and ticket (e.g.,
TODO(`#1234`): temporary disablement) so rg searches for 'StylisticConfigArray' or
'TODO(#' will find the tracking note.
In `@docs/homepage/next-sitemap.config.mjs`:
- Line 1: The top-of-file JSDoc currently has the `@type` tag wrapped in backticks
(the line that reads `/** `@type` {import('next-sitemap').IConfig} */`), which
prevents editors/type-checkers from recognizing it; fix this by removing the
surrounding backticks so the JSDoc contains a proper `@type` tag and keep the
import('next-sitemap').IConfig reference unchanged.
In `@docs/homepage/package.json`:
- Line 11: The "start" script in package.json currently uses an invalid Next.js
CLI command ("next serve out"); update the package.json "start" script to use a
static file server since next.config.ts uses output: "export". Replace the
script value with a valid static-server invocation (for example "npx serve -s
out" or "npx http-server out") OR add "serve" as a devDependency and set the
script to "serve -s out"; ensure the script name "start" is updated and tested
to serve the exported "out" directory.
In `@taskfiles/docs.yaml`:
- Around line 74-81: Add strict shell error handling by inserting set -euo
pipefail at the start of the shell command block that begins with rm -rf
docs/homepage/out docs/homepage/.next and ends with rsync -a docs/homepage/out/
"{{.G_HOMEPAGE_OUTPUT_DIR}}/"; also add the same set -euo pipefail at the start
of the separate merge step block (the block that creates the target dir and runs
rsync to merge outputs) so any failing command (e.g., npm install or npm run
build) will abort the step immediately.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 07ef0f64-6f67-4f0b-b5db-6e717e6791ad
📒 Files selected for processing (18)
docs/homepage/.prettierrc.yamldocs/homepage/app/assets/scss/styles.scssdocs/homepage/app/content.tsdocs/homepage/app/layout.tsxdocs/homepage/app/page.tsxdocs/homepage/app/shared/ThemeProvider.tsxdocs/homepage/components/icons/GithubIcon.tsxdocs/homepage/components/sections/Footer.tsxdocs/homepage/components/sections/Navbar.tsxdocs/homepage/components/sections/ZohoSignupForm.tsxdocs/homepage/components/ui/button.tsxdocs/homepage/components/ui/sheet.tsxdocs/homepage/eslint.config.mjsdocs/homepage/next-sitemap.config.mjsdocs/homepage/next.config.tsdocs/homepage/package.jsondocs/homepage/tailwind.config.cjstaskfiles/docs.yaml
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
taskfiles/docs.yaml (1)
73-81:⚠️ Potential issue | 🟠 MajorBuild the homepage in
{{.G_BUILD_DIR}}and fail fast.This still creates Next.js artefacts under
docs/homepage, which can be picked up by thedocs/**/*source glob on Line 56 and dirty local checkouts. The block also still falls through after failed npm commands unless the finalrsynchappens to fail. Build from a temporary workspace under{{.G_BUILD_DIR}}and enable fail-fast shell behaviour.🛡️ Proposed fix
# Build the homepage - |- - rm -rf docs/homepage/out docs/homepage/.next - cd docs/homepage || exit 1 - npm install --no-audit --no-fund - npm run build - cd - + set -eu + homepage_build_dir="{{.G_BUILD_DIR}}/homepage-src" + rm -rf "${homepage_build_dir}" "{{.G_HOMEPAGE_OUTPUT_DIR}}" + mkdir -p "${homepage_build_dir}" + rsync -a \ + --exclude='node_modules' \ + --exclude='.next' \ + --exclude='out' \ + docs/homepage/ "${homepage_build_dir}/" + ( + cd "${homepage_build_dir}" + npm install --no-audit --no-fund + npm run build + ) mkdir -p "{{.G_HOMEPAGE_OUTPUT_DIR}}" - rsync -a docs/homepage/out/ "{{.G_HOMEPAGE_OUTPUT_DIR}}/" + rsync -a "${homepage_build_dir}/out/" "{{.G_HOMEPAGE_OUTPUT_DIR}}/"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@taskfiles/docs.yaml` around lines 73 - 81, Replace the in-place build with a fail-fast build in a temp workspace under {{.G_BUILD_DIR}}: at the top of the block enable strict shell modes (set -euo pipefail) then create a clean build dir like "{{.G_BUILD_DIR}}/homepage_build", copy or rsync the source from docs/homepage into that workspace, run npm install (or npm ci) and npm run build inside the workspace (instead of cd docs/homepage), and finally rsync the workspace's out/ to "{{.G_HOMEPAGE_OUTPUT_DIR}}/"; remove the original rm -rf of docs/homepage/out and docs/homepage/.next and ensure all commands run in the workspace so failures abort immediately and no Next.js artefacts are left under docs/homepage.docs/homepage/components/sections/ZohoSignupForm.tsx (1)
238-244:⚠️ Potential issue | 🟡 MinorBreak long
classNameto respectprintWidthconstraint.Line 238 exceeds the project's
printWidthsetting (100 characters in.prettierrc.yaml). The line is 173 characters and must be split across multiple lines or extracted to a named constant.♻️ Proposed fix
+const SUBMIT_BUTTON_CLASS = [ + "w-100", + "bg-[var(--brilliant-azure-550)]", + "hover:bg-[var(--brilliant-azure-700)]", + "text-lg text-white px-4 py-2", + "rounded-md cursor-pointer", +].join(" "); ... - className={"w-100 bg-[var(--brilliant-azure-550)] hover:bg-[var(--brilliant-azure-700)] text-lg text-white px-4 py-2 rounded-md cursor-pointer"} + className={SUBMIT_BUTTON_CLASS}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/components/sections/ZohoSignupForm.tsx` around lines 238 - 244, The long className string in the input element inside ZohoSignupForm (the input with id "zcWebOptin", type "submit" and value conditional on submitting) exceeds the project's printWidth; extract the class list into a named constant (e.g., BUTTON_CLASS or buttonClass) or split the string across multiple concatenated/template literal lines so the JSX line no longer exceeds 100 chars, then replace the inline className with that constant (preserving the exact class names and other props like disabled and value).docs/homepage/app/content.ts (1)
16-153:⚠️ Potential issue | 🟡 MinorNormalize internal link href styles to be consistent.
Internal docs links use mixed conventions within the same data structure:
- Trailing slash: lines 20, 43, 74, 86, 134
.htmlextension: lines 26, 32, 56, 67, 80, 92, 98, 109, 140Choose one style and apply it consistently throughout the file to reduce maintenance friction and avoid potential routing issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/app/content.ts` around lines 16 - 153, Several internal hrefs in the items arrays (e.g., the entries under "Deploy CLP", "Log Input", "Analyze & View", "Resources", "References") mix trailing-slash routes and .html file links; normalize them to a single style (use trailing-slash routes for all internal docs) by replacing occurrences of "...-deployment.html", "...-using-log-ingestor.html", "...-using-presto.html", "...-using-the-api-server.html", "...reference-*.html" etc. with their trailing-slash equivalents, leaving external links (isExternal: true) and static asset imports like prestoSrc/mcpSrc unchanged; update every href string in the items arrays to the chosen canonical form so routing is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/homepage/app/page.tsx`:
- Around line 145-158: The flicker occurs because prestoSrc/mcpSrc are
initialized to light variants and only updated in the useEffect after mount;
instead compute their initial values synchronously from the current theme
returned by useTheme and initialize the useState calls accordingly (in the Home
component), then remove the useEffect that calls setPrestoSrc/setMcpSrc; keep
references to useTheme, prestoSrc, mcpSrc, setPrestoSrc, setMcpSrc and the Home
component to locate and change the initialization and delete the effect.
- Around line 164-260: The horizontal scrollbar is caused by Bootstrap .row
negative gutters in the hero and categories rows; update the two offending divs
that use className "row align-items-center justify-content-center
homepage-hero-row" and "row align-items-center justify-content-center" so they
no longer emit negative side margins — either replace each "row ..." with
"d-flex flex-wrap align-items-center justify-content-center" or add the
gutter-neutralizing utilities (e.g. append "g-0 mx-0") to those row classNames;
ensure the layout still renders by leaving the inner Category usage
(categories.map → Category) and Divider in place, or alternatively add
overflow-hidden to the outer container div to contain any remaining overflow.
In `@docs/homepage/components/sections/Navbar.tsx`:
- Around line 104-122: The current catch in the useEffect swallows all errors;
change it to only ignore AbortError and surface others by checking the thrown
error's name (e.g., err.name === 'AbortError') in the catch handler used with
fetch in the useEffect that creates controller and calls setGithubStars; if it's
an AbortError do nothing, otherwise log the error (console.error or your logger)
with a clear message like "Failed to fetch GitHub stars" so real network/parse
errors are not silently ignored.
- Around line 162-196: The onFocus and onClick handlers duplicate logic already
present in handleProjectDocsEnter/handleProjectDocsKeyDown; extract a single
toggleProjectDocs (or openProjectDocs/closeProjectDocs) helper that centralizes
clearing projectDocsTimeoutRef, calling setProjectDocsOpen(true/false), and
invoking focusFirstProjectDocsItem when opening, then replace the inline onFocus
and onClick handlers to call that shared function and update
handleProjectDocsKeyDown to call the same toggle/open helper for the Enter/Space
branch; remove the duplicated logic from the inline handlers so all open/toggle
behaviour is driven by handleProjectDocsEnter/handleProjectDocsKeyDown and the
new toggleProjectDocs helper.
In `@docs/homepage/components/sections/ZohoSignupForm.tsx`:
- Around line 132-143: The onLoad handler in ZohoSignupForm currently treats any
iframe load during submitting as success (it calls setSuccessVisible, clears
inputs, and setSubmitting(false)), but onLoad fires on initial mount and for
error pages too; change this by removing success logic from the iframe onLoad
and instead rely on the postMessage handler to confirm success: implement/verify
a window message event listener that checks event.origin against the expected
Zoho origin and validates the message payload, only then call
setSuccessVisible(true), clear inputs (setEmail, setFirstName) and
setSubmitting(false); if you can't fully implement postMessage now, add a clear
comment in the onLoad block documenting this limitation and debounce/ignore
onLoad-based success to avoid clearing inputs on server-side errors.
- Line 156: Replace the raw JSX text "Join YScope's Newsletter" with the
consistent {"Join YScope's Newsletter"} string-in-JSX form used elsewhere in the
ZohoSignupForm component; update the heading/label JSX (in the ZohoSignupForm
component) to use {"Join YScope's Newsletter"} so it matches the other
user-facing strings like placeholders and titles.
- Around line 145-157: ZohoSignupForm.tsx uses a hardcoded "newsletter-card"
wrapper which conflicts with the parent CardContent padding and forces
text-align; change the component to stop forcing that class by merging or
replacing it with the incoming props.className (e.g., use
className={`newsletter-card ${className ?? ""}`} or remove the inner wrapper
entirely) so the parent ContactSection/CardContent can control spacing and
alignment; update references to the div with id "SIGNUP_HEADING" and the outer
wrapper in ZohoSignupForm to use the merged or removed class approach and ensure
consumers can supply their own className.
- Around line 87-104: The effect that injects the Zoho script (useEffect) can
call callSetup after the component unmounts and leaks the <script> element;
update the effect to keep a mounted flag and return a cleanup that sets
mounted=false, nulls s.onload, and removes the appended script if it was added
(only if it still exists and was not previously present), so callSetup is only
executed while mounted and the script tag is not leaked; reference symbols:
useEffect, callSetup, s (the created script), and the existing query selector
for "ma.zoho.com/js/optin.min.js".
- Around line 107-122: The origin check in useEffect's handleMessage is unsafe
(ev.origin.includes("zoho.com")) and also wrong for Zoho weboptin (the
iframe/postMessage comes from maillist-manage.com), so replace the substring
check with an exact whitelist check: determine the real postMessage origin(s)
Zoho uses for weboptin (e.g., publ.maillist-manage.com or other
maillist-manage.com hostnames) and validate ev.origin against a constant
allowedOrigins array (or single allowedOrigin) using a strict equality or
includes on that array; update handleMessage (and the return logic) to only
proceed to call setSuccessVisible(true) when ev.origin exactly matches an entry
in allowedOrigins, and keep the rest of the event teardown
(window.addEventListener/removeEventListener) as-is.
---
Duplicate comments:
In `@docs/homepage/app/content.ts`:
- Around line 16-153: Several internal hrefs in the items arrays (e.g., the
entries under "Deploy CLP", "Log Input", "Analyze & View", "Resources",
"References") mix trailing-slash routes and .html file links; normalize them to
a single style (use trailing-slash routes for all internal docs) by replacing
occurrences of "...-deployment.html", "...-using-log-ingestor.html",
"...-using-presto.html", "...-using-the-api-server.html", "...reference-*.html"
etc. with their trailing-slash equivalents, leaving external links (isExternal:
true) and static asset imports like prestoSrc/mcpSrc unchanged; update every
href string in the items arrays to the chosen canonical form so routing is
consistent.
In `@docs/homepage/components/sections/ZohoSignupForm.tsx`:
- Around line 238-244: The long className string in the input element inside
ZohoSignupForm (the input with id "zcWebOptin", type "submit" and value
conditional on submitting) exceeds the project's printWidth; extract the class
list into a named constant (e.g., BUTTON_CLASS or buttonClass) or split the
string across multiple concatenated/template literal lines so the JSX line no
longer exceeds 100 chars, then replace the inline className with that constant
(preserving the exact class names and other props like disabled and value).
In `@taskfiles/docs.yaml`:
- Around line 73-81: Replace the in-place build with a fail-fast build in a temp
workspace under {{.G_BUILD_DIR}}: at the top of the block enable strict shell
modes (set -euo pipefail) then create a clean build dir like
"{{.G_BUILD_DIR}}/homepage_build", copy or rsync the source from docs/homepage
into that workspace, run npm install (or npm ci) and npm run build inside the
workspace (instead of cd docs/homepage), and finally rsync the workspace's out/
to "{{.G_HOMEPAGE_OUTPUT_DIR}}/"; remove the original rm -rf of
docs/homepage/out and docs/homepage/.next and ensure all commands run in the
workspace so failures abort immediately and no Next.js artefacts are left under
docs/homepage.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8860030e-6afd-4dc8-b6b0-7ee3927de4ac
📒 Files selected for processing (8)
docs/homepage/app/content.tsdocs/homepage/app/layout.tsxdocs/homepage/app/page.tsxdocs/homepage/components/icons/GithubIcon.tsxdocs/homepage/components/sections/Navbar.tsxdocs/homepage/components/sections/ZohoSignupForm.tsxdocs/homepage/components/ui/button.tsxtaskfiles/docs.yaml
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docs/homepage/app/page.tsx (1)
29-33:⚠️ Potential issue | 🟡 MinorNeutralize the remaining Bootstrap row gutters.
The outer hero/category rows now use
g-0 mx-0, butDividerandCategorystill emit default.rowside gutters, which can reintroduce the reported horizontal scrollbar.Proposed fix
- <div className={"row my-3"}> + <div className={"row g-0 mx-0 my-3"}> ... - <div className={"row align-items-center justify-content-center"}> + <div className={"row g-0 mx-0 align-items-center justify-content-center"}>Based on learnings, the Bootstrap/Tailwind container padding override is intentional and should be preserved.
Also applies to: 121-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/homepage/app/page.tsx` around lines 29 - 33, The Bootstrap row gutters still appear in the Divider/Category blocks — change the outer row DIVs (currently using className "row my-3") to neutralize gutters by adding g-0 and mx-0 (e.g. className "row g-0 mx-0 my-3") or update the Divider and Category components to render their root row with "row g-0 mx-0" so side gutters are removed and the horizontal scrollbar cannot reappear; ensure any sibling col containers (e.g., "col-lg-10 mx-auto divider-whitespace") remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/homepage/components/sections/Navbar.tsx`:
- Around line 131-133: The hover-open handler handleProjectDocsEnter currently
opens the dropdown with the default focus behavior, so change it to open without
moving focus by calling toggleProjectDocs(true, { focusOnOpen: false }) (or pass
the equivalent option your toggle accepts) and apply the same change to the
other hover-open handlers mentioned (the same pattern used around the other
dropdown hover handlers).
- Around line 316-335: The GitHub icon-only link and other icon-only controls
(e.g., the Button wrapping GithubIcon, the desktop theme toggle, and the mobile
menu trigger) lack accessible names for assistive tech; update each control to
provide an explicit accessible name by adding an aria-label or a visually-hidden
text node and mark decorative icons aria-hidden="true". Specifically, for the
Github link/Button and GithubIcon, add aria-hidden=true to GithubIcon and add
aria-label on the anchor/Button (or include a <span className="sr-only">Open CLP
on GitHub — X stars</span> that uses formatNumber(githubStars)); apply the same
pattern to the theme toggle and mobile menu trigger components so each icon-only
control exposes a clear label to screen readers.
- Around line 88-91: The nullability check in focusRelativeProjectDocsItem uses
a negation (!container); replace it with an explicit null/undefined comparison
on projectDocsContainerRef.current — e.g., change the guard to check container
== null (or container === null / container === undefined consistently with
project conventions) and return if true so the subsequent use of
container.querySelectorAll is safe; locate this in the
focusRelativeProjectDocsItem function where projectDocsContainerRef.current is
assigned to container and update that if-check.
- Around line 272-284: The link onKeyDown handler only handles ArrowUp/ArrowDown
and needs Escape support so keyboard users can close the dropdown; update the
onKeyDown in the anchor (the same handler that calls
focusRelativeProjectDocsItem) to also detect "Escape" (or "Esc"), call the
component's menu-close logic (e.g., closeProjectDocsMenu,
setProjectDocsOpen(false), or the existing close/toggle handler used elsewhere),
preventDefault()/stopPropagation(), and move focus back to the menu
toggle/button that opened the menu so focus is restored.
---
Duplicate comments:
In `@docs/homepage/app/page.tsx`:
- Around line 29-33: The Bootstrap row gutters still appear in the
Divider/Category blocks — change the outer row DIVs (currently using className
"row my-3") to neutralize gutters by adding g-0 and mx-0 (e.g. className "row
g-0 mx-0 my-3") or update the Divider and Category components to render their
root row with "row g-0 mx-0" so side gutters are removed and the horizontal
scrollbar cannot reappear; ensure any sibling col containers (e.g., "col-lg-10
mx-auto divider-whitespace") remain unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e745979a-f71c-41ba-a23c-4573b56c9ac9
📒 Files selected for processing (5)
docs/dev-docs/_static/clp-versions.jsondocs/dev-docs/dev-guide/contrib-guides-cpp.mddocs/homepage/app/page.tsxdocs/homepage/components/sections/Navbar.tsxdocs/homepage/components/sections/ZohoSignupForm.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
2 participants
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.
Description
This PR refactors the main YScope docs pages to have a custom typescript home page. We refocussed the main page to a simplified grid of categorized icons which link to key docs pages, resources, guides, references, etc. We focus on CLP and how out other projects interact with CLP. It's designed to be visual cohesive with the yscope.com main page.
Started as PR #52, but to started new PR to properly merge after PR #54 was merged into main.
Checklist
breaking change.
Validation performed
Built and served webpage locally
task docs:buildtask docs:serveSummary by CodeRabbit
New Features
Documentation