Repository navigation
Frontend/v1 - #34
Frontend/v1#34
Conversation
docs: Add MDX contribution guide for content creators
Adds the React MDX cheatsheet to the reference library. --------- Co-authored-by: Yash Vardhan <vardhan.yash3105@gmail.com> Co-authored-by: RamKumar-Muthuraj <muthuraj.ramkumar@gwcdata.ai>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR replaces the reference landing page with a responsive documentation catalog. It adds topic search, section navigation, heading metadata, adjacent-topic controls, updated MDX styling, custom code themes, CI checks, and CodeRabbit configuration. ChangesDocumentation catalog
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Reader
participant TopicPage
participant MDX
participant TopicLayout
Reader->>TopicPage: Open a topic route
TopicPage->>MDX: Load topic content and headings
MDX-->>TopicPage: Return article data and heading metadata
TopicPage->>TopicLayout: Provide topics, headings, and adjacent topics
TopicLayout->>Reader: Render documentation navigation and article content
Reader->>TopicLayout: Search topics or select a section
TopicLayout->>Reader: Navigate to a topic or scroll to an anchor
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (6)
frontend/app/[topic]/TopicLayout.tsx (1)
243-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
scrollToSectionignoresprefers-reduced-motionand does not move focus.Smooth scrolling triggers motion sickness for some readers. Keyboard users also lose their position, because the focus stays on the outline button after the scroll. Read the media query and move focus to the target heading.
♿ Proposed fix
const scrollToSection = (id: string) => { if (!id) return; const target = document.getElementById(id); if (!target) return; + const prefersReducedMotion = window.matchMedia("(prefers-reduced-motion: reduce)").matches; const headerOffset = 88; const top = target.getBoundingClientRect().top + window.scrollY - headerOffset; - window.scrollTo({ top, behavior: "smooth" }); + window.scrollTo({ top, behavior: prefersReducedMotion ? "auto" : "smooth" }); + + target.setAttribute("tabindex", "-1"); + target.focus({ preventScroll: true }); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/app/`[topic]/TopicLayout.tsx around lines 243 - 252, Update scrollToSection to respect the prefers-reduced-motion media query by using non-animated scrolling when reduced motion is requested, and move keyboard focus to the target heading after locating it. Ensure the heading can receive focus without changing its visible styling, while preserving the existing offset and smooth-scroll behavior for other users.frontend/app/components/MdxTypography.tsx (2)
41-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
MdxH3drops callerclassNameand cannot merge styles.
MdxH3spreadsrestafter theclassNameattribute. If MDX suppliesclassName, the spread replaces the component classes instead of merging them.MdxH2already handles this correctly. Align the two components.♻️ Proposed fix
export const MdxH3 = (props: any) => { const { darkMode } = useTheme(); - const { children, ...rest } = props; + const { className = "", children, ...rest } = props; const headingId = props.id || getHeadingId(children); return ( <h3 id={headingId} - className={`pt-8 text-xl font-semibold tracking-[-0.04em] ${ - darkMode ? "text-zinc-100" : "text-zinc-900" - }`} + className={`pt-8 text-xl font-semibold tracking-[-0.04em] ${ + darkMode ? "text-zinc-100" : "text-zinc-900" + } ${className}`.trim()} + style={{ scrollMarginTop: "1rem", ...props.style }} {...rest} >Note the added
scrollMarginTop. Without it,scrollToSectioninTopicLayout.tsxlands H3 anchors under the sticky header only because of its manual 88px offset; the two mechanisms should agree.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/app/components/MdxTypography.tsx` around lines 41 - 57, Update MdxH3 to destructure and merge the caller-provided className with its default heading classes, matching MdxH2 while preserving other props and the explicit id. Add the same scrollMarginTop behavior used by MdxH2 so H3 anchors align with scrollToSection’s sticky-header offset.
63-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the conflicting
text-current/80utility.
text-current/80andtext-zinc-300set the same CSS property. Tailwind emits utilities in its own sort order, not in class-attribute order, so the winner is not guaranteed by the template string. Drop thetext-currentvariants and encode the intended opacity in the zinc shade. Line 30 inMdxH2has the same conflict betweentext-currentandtext-zinc-100/text-zinc-900.🎨 Proposed fix
- className={`mb-6 max-w-3xl text-[1.03rem] leading-8 text-current/80 ${ + className={`mb-6 max-w-3xl text-[1.03rem] leading-8 ${ darkMode ? "text-zinc-300" : "text-zinc-700" }`}- className={`mb-6 list-disc list-inside space-y-2 text-[1.02rem] leading-7 text-current/80 ${ + className={`mb-6 list-disc list-inside space-y-2 text-[1.02rem] leading-7 ${ darkMode ? "text-zinc-300" : "text-zinc-700" }`}As per path instructions: "Ensure React components are using modern hooks, avoid unnecessary re-renders, and use TailwindCSS properly."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/app/components/MdxTypography.tsx` around lines 63 - 76, Remove the conflicting text-current/80 utility from the className strings in MdxUl and the nearby typography components, including MdxH2’s text-current usage. Preserve the darkMode-specific zinc colors while encoding the intended opacity directly in the selected zinc shade, without changing other spacing or typography classes.Source: Path instructions
frontend/app/page.tsx (1)
22-123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStore icon components in the catalog, not rendered elements.
REFERENCE_DATAholds pre-rendered JSX (icon: <Code2 size={22} className='text-zinc-200' />). That single decision forcescloneElementdownstream and creates two sources of truth for icon colour. Holding the component reference removes both problems and gives the catalog a cleanLucideIcontype.
frontend/app/page.tsx#L22-L123: change eachiconfield to the component reference (icon: Code2), type the array asReferenceSection[], drop the unusedsizefield, and move the catalog out of the route module into a plain data module such asfrontend/src/data/reference.ts.frontend/app/components/DirectoryList.tsx#L51-L57: delete thecloneElementcalls and thecloneElementimport, then render the icon directly as<Icon size={22} className={...} />with theme-aware Tailwind classes instead of an inlinestylecolour.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/app/page.tsx` around lines 22 - 123, The reference catalog in frontend/app/page.tsx lines 22-123 should store Lucide component references rather than rendered JSX: move it to a plain data module such as frontend/src/data/reference.ts, type it as ReferenceSection[], use values like Code2, and remove every size field. Update frontend/app/components/DirectoryList.tsx lines 51-57 to remove cloneElement and its import, rendering the stored Icon directly with size 22 and theme-aware Tailwind color classes instead of inline color styles.Source: Path instructions
frontend/app/components/DirectoryList.tsx (1)
60-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the unused utilities and add a focus ring.
Three notes on the card class list:
groupis applied, but nogroup-hover:orgroup-focus:utility appears in the card body. It has no effect.relative overflow-hiddenhas no absolutely positioned or overflowing child. It also has no effect.- The card is the primary interactive control in the directory, and it has no
focus-visible:style. Keyboard users get only the user-agent outline against a custom rounded border.Add an explicit focus ring so keyboard navigation stays visible in both themes.
♻️ Proposed fix
<Link key={item.name} href={item.href} - className={`group relative overflow-hidden rounded-3xl border p-5 transition-colors ${darkMode ? "border-[`#1f1f1f`] bg-[`#0d0d0d`] hover:bg-[`#121212`]" : "border-[`#e1dfd8`] bg-[`#f9f7f4`] hover:bg-[`#f3f0ea`]"}`} + className={`rounded-3xl border p-5 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#C699FF`] focus-visible:ring-offset-2 ${darkMode ? "border-[`#1f1f1f`] bg-[`#0d0d0d`] hover:bg-[`#121212`] focus-visible:ring-offset-[`#050505`]" : "border-[`#e1dfd8`] bg-[`#f9f7f4`] hover:bg-[`#f3f0ea`] focus-visible:ring-offset-[`#f3f0ea`]"}`} >As per path instructions: "Ensure React components are ... use TailwindCSS properly."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/app/components/DirectoryList.tsx` around lines 60 - 64, Update the Link className in DirectoryList to remove the unused group, relative, and overflow-hidden utilities, and add an explicit focus-visible ring style that remains visible in both dark and light themes. Preserve the existing theme-specific border, background, hover, and rounded-card styling.Source: Path instructions
frontend/app/components/ClientCodeBlock.tsx (1)
42-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse Prism token names in both theme objects.
react-syntax-highlightermatches keys directly against Prism class names. Remove the unused camelCase keysclassName,attrName,attrValue,maybeClassName,selectorClass,selectorId, anddeletedCode. Keepclass-name,attr-name, andattr-value.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/app/components/ClientCodeBlock.tsx` around lines 42 - 95, Update the theme object in ClientCodeBlock to use only Prism token names: remove the unused camelCase keys className, attrName, attrValue, maybeClassName, selectorClass, selectorId, and deletedCode, while retaining the hyphenated class-name, attr-name, and attr-value entries.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 7-9: Harden the quality-checks workflow by declaring explicit
read-only GITHUB_TOKEN permissions at the workflow or job level, and configure
the checkout step to avoid persisting credentials. Update the
jobs.quality-checks configuration and its actions/checkout invocation while
preserving the existing CI steps.
- Around line 20-25: Update the node-version setting in the “Setup Node.js”
workflow step from 20 to 24, while preserving the existing npm cache and
cache-dependency-path configuration.
In `@frontend/app/`[topic]/page.tsx:
- Around line 32-38: Sort the array returned by getAllTopics before calculating
currentIndex, prevTopic, and nextTopic, using each topic’s frontmatter order
value when available and falling back to title for deterministic ordering.
Ensure the sorted order is also used by the sidebar so navigation and display
remain consistent across environments.
In `@frontend/app/`[topic]/TopicLayout.tsx:
- Line 20: Remove the unused frontmatter prop and its type from the layout
component, then remove the corresponding frontmatter={...} arguments from both
call sites in page.tsx; also eliminate the any type reported in the component
props.
- Line 343: Update the sidebar className expressions in TopicLayout to apply the
theme.border class alongside border-r and border-l, matching the header’s
dark/light palette. Convert the line 343 className from an unnecessary template
literal to a regular string or an expression that includes theme.border, and
make the corresponding change for the sidebar using border-l.
- Around line 163-171: Update the expanded-outline effect in TopicLayout so the
first group is expanded only during initial setup, not whenever outlineGroups
changes due to normalizedQuery updates. Track whether default initialization has
already occurred, preserve the user’s existing expanded/collapsed state on
subsequent updates, and continue retaining prior state for other groups.
- Around line 173-190: Update the sectionGroups useMemo to iterate over
activeHeadings instead of the raw headings prop, while preserving the existing
grouping logic and dependency tracking so section navigation uses the same
fallback heading source as outlineGroups.
- Around line 115-147: Throttle updateActiveOutline with a requestAnimationFrame
guard so rapid scroll events schedule at most one layout read per animation
frame. Keep the existing heading-order calculation and activeOutlineId update
behavior, cancel any pending frame during effect cleanup, and retain the resize
handling without introducing unnecessary renders.
- Around line 268-330: Update the search state and input in TopicLayout to
implement an accessible combobox: track the active result, add
combobox/listbox/option roles with aria-expanded, aria-controls, and
aria-activedescendant, and support ArrowUp/ArrowDown plus Enter activation for
topic navigation or section scrolling. Handle Escape by clearing the query and
closing the popover, and replace the hardcoded ⌘K hint with a post-mount
platform-aware Ctrl/⌘ label consistent with the existing shortcut handler.
In `@frontend/app/components/ClientCodeBlock.tsx`:
- Around line 31-161: Move the module-constant style objects
pitchBlackModernStyle and softLightModernStyle outside the ClientCodeBlock
component, preserving their definitions and keys unchanged. Keep the existing
theme selection and SyntaxHighlighter style usage unchanged so the component
reuses stable object references across renders.
In `@frontend/app/components/MdxTypography.tsx`:
- Line 22: Replace the any-typed props in MdxH2, MdxH3, MdxP, and MdxUl with
appropriate built-in JSX intrinsic prop types, using ComponentPropsWithoutRef so
React 19 ref handling remains correct. Preserve each component’s existing
rendering and behavior while eliminating all explicit any usage.
- Around line 6-20: Unify heading slug generation by adding a framework-agnostic
shared slugify helper and using it from both sites: in
frontend/app/components/MdxTypography.tsx lines 6-20, recursively collect text
from nested React element children before delegating to slugify; in
frontend/src/lib/mdx.ts lines 15-39, export or import the same helper and
replace the inline normalization chain with it. Ensure headings containing
inline code and bold text resolve to the same ids from the outline and rendered
anchors.
In `@frontend/app/components/SkeletonTopic.tsx`:
- Around line 3-40: The hardcoded dark-mode colors in SkeletonTopic and the page
label bypass the shared theme mechanism. In
frontend/app/components/SkeletonTopic.tsx lines 3-40, replace the specified
background, border, and text colors with theme-aware dark/light variants or the
existing useTheme map; in frontend/app/page.tsx lines 170-173, replace
text-zinc-300 on the “Developer reference library” label with ${theme.muted}.
In `@frontend/app/globals.css`:
- Line 5: Update the --font-sans declaration to use lowercase font-family
keyword values for Inter and Arial, while preserving the existing stack order
and other fonts so it passes Stylelint's value-keyword-case check.
- Around line 26-27: Update the code-block font declaration in the global CSS to
use the defined --font-mono variable instead of the removed
--font-jetbrains-mono variable, and revise the adjacent comment to accurately
describe the applied monospace font.
In `@frontend/app/page.tsx`:
- Around line 185-194: Add an explicit accessible name to the search input in
the existing label/input block by adding an aria-label such as “Search topics,”
or provide equivalent visually hidden label text. Preserve the current
searchQuery value and onChange behavior.
In `@frontend/src/lib/mdx.ts`:
- Around line 15-39: Update extractHeadings to track fenced code-block state and
ignore fence delimiter lines and all content while inside ``` or ~~~ fences.
Rename the generated heading identifier to a base ID, maintain a seen-count map,
and append an incrementing suffix for repeated IDs while preserving the first
heading’s unsuffixed ID.
---
Nitpick comments:
In `@frontend/app/`[topic]/TopicLayout.tsx:
- Around line 243-252: Update scrollToSection to respect the
prefers-reduced-motion media query by using non-animated scrolling when reduced
motion is requested, and move keyboard focus to the target heading after
locating it. Ensure the heading can receive focus without changing its visible
styling, while preserving the existing offset and smooth-scroll behavior for
other users.
In `@frontend/app/components/ClientCodeBlock.tsx`:
- Around line 42-95: Update the theme object in ClientCodeBlock to use only
Prism token names: remove the unused camelCase keys className, attrName,
attrValue, maybeClassName, selectorClass, selectorId, and deletedCode, while
retaining the hyphenated class-name, attr-name, and attr-value entries.
In `@frontend/app/components/DirectoryList.tsx`:
- Around line 60-64: Update the Link className in DirectoryList to remove the
unused group, relative, and overflow-hidden utilities, and add an explicit
focus-visible ring style that remains visible in both dark and light themes.
Preserve the existing theme-specific border, background, hover, and rounded-card
styling.
In `@frontend/app/components/MdxTypography.tsx`:
- Around line 41-57: Update MdxH3 to destructure and merge the caller-provided
className with its default heading classes, matching MdxH2 while preserving
other props and the explicit id. Add the same scrollMarginTop behavior used by
MdxH2 so H3 anchors align with scrollToSection’s sticky-header offset.
- Around line 63-76: Remove the conflicting text-current/80 utility from the
className strings in MdxUl and the nearby typography components, including
MdxH2’s text-current usage. Preserve the darkMode-specific zinc colors while
encoding the intended opacity directly in the selected zinc shade, without
changing other spacing or typography classes.
In `@frontend/app/page.tsx`:
- Around line 22-123: The reference catalog in frontend/app/page.tsx lines
22-123 should store Lucide component references rather than rendered JSX: move
it to a plain data module such as frontend/src/data/reference.ts, type it as
ReferenceSection[], use values like Code2, and remove every size field. Update
frontend/app/components/DirectoryList.tsx lines 51-57 to remove cloneElement and
its import, rendering the stored Icon directly with size 22 and theme-aware
Tailwind color classes instead of inline color styles.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 713d6190-66d6-463f-b3c4-da4ca8133803
📒 Files selected for processing (13)
.coderabbit.yaml.github/workflows/ci.ymlfrontend/README.mdfrontend/app/[topic]/TopicLayout.tsxfrontend/app/[topic]/page.tsxfrontend/app/components/ClientCodeBlock.tsxfrontend/app/components/DirectoryList.tsxfrontend/app/components/MdxTypography.tsxfrontend/app/components/SkeletonTopic.tsxfrontend/app/globals.cssfrontend/app/layout.tsxfrontend/app/page.tsxfrontend/src/lib/mdx.ts
💤 Files with no reviewable changes (1)
- frontend/README.md
| jobs: | ||
| quality-checks: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict the workflow token and remove checkout credentials.
npm ci can run lifecycle scripts from the checked-out PR. If repository defaults grant write access, the current workflow exposes a broader token than this read-only CI job requires. Set explicit read-only permissions and disable persisted checkout credentials.
Proposed workflow hardening
on:
pull_request:
branches: [ main ]
+permissions:
+ contents: read
+
jobs:
quality-checks:
@@
- name: Checkout repository
uses: actions/checkout@v4
+ with:
+ persist-credentials: falseAlso applies to: 17-18
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 7 - 9, Harden the quality-checks
workflow by declaring explicit read-only GITHUB_TOKEN permissions at the
workflow or job level, and configure the checkout step to avoid persisting
credentials. Update the jobs.quality-checks configuration and its
actions/checkout invocation while preserving the existing CI steps.
Source: Linters/SAST tools
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' | ||
| cache: 'npm' | ||
| cache-dependency-path: ./frontend/package-lock.json |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the declared runtime and the Next.js engine requirement before changing CI.
sed -n '20,26p' .github/workflows/ci.yml
rg -n -C2 '"next"|"engines"' frontend/package.json
npm view next@16.2.4 engines --jsonRepository: Yash-pluto/refme
Length of output: 837
🌐 Web query:
site:nodejs.org/en/about/previous-releases Node.js 20 EOL date Node.js 24 LTS
💡 Result:
Node.js v20 (Iron) has reached its End-of-Life (EOL) date of March 24, 2026 [1]. Node.js v24 (Krypton) is an Long-Term Support (LTS) release that was first released on May 6, 2025 [1].
Citations:
Use Node.js 24 for CI.
Node.js 20 reached EOL on March 24, 2026. next@16.2.4 requires Node.js >=20.9.0, so Node.js 24 satisfies its engine requirement.
Proposed update
- node-version: '20'
+ node-version: '24'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: '20' | |
| cache: 'npm' | |
| cache-dependency-path: ./frontend/package-lock.json | |
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: '24' | |
| cache: 'npm' | |
| cache-dependency-path: ./frontend/package-lock.json |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 20 - 25, Update the node-version
setting in the “Setup Node.js” workflow step from 20 to 24, while preserving the
existing npm cache and cache-dependency-path configuration.
| const allTopics = getAllTopics(); | ||
| const currentIndex = allTopics.findIndex((topic) => topic.id === resolvedParams.topic); | ||
| const prevTopic = currentIndex > 0 ? allTopics[currentIndex - 1] : null; | ||
| const nextTopic = | ||
| currentIndex >= 0 && currentIndex < allTopics.length - 1 | ||
| ? allTopics[currentIndex + 1] | ||
| : null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Sort allTopics before deriving previous/next.
getAllTopics returns entries in fs.readdirSync order. That order is filesystem-dependent and is not guaranteed to be stable across the dev machine and the deployment target. Previous/next navigation and the sidebar order can therefore differ between environments. Sort explicitly, ideally by a frontmatter order field with a title fallback.
🐛 Proposed fix
- const allTopics = getAllTopics();
+ const allTopics = [...getAllTopics()].sort((a, b) =>
+ String(a.title ?? a.id).localeCompare(String(b.title ?? b.id))
+ );
const currentIndex = allTopics.findIndex((topic) => topic.id === resolvedParams.topic);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const allTopics = getAllTopics(); | |
| const currentIndex = allTopics.findIndex((topic) => topic.id === resolvedParams.topic); | |
| const prevTopic = currentIndex > 0 ? allTopics[currentIndex - 1] : null; | |
| const nextTopic = | |
| currentIndex >= 0 && currentIndex < allTopics.length - 1 | |
| ? allTopics[currentIndex + 1] | |
| : null; | |
| const allTopics = [...getAllTopics()].sort((a, b) => | |
| String(a.title ?? a.id).localeCompare(String(b.title ?? b.id)) | |
| ); | |
| const currentIndex = allTopics.findIndex((topic) => topic.id === resolvedParams.topic); | |
| const prevTopic = currentIndex > 0 ? allTopics[currentIndex - 1] : null; | |
| const nextTopic = | |
| currentIndex >= 0 && currentIndex < allTopics.length - 1 | |
| ? allTopics[currentIndex + 1] | |
| : null; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/app/`[topic]/page.tsx around lines 32 - 38, Sort the array returned
by getAllTopics before calculating currentIndex, prevTopic, and nextTopic, using
each topic’s frontmatter order value when available and falling back to title
for deterministic ordering. Ensure the sorted order is also used by the sidebar
so navigation and display remain consistent across environments.
|
|
||
| export default function TopicLayout({ | ||
| children, | ||
| frontmatter, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Type frontmatter and remove it if it stays unused.
The quality-checks job reports two findings here: 'frontmatter' is defined but never used at line 20 and Unexpected any at line 28. The new layout renders the title from the topic list, not from frontmatter. Either drop the prop and its type, or render it and give it a concrete type.
♻️ Option A: drop the unused prop
export default function TopicLayout({
children,
- frontmatter,
topicKey,
topics = [],
headings = [],
prevTopic,
nextTopic,
}: {
children: React.ReactNode;
- frontmatter: any;
topicKey: string;Then remove frontmatter={...} from both call sites in frontend/app/[topic]/page.tsx.
♻️ Option B: keep and type it
- frontmatter: any;
+ frontmatter?: { title?: string; description?: string; source?: string };Also applies to: 28-28
🧰 Tools
🪛 GitHub Actions: PR Checks / 0_quality-checks.txt
[warning] 20-20: ESLint: 'frontmatter' is defined but never used. (@typescript-eslint/no-unused-vars)
🪛 GitHub Actions: PR Checks / quality-checks
[warning] 20-20: ESLint: 'frontmatter' is defined but never used. (@typescript-eslint/no-unused-vars)
🪛 GitHub Check: quality-checks
[warning] 20-20:
'frontmatter' is defined but never used
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/app/`[topic]/TopicLayout.tsx at line 20, Remove the unused
frontmatter prop and its type from the layout component, then remove the
corresponding frontmatter={...} arguments from both call sites in page.tsx; also
eliminate the any type reported in the component props.
Source: Linters/SAST tools
| useEffect(() => { | ||
| const ids = outlineGroups.flatMap((group) => [group.id, ...group.items.map((item) => item.id)]); | ||
| if (ids.length === 0) { | ||
| setActiveOutlineId(null); | ||
| return; | ||
| } | ||
|
|
||
| const updateActiveOutline = () => { | ||
| const offset = 140; | ||
| let currentId: string | null = ids[0] ?? null; | ||
|
|
||
| for (const id of ids) { | ||
| const element = document.getElementById(id); | ||
| if (!element) continue; | ||
|
|
||
| const rect = element.getBoundingClientRect(); | ||
| if (rect.top <= offset) { | ||
| currentId = id; | ||
| } | ||
| } | ||
|
|
||
| setActiveOutlineId((previous) => previous === currentId ? previous : currentId); | ||
| }; | ||
|
|
||
| updateActiveOutline(); | ||
| window.addEventListener("scroll", updateActiveOutline, { passive: true }); | ||
| window.addEventListener("resize", updateActiveOutline); | ||
|
|
||
| return () => { | ||
| window.removeEventListener("scroll", updateActiveOutline); | ||
| window.removeEventListener("resize", updateActiveOutline); | ||
| }; | ||
| }, [outlineGroups]); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Throttle the scroll handler. It reads layout for every heading on every scroll event.
updateActiveOutline calls getBoundingClientRect once per heading id inside a raw scroll listener. On a long topic page with many H2/H3 anchors, this forces a synchronous layout read on every scroll frame and causes jank on lower-end devices. IntersectionObserver is the idiomatic solution, and it needs no scroll listener at all. A requestAnimationFrame guard is the smaller change.
⚡ Proposed fix: rAF throttle
const updateActiveOutline = () => {
const offset = 140;
let currentId: string | null = ids[0] ?? null;
for (const id of ids) {
const element = document.getElementById(id);
if (!element) continue;
const rect = element.getBoundingClientRect();
if (rect.top <= offset) {
currentId = id;
}
}
setActiveOutlineId((previous) => previous === currentId ? previous : currentId);
};
+ let frame = 0;
+ const scheduleUpdate = () => {
+ if (frame) return;
+ frame = window.requestAnimationFrame(() => {
+ frame = 0;
+ updateActiveOutline();
+ });
+ };
+
updateActiveOutline();
- window.addEventListener("scroll", updateActiveOutline, { passive: true });
- window.addEventListener("resize", updateActiveOutline);
+ window.addEventListener("scroll", scheduleUpdate, { passive: true });
+ window.addEventListener("resize", scheduleUpdate);
return () => {
- window.removeEventListener("scroll", updateActiveOutline);
- window.removeEventListener("resize", updateActiveOutline);
+ if (frame) window.cancelAnimationFrame(frame);
+ window.removeEventListener("scroll", scheduleUpdate);
+ window.removeEventListener("resize", scheduleUpdate);
};
}, [outlineGroups]);As per path instructions: "Ensure React components are using modern hooks, avoid unnecessary re-renders, and use TailwindCSS properly."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| const ids = outlineGroups.flatMap((group) => [group.id, ...group.items.map((item) => item.id)]); | |
| if (ids.length === 0) { | |
| setActiveOutlineId(null); | |
| return; | |
| } | |
| const updateActiveOutline = () => { | |
| const offset = 140; | |
| let currentId: string | null = ids[0] ?? null; | |
| for (const id of ids) { | |
| const element = document.getElementById(id); | |
| if (!element) continue; | |
| const rect = element.getBoundingClientRect(); | |
| if (rect.top <= offset) { | |
| currentId = id; | |
| } | |
| } | |
| setActiveOutlineId((previous) => previous === currentId ? previous : currentId); | |
| }; | |
| updateActiveOutline(); | |
| window.addEventListener("scroll", updateActiveOutline, { passive: true }); | |
| window.addEventListener("resize", updateActiveOutline); | |
| return () => { | |
| window.removeEventListener("scroll", updateActiveOutline); | |
| window.removeEventListener("resize", updateActiveOutline); | |
| }; | |
| }, [outlineGroups]); | |
| useEffect(() => { | |
| const ids = outlineGroups.flatMap((group) => [group.id, ...group.items.map((item) => item.id)]); | |
| if (ids.length === 0) { | |
| setActiveOutlineId(null); | |
| return; | |
| } | |
| const updateActiveOutline = () => { | |
| const offset = 140; | |
| let currentId: string | null = ids[0] ?? null; | |
| for (const id of ids) { | |
| const element = document.getElementById(id); | |
| if (!element) continue; | |
| const rect = element.getBoundingClientRect(); | |
| if (rect.top <= offset) { | |
| currentId = id; | |
| } | |
| } | |
| setActiveOutlineId((previous) => previous === currentId ? previous : currentId); | |
| }; | |
| let frame = 0; | |
| const scheduleUpdate = () => { | |
| if (frame) return; | |
| frame = window.requestAnimationFrame(() => { | |
| frame = 0; | |
| updateActiveOutline(); | |
| }); | |
| }; | |
| updateActiveOutline(); | |
| window.addEventListener("scroll", scheduleUpdate, { passive: true }); | |
| window.addEventListener("resize", scheduleUpdate); | |
| return () => { | |
| if (frame) window.cancelAnimationFrame(frame); | |
| window.removeEventListener("scroll", scheduleUpdate); | |
| window.removeEventListener("resize", scheduleUpdate); | |
| }; | |
| }, [outlineGroups]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/app/`[topic]/TopicLayout.tsx around lines 115 - 147, Throttle
updateActiveOutline with a requestAnimationFrame guard so rapid scroll events
schedule at most one layout read per animation frame. Keep the existing
heading-order calculation and activeOutlineId update behavior, cancel any
pending frame during effect cleanup, and retain the resize handling without
introducing unnecessary renders.
Source: Path instructions
| <div className='mx-auto max-w-3xl'> | ||
| <div className='overflow-hidden rounded-2xl border border-[#1F1F1F] bg-[#0B0B0B] shadow-[0_0_0_1px_rgba(255,255,255,0.02)]'> | ||
| <div className='border-b border-[#1F1F1F] bg-[#0E0E0E] px-6 py-4'> | ||
| <div className='inline-flex items-center gap-2 rounded-full border border-[#2A2A2A] bg-[#111111] px-3 py-1.5 text-[10px] font-medium uppercase tracking-[0.2em] text-[#B4B4B4]'> | ||
| Draft section | ||
| </div> | ||
| </div> | ||
|
|
||
| <p className='mb-4'> | ||
| In the meantime, you can check out the project's author: | ||
| <a | ||
| href='https://github.com/yash-pluto' | ||
| target='_blank' | ||
| rel='noopener noreferrer' | ||
| className='ml-2 text-indigo-600 hover:underline' | ||
| > | ||
| @Yash-pluto | ||
| </a> | ||
| </p> | ||
| <div className='space-y-6 p-8 md:p-10'> | ||
| <div className='space-y-3'> | ||
| <h2 className='text-3xl font-semibold tracking-[-0.06em] text-white md:text-4xl'> | ||
| This topic is still being written. | ||
| </h2> | ||
| <p className='max-w-2xl text-base leading-7 text-[#B8B8B8]'> | ||
| The reference for <span className='font-medium text-[#C699FF]'>{topicKey}</span> is queued up for later, and it will be expanded into a full guide with examples, explanations, and patterns. | ||
| </p> | ||
| </div> | ||
|
|
||
| <p className='text-sm text-gray-500'> | ||
| Requested topic: <span className='font-mono'>{topicKey}</span> | ||
| </p> | ||
| <div className='flex flex-wrap gap-3 text-[11px] font-medium uppercase tracking-[0.2em] text-[#8B8B8B]'> | ||
| <span className='rounded-full border border-[#2A2A2A] bg-[#111111] px-3 py-1.5'>Examples</span> | ||
| <span className='rounded-full border border-[#2A2A2A] bg-[#111111] px-3 py-1.5'>Patterns</span> | ||
| <span className='rounded-full border border-[#2A2A2A] bg-[#111111] px-3 py-1.5'>Best practices</span> | ||
| </div> | ||
|
|
||
| <div className='rounded-xl border border-[#1F1F1F] bg-[#101010] p-4'> | ||
| <p className='text-sm leading-7 text-[#D0D0D0]'> | ||
| Until then, you can explore the rest of the library or check the project author: | ||
| <a | ||
| href='https://github.com/yash-pluto' | ||
| target='_blank' | ||
| rel='noopener noreferrer' | ||
| className='ml-2 font-medium text-[#C699FF] underline decoration-[#C699FF]/50 underline-offset-4 transition hover:text-[#D7B9FF]' | ||
| > | ||
| @Yash-pluto | ||
| </a> | ||
| </p> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Two surfaces hardcode dark colours and break light mode. Both sites set colour values directly instead of resolving them through the theme, so light mode renders unreadable or inverted UI. Route both through the same theme mechanism the rest of the PR uses.
frontend/app/components/SkeletonTopic.tsx#L3-L40: replace the hardcodedbg-[#0B0B0B],bg-[#0E0E0E],bg-[#101010],border-[#1F1F1F],text-white, andtext-[#B8B8B8]values with theme-aware pairs, either throughdark:variants or auseTheme()map.frontend/app/page.tsx#L170-L173: replace the hardcodedtext-zinc-300on the "Developer reference library" label with${theme.muted}.
📍 Affects 2 files
frontend/app/components/SkeletonTopic.tsx#L3-L40(this comment)frontend/app/page.tsx#L170-L173
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/app/components/SkeletonTopic.tsx` around lines 3 - 40, The hardcoded
dark-mode colors in SkeletonTopic and the page label bypass the shared theme
mechanism. In frontend/app/components/SkeletonTopic.tsx lines 3-40, replace the
specified background, border, and text colors with theme-aware dark/light
variants or the existing useTheme map; in frontend/app/page.tsx lines 170-173,
replace text-zinc-300 on the “Developer reference library” label with
${theme.muted}.
Source: Path instructions
| --font-mono: | ||
| var(--font-jetbrains-mono), ui-monospace, SFMono-Regular, Menlo, Monaco, | ||
| Consolas, monospace; | ||
| --font-sans: "Segoe UI", Inter, "Helvetica Neue", Arial, sans-serif; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the font stack compatible with Stylelint.
Stylelint reports value-keyword-case errors for Inter and Arial on Line 5. Use the configured lowercase values so this declaration passes the repository style check.
Proposed lint fix
- --font-sans: "Segoe UI", Inter, "Helvetica Neue", Arial, sans-serif;
+ --font-sans: "Segoe UI", inter, "Helvetica Neue", arial, sans-serif;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --font-sans: "Segoe UI", Inter, "Helvetica Neue", Arial, sans-serif; | |
| --font-sans: "Segoe UI", inter, "Helvetica Neue", arial, sans-serif; |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 5-5: Expected "Inter" to be "inter" (value-keyword-case)
(value-keyword-case)
[error] 5-5: Expected "Arial" to be "arial" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/app/globals.css` at line 5, Update the --font-sans declaration to
use lowercase font-family keyword values for Inter and Arial, while preserving
the existing stack order and other fonts so it passes Stylelint's
value-keyword-case check.
Source: Linters/SAST tools
| font-family: var(--font-jetbrains-mono) !important; | ||
| /* Force all code blocks to use JetBrains */ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the replacement monospace variable.
frontend/app/layout.tsx no longer defines --font-jetbrains-mono. This unresolved custom property invalidates the font-family declaration. Use --font-mono, which Line 6 defines, and update the stale comment.
Proposed font-variable fix
- font-family: var(--font-jetbrains-mono) !important;
- /* Force all code blocks to use JetBrains */
+ font-family: var(--font-mono) !important;
+ /* Use the global monospace stack for code blocks */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| font-family: var(--font-jetbrains-mono) !important; | |
| /* Force all code blocks to use JetBrains */ | |
| font-family: var(--font-mono) !important; | |
| /* Use the global monospace stack for code blocks */ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/app/globals.css` around lines 26 - 27, Update the code-block font
declaration in the global CSS to use the defined --font-mono variable instead of
the removed --font-jetbrains-mono variable, and revise the adjacent comment to
accurately describe the applied monospace font.
| <label className={`flex w-full items-center gap-3 rounded-2xl border px-4 py-3 shadow-sm transition ${theme.panel} focus-within:border-[#C699FF]/60`}> | ||
| <Search size={16} className='text-zinc-500' /> | ||
| <input | ||
| type='text' | ||
| value={searchQuery} | ||
| onChange={(event) => setSearchQuery(event.target.value)} | ||
| placeholder='Search topics...' | ||
| className='w-full bg-transparent text-sm outline-none placeholder:text-zinc-500' | ||
| /> | ||
| </label> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Give the search input an accessible name.
The <label> wraps the input, so pointer clicks focus it. But the label has no text content. Screen readers announce the field with the placeholder only, and placeholders are not a reliable accessible name.
Add aria-label to the input, or add visually hidden label text.
♿ Proposed fix
<input
type='text'
+ aria-label='Search topics'
value={searchQuery}
onChange={(event) => setSearchQuery(event.target.value)}
placeholder='Search topics...'
className='w-full bg-transparent text-sm outline-none placeholder:text-zinc-500'
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <label className={`flex w-full items-center gap-3 rounded-2xl border px-4 py-3 shadow-sm transition ${theme.panel} focus-within:border-[#C699FF]/60`}> | |
| <Search size={16} className='text-zinc-500' /> | |
| <input | |
| type='text' | |
| value={searchQuery} | |
| onChange={(event) => setSearchQuery(event.target.value)} | |
| placeholder='Search topics...' | |
| className='w-full bg-transparent text-sm outline-none placeholder:text-zinc-500' | |
| /> | |
| </label> | |
| <label className={`flex w-full items-center gap-3 rounded-2xl border px-4 py-3 shadow-sm transition ${theme.panel} focus-within:border-[`#C699FF`]/60`}> | |
| <Search size={16} className='text-zinc-500' /> | |
| <input | |
| type='text' | |
| aria-label='Search topics' | |
| value={searchQuery} | |
| onChange={(event) => setSearchQuery(event.target.value)} | |
| placeholder='Search topics...' | |
| className='w-full bg-transparent text-sm outline-none placeholder:text-zinc-500' | |
| /> | |
| </label> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/app/page.tsx` around lines 185 - 194, Add an explicit accessible
name to the search input in the existing label/input block by adding an
aria-label such as “Search topics,” or provide equivalent visually hidden label
text. Preserve the current searchQuery value and onChange behavior.
| export function extractHeadings(content: string): DocHeading[] { | ||
| return content | ||
| .split("\n") | ||
| .reduce<DocHeading[]>((acc, line) => { | ||
| const match = line.match(/^(##|###)\s+(.+)$/); | ||
|
|
||
| if (!match) return acc; | ||
|
|
||
| const depth = match[1].length === 2 ? 2 : 3; | ||
| const title = match[2] | ||
| .trim() | ||
| .replace(/[*_`>#]/g, "") | ||
| .replace(/\s+/g, " "); | ||
| const id = title | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9\s-]/g, "") | ||
| .trim() | ||
| .replace(/\s+/g, "-"); | ||
|
|
||
| if (!id) return acc; | ||
|
|
||
| acc.push({ id, title, depth }); | ||
| return acc; | ||
| }, []); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make extractHeadings skip fenced code blocks.
The reducer scans every line of the raw MDX. A line inside a fenced code block that starts with ## or ### is treated as a heading. Python and shell samples commonly contain ## comment lines. The sidebar then shows sections that have no anchor in the DOM, and scrollToSection silently does nothing.
A second, smaller point: two headings with the same title produce the same id. document.getElementById resolves both outline entries to the first element. Add a suffix on collision.
🐛 Proposed fix: fence tracking plus ID de-duplication
export function extractHeadings(content: string): DocHeading[] {
+ const seen = new Map<string, number>();
+ let insideFence = false;
+
return content
.split("\n")
.reduce<DocHeading[]>((acc, line) => {
+ if (/^\s*(```|~~~)/.test(line)) {
+ insideFence = !insideFence;
+ return acc;
+ }
+ if (insideFence) return acc;
+
const match = line.match(/^(##|###)\s+(.+)$/);
if (!match) return acc;
const depth = match[1].length === 2 ? 2 : 3;
const title = match[2]
.trim()
.replace(/[*_`>#]/g, "")
.replace(/\s+/g, " ");
- const id = title
+ const baseId = title
.toLowerCase()
.replace(/[^a-z0-9\s-]/g, "")
.trim()
.replace(/\s+/g, "-");
- if (!id) return acc;
+ if (!baseId) return acc;
+
+ const count = seen.get(baseId) ?? 0;
+ seen.set(baseId, count + 1);
+ const id = count === 0 ? baseId : `${baseId}-${count}`;
acc.push({ id, title, depth });
return acc;
}, []);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function extractHeadings(content: string): DocHeading[] { | |
| return content | |
| .split("\n") | |
| .reduce<DocHeading[]>((acc, line) => { | |
| const match = line.match(/^(##|###)\s+(.+)$/); | |
| if (!match) return acc; | |
| const depth = match[1].length === 2 ? 2 : 3; | |
| const title = match[2] | |
| .trim() | |
| .replace(/[*_`>#]/g, "") | |
| .replace(/\s+/g, " "); | |
| const id = title | |
| .toLowerCase() | |
| .replace(/[^a-z0-9\s-]/g, "") | |
| .trim() | |
| .replace(/\s+/g, "-"); | |
| if (!id) return acc; | |
| acc.push({ id, title, depth }); | |
| return acc; | |
| }, []); | |
| } | |
| export function extractHeadings(content: string): DocHeading[] { | |
| const seen = new Map<string, number>(); | |
| let insideFence = false; | |
| return content | |
| .split("\n") | |
| .reduce<DocHeading[]>((acc, line) => { | |
| if (/^\s*( |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/lib/mdx.ts` around lines 15 - 39, Update extractHeadings to
track fenced code-block state and ignore fence delimiter lines and all content
while inside ``` or ~~~ fences. Rename the generated heading identifier to a
base ID, maintain a seen-count map, and append an incrementing suffix for
repeated IDs while preserving the first heading’s unsuffixed ID.
Summary by CodeRabbit
New Features
Bug Fixes