document prefixclassname + other QoL improvements#1896
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:
📝 WalkthroughWalkthroughAdds navigation links (Playground, Colors), a FullHeightScroll Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
docs/components/Main/NavBar/helpers/NavRoot/index.tsx (1)
116-131: Centralize primary nav items to avoid desktop/sidebar drift.These entries are now duplicated with
docs/components/navigation/Navigation.tsx(Main category), so updates will need to be synchronized manually in two places.♻️ Suggested extraction
+// docs/components/navigation/navItems.ts +export const MAIN_NAV_ITEMS = [ + { title: "Documentation", path: "/docs/first-steps/introduction" }, + { title: "Playground", path: "/playground" }, + { title: "Colors", path: "/colors" }, + { title: "Showcase", path: "/showcase/music-app" }, +] as const;-<li>...hardcoded links...</li> +{MAIN_NAV_ITEMS.map((item) => ( + <li key={item.path}> + <Link className="text-gray-950 hover:text-gray-1000" href={item.path}> + {item.title === "Documentation" ? "Docs" : item.title} + </Link> + </li> +))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/components/Main/NavBar/helpers/NavRoot/index.tsx` around lines 116 - 131, The Playground and Colors <Link> entries in NavRoot are duplicated with the Main category in Navigation.tsx; extract the shared primary nav items into a single exported array or component (e.g., export const MAIN_NAV or a MainNav component) from docs/components/navigation/Navigation.tsx (or a new shared module) and replace the hard-coded <Link> entries in NavRoot with an import that maps over that shared list (or renders the shared component) so both NavRoot and Navigation.tsx consume the same source of truth.docs/components/ColorTemplate.js (1)
5-44: Avoid a second source of truth for the color catalog.
colorFamiliesandscaleStepsnow exist here and again inside the safelist regex indocs/tailwind.config.ts. If one side changes without the other, this page will render swatches whose CSS never gets generated. Please move this catalog into a shared constants module and derive the safelist from that shared data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/components/ColorTemplate.js` around lines 5 - 44, Move the color catalog out of docs/components/ColorTemplate.js into a shared constants module (e.g., export const colorFamilies and export const scaleSteps from a new module) and update both docs/components/ColorTemplate.js and docs/tailwind.config.ts to import these exports; replace the hard-coded colorFamilies and scaleSteps in ColorTemplate.js with imports, and derive the safelist regex or safelist generation in docs/tailwind.config.ts from the imported colorFamilies and scaleSteps (use programmatic concatenation to build the same patterns rather than a duplicated literal regex).
🤖 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/docs/components/separator/content.mdx`:
- Around line 16-18: The SVG logo element (the <svg> tag containing the long
<path> string) is decorative and should be hidden from assistive tech; update
that <svg> to include accessibility attributes such as aria-hidden="true",
focusable="false", and role="presentation" (or role="img" only if you add a
descriptive <title>), so screen readers will ignore the unlabeled graphic.
In `@docs/components/ColorTemplate.js`:
- Around line 98-134: The 13-column swatch matrix (rendered by mapping
colorFamilies and scaleSteps and producing ColorSwatch components) lacks
horizontal overflow handling on small screens; wrap the grid that contains the
label column + 12 swatch columns (the parent div that maps colorFamilies and
renders formatFamilyName and ColorSwatch items) in a container with
overflow-x-auto and give the inner grid a sensible min-width (e.g., a min-w that
ensures each swatch remains tappable) so the matrix scrolls horizontally on
narrow viewports; ensure only the matrix is scrollable (not the labels) and keep
existing class names like grid-cols-[84px_repeat(12,minmax(0,1fr))] intact.
In `@docs/utils/parseSourceCode.tsx`:
- Around line 57-60: The current VERCEL branch returns
readGithubSourceCode(normalizedSourcePath) which may return response.text()
containing GitHub error HTML; update readGithubSourceCode (or the call site in
parseSourceCode.tsx) to check response.ok and, if false, throw an Error that
includes the HTTP status and the normalizedSourcePath so failures surface
instead of injecting error payloads into docs; locate the readGithubSourceCode
function and ensure it either throws on non-ok responses or that
parseSourceCode.tsx inspects the returned Response, checks response.ok, and
throws a descriptive error with status and path before attempting to read text.
---
Nitpick comments:
In `@docs/components/ColorTemplate.js`:
- Around line 5-44: Move the color catalog out of
docs/components/ColorTemplate.js into a shared constants module (e.g., export
const colorFamilies and export const scaleSteps from a new module) and update
both docs/components/ColorTemplate.js and docs/tailwind.config.ts to import
these exports; replace the hard-coded colorFamilies and scaleSteps in
ColorTemplate.js with imports, and derive the safelist regex or safelist
generation in docs/tailwind.config.ts from the imported colorFamilies and
scaleSteps (use programmatic concatenation to build the same patterns rather
than a duplicated literal regex).
In `@docs/components/Main/NavBar/helpers/NavRoot/index.tsx`:
- Around line 116-131: The Playground and Colors <Link> entries in NavRoot are
duplicated with the Main category in Navigation.tsx; extract the shared primary
nav items into a single exported array or component (e.g., export const MAIN_NAV
or a MainNav component) from docs/components/navigation/Navigation.tsx (or a new
shared module) and replace the hard-coded <Link> entries in NavRoot with an
import that maps over that shared list (or renders the shared component) so both
NavRoot and Navigation.tsx consume the same source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 142e68dd-e3d9-4686-a393-16cf5a4150d6
📒 Files selected for processing (14)
docs/app/colors/page.tsxdocs/app/docs/components/separator/content.mdxdocs/app/docs/components/separator/docs/codeUsage.jsdocs/app/docs/components/switch/docs/codeUsage.jsdocs/app/docs/first-steps/usage/content.mdxdocs/app/playground/components/TabsPlayground.jsdocs/app/playground/page.tsxdocs/components/ColorTemplate.jsdocs/components/Main/NavBar/helpers/NavRoot/index.tsxdocs/components/docsHelpers/EditPageOnGithub.tsxdocs/components/layout/ScrollContainers/FullHeightScroll.tsxdocs/components/navigation/Navigation.tsxdocs/tailwind.config.tsdocs/utils/parseSourceCode.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/pr-title.yml (1)
4-5: Reduce unnecessary workflow executions by removingsynchronizefrom trigger types.This workflow only validates the PR title; rerunning on every new commit adds CI noise/cost without improving correctness.
💡 Proposed diff
on: pull_request_target: - types: [opened, edited, reopened, synchronize] + types: [opened, edited, reopened]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-title.yml around lines 4 - 5, Remove the unnecessary 'synchronize' trigger from the pull_request_target types list so the workflow only runs on PR open/edit/reopen events; update the pull_request_target types array (currently types: [opened, edited, reopened, synchronize]) to exclude 'synchronize' and leave types: [opened, edited, reopened] to avoid executing the PR title check on every new commit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-title.yml:
- Around line 31-33: Trim the summary before validating and replace the current
startsLowercase boolean with a check that the first character is an actual
lowercase letter (e.g., const trimmed = summary.trim(); const startsLowercase =
/^[a-z]/.test(trimmed);), and use trimmed for subsequent checks like
hasTrailingPeriod (trimmed.endsWith('.')) and any length checks so whitespace or
non-letter prefixes can't bypass the lowercase rule; update references to
summary in these validations to use trimmed.
---
Nitpick comments:
In @.github/workflows/pr-title.yml:
- Around line 4-5: Remove the unnecessary 'synchronize' trigger from the
pull_request_target types list so the workflow only runs on PR open/edit/reopen
events; update the pull_request_target types array (currently types: [opened,
edited, reopened, synchronize]) to exclude 'synchronize' and leave types:
[opened, edited, reopened] to avoid executing the PR title check on every new
commit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bfe31587-39a6-4d31-bef9-6615d989fdfb
📒 Files selected for processing (2)
.github/workflows/docs-build.yml.github/workflows/pr-title.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/docs-build.yml
| const startsLowercase = summary[0] === summary[0].toLowerCase(); | ||
| const hasTrailingPeriod = summary.endsWith('.'); | ||
| const withinLimit = title.length <= 72; |
There was a problem hiding this comment.
Lowercase-summary rule is currently bypassable for non-letter prefixes.
Line 31 treats any non-letter first character as valid lowercase (e.g., leading space, digit, punctuation). Tighten this to enforce an actual lowercase letter and validate against trimmed summary text.
💡 Proposed diff
if (match) {
const summary = match[3];
- const startsLowercase = summary[0] === summary[0].toLowerCase();
- const hasTrailingPeriod = summary.endsWith('.');
+ const trimmedSummary = summary.trim();
+ const startsLowercase = /^[a-z]/.test(trimmedSummary);
+ const hasTrailingPeriod = trimmedSummary.endsWith('.');
const withinLimit = title.length <= 72;
- if (startsLowercase && !hasTrailingPeriod && withinLimit) {
+ if (trimmedSummary === summary && startsLowercase && !hasTrailingPeriod && withinLimit) {
core.info(`PR title is valid: "${title}"`);
return;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-title.yml around lines 31 - 33, Trim the summary before
validating and replace the current startsLowercase boolean with a check that the
first character is an actual lowercase letter (e.g., const trimmed =
summary.trim(); const startsLowercase = /^[a-z]/.test(trimmed);), and use
trimmed for subsequent checks like hasTrailingPeriod (trimmed.endsWith('.')) and
any length checks so whitespace or non-letter prefixes can't bypass the
lowercase rule; update references to summary in these validations to use
trimmed.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
knowledge/MASTER_LIST.md (1)
48-61: Consider reducing repetitive sentence openings for readability.Lines 48-61 and Line 67 repeatedly start bullets with “Use/For”. A light reword pass would make the section scan faster without changing meaning.
Suggested wording refinement
- `knowledge/features/styling-namespaces.md` - Use for `Theme classNamespace` behavior and generated class rules. + Reference for `Theme classNamespace` behavior and generated class rules. - `knowledge/good_practices/index.md` - Use for implementation guardrails, especially SSR-safe class generation and avoiding `querySelector`. + Covers implementation guardrails, especially SSR-safe class generation and avoiding `querySelector`. - `knowledge/querySelector-to-refs/components-analysis.md` - Use when touching components that currently rely on `querySelector`. + Consult when touching components that currently rely on `querySelector`. - `knowledge/design_system/clarity_design_system.md` - Use for design-system thinking and perceptual consistency decisions. + Provides guidance for design-system thinking and perceptual consistency decisions. - `knowledge/releases/how-rad-ui-releases-are-made.md` - Use for release-stage terminology and release documentation. + Defines release-stage terminology and release documentation.Also applies to: 65-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@knowledge/MASTER_LIST.md` around lines 48 - 61, The bulleted entries in MASTER_LIST.md repeatedly begin with "Use/For", which reduces scanability; edit the bullets for items like `knowledge/what-makes-a-library-truly-headless.md`, `knowledge/features/data-attributes.md`, `knowledge/features/styling-namespaces.md`, `knowledge/good_practices/index.md`, `knowledge/querySelector-to-refs/components-analysis.md`, `knowledge/design_system/clarity_design_system.md`, `knowledge/releases/how-rad-ui-releases-are-made.md` (and the nearby bullets at the 65-67 block) to start with the topic or file name followed by a short descriptor (e.g., "what-makes-a-library-truly-headless.md — headless architecture and API boundaries" or "features/data-attributes.md — public data-* conventions"), keeping meaning identical but removing repeated "Use/For" phrasing to improve readability and consistent concise style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@knowledge/MASTER_LIST.md`:
- Around line 48-61: The bulleted entries in MASTER_LIST.md repeatedly begin
with "Use/For", which reduces scanability; edit the bullets for items like
`knowledge/what-makes-a-library-truly-headless.md`,
`knowledge/features/data-attributes.md`,
`knowledge/features/styling-namespaces.md`, `knowledge/good_practices/index.md`,
`knowledge/querySelector-to-refs/components-analysis.md`,
`knowledge/design_system/clarity_design_system.md`,
`knowledge/releases/how-rad-ui-releases-are-made.md` (and the nearby bullets at
the 65-67 block) to start with the topic or file name followed by a short
descriptor (e.g., "what-makes-a-library-truly-headless.md — headless
architecture and API boundaries" or "features/data-attributes.md — public data-*
conventions"), keeping meaning identical but removing repeated "Use/For"
phrasing to improve readability and consistent concise style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c45f16f-1584-4f51-842e-adeb102c92fd
📒 Files selected for processing (7)
.cursor/rules/00-master-list.mdc.gitignoreAGENTS.mdCLAUDE.mddocs/app/page.tsxdocs/components/layout/ScrollContainers/FullHeightScroll.tsxknowledge/MASTER_LIST.md
✅ Files skipped from review due to trivial changes (4)
- AGENTS.md
- .gitignore
- CLAUDE.md
- .cursor/rules/00-master-list.mdc
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/components/layout/ScrollContainers/FullHeightScroll.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/components/ColorTemplate.js (1)
98-136:⚠️ Potential issue | 🟠 MajorKeep the step headers in the same horizontal scroll region as the swatches.
Right now only the matrix scrolls. On small screens the
50…1000header stays fixed while the swatches move, so the columns stop lining up and the comparison becomes hard to use.
↔️ One straightforward fix- <div className="space-y-4"> - <div className="pl-[96px]"> + <div className="overflow-x-auto"> + <div className="min-w-[960px] space-y-4"> + <div className="pl-[96px]"> ... - <div className="overflow-x-auto"> - <div className="min-w-[960px] space-y-1.5"> + <div className="space-y-1.5"> ... - </div> + </div> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/components/ColorTemplate.js` around lines 98 - 136, The step headers are outside the horizontal scroll container so they don't scroll with the swatches; wrap the header block (the div that contains groups.map and the scaleSteps row — the element with class pl-[96px]) together with the swatch matrix (the element with class overflow-x-auto / the inner div with min-w-[960px] and the colorFamilies.map) in the same horizontal-scrolling container (i.e., move the header into the same overflow-x-auto parent or make a new parent with overflow-x-auto that contains both), ensure the shared inner container uses the same min-width/grid layout (grid-cols-[84px_repeat(12,minmax(0,1fr))]) so the columns line up.
🧹 Nitpick comments (2)
src/components/ui/Tree/stories/Tree.stories.tsx (1)
191-238: Avoid hardcoding therad-uiclass prefix in story CSS.These selectors will break if
classNamespaceis changed (which this PR is documenting elsewhere). Prefer namespace-agnostic selectors or derive the active prefix before building the CSS string.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Tree/stories/Tree.stories.tsx` around lines 191 - 238, The story CSS hardcodes the "rad-ui" class prefix which will break if classNamespace changes; update Tree.stories.tsx to build selectors dynamically using the story's classNamespace (or the same variable used elsewhere in the file) instead of "rad-ui" (e.g. replace ".rad-ui-tree", ".rad-ui-tree-branch", ".rad-ui-tree-item" etc. with template strings like `.${classNamespace}-tree`, `.${classNamespace}-tree-branch`, `.${classNamespace}-tree-item` or use namespace-agnostic attribute selectors if preferred); locate the CSS string generation in Tree.stories.tsx and substitute the hardcoded tokens with the derived classNamespace (or a helper like getClassNamespace) so the selectors remain correct when the prefix changes.docs/components/ColorTemplate.js (1)
1-3: Remove the unnecessary'use client'directive.The component uses
<Link>fromnext/link(line 86), which is itself a Client Component and acts as the client boundary—no additional'use client'declaration is needed here. Since there are no other client-only hooks or APIs, the parent component can remain server-rendered while<Link>handles its own client requirements.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/components/ColorTemplate.js` around lines 1 - 3, Remove the unnecessary 'use client' directive at the top of ColorTemplate.js: the file only imports and uses the Link component from next/link (symbol: Link) and has no client-only hooks or APIs, so delete the "'use client'" line to keep the component server-rendered while allowing Link to act as the client boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/Tree/stories/Tree.stories.tsx`:
- Around line 226-229: The CSS rule that targets ".tree-anatomy-story
.rad-ui-tree-item:focus-visible" removes all visible focus styling, leaving
keyboard users without a focus cue; update that selector to preserve or provide
an accessible focus indicator (for example, keep a visible outline or a
box-shadow with sufficient contrast) instead of "outline: none; box-shadow:
none;", so keyboard focus on .rad-ui-tree-item in the anatomy story remains
visible and meets contrast/accessibility expectations.
---
Duplicate comments:
In `@docs/components/ColorTemplate.js`:
- Around line 98-136: The step headers are outside the horizontal scroll
container so they don't scroll with the swatches; wrap the header block (the div
that contains groups.map and the scaleSteps row — the element with class
pl-[96px]) together with the swatch matrix (the element with class
overflow-x-auto / the inner div with min-w-[960px] and the colorFamilies.map) in
the same horizontal-scrolling container (i.e., move the header into the same
overflow-x-auto parent or make a new parent with overflow-x-auto that contains
both), ensure the shared inner container uses the same min-width/grid layout
(grid-cols-[84px_repeat(12,minmax(0,1fr))]) so the columns line up.
---
Nitpick comments:
In `@docs/components/ColorTemplate.js`:
- Around line 1-3: Remove the unnecessary 'use client' directive at the top of
ColorTemplate.js: the file only imports and uses the Link component from
next/link (symbol: Link) and has no client-only hooks or APIs, so delete the
"'use client'" line to keep the component server-rendered while allowing Link to
act as the client boundary.
In `@src/components/ui/Tree/stories/Tree.stories.tsx`:
- Around line 191-238: The story CSS hardcodes the "rad-ui" class prefix which
will break if classNamespace changes; update Tree.stories.tsx to build selectors
dynamically using the story's classNamespace (or the same variable used
elsewhere in the file) instead of "rad-ui" (e.g. replace ".rad-ui-tree",
".rad-ui-tree-branch", ".rad-ui-tree-item" etc. with template strings like
`.${classNamespace}-tree`, `.${classNamespace}-tree-branch`,
`.${classNamespace}-tree-item` or use namespace-agnostic attribute selectors if
preferred); locate the CSS string generation in Tree.stories.tsx and substitute
the hardcoded tokens with the derived classNamespace (or a helper like
getClassNamespace) so the selectors remain correct when the prefix changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d203268-f770-46cd-a2f7-20f07baf1812
📒 Files selected for processing (6)
.github/workflows/pr-title.ymldocs/app/docs/components/separator/content.mdxdocs/components/ColorTemplate.jsdocs/components/layout/Documentation/helpers/ComponentHero/ComponentHero.jsdocs/utils/parseSourceCode.tsxsrc/components/ui/Tree/stories/Tree.stories.tsx
✅ Files skipped from review due to trivial changes (2)
- docs/components/layout/Documentation/helpers/ComponentHero/ComponentHero.js
- .github/workflows/pr-title.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/app/docs/components/separator/content.mdx
- docs/utils/parseSourceCode.tsx
CoverageThis report compares the PR with the base branch. "Δ" shows how the PR affects each metric.
Coverage improved or stayed the same. Great job! Run |
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores