Conversation
🦋 Changeset detectedLatest commit: 48cf394 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis PR implements a comprehensive design system UI revamp across multiple RadUI components, transitioning from semantic color tokens to new Clarity design system gray and accent color tokens while adding CSS transitions for improved visual feedback on interactive states. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.6.0)styles/themes/components/quote.scssConfigurationError: Could not find "stylelint-config-tailwindcss". Do you need to install the package or use the "configBasedir" option? styles/themes/components/radiocards.scssConfigurationError: Could not find "stylelint-config-tailwindcss". Do you need to install the package or use the "configBasedir" option? styles/themes/components/radio-group.scssConfigurationError: Could not find "stylelint-config-tailwindcss". Do you need to install the package or use the "configBasedir" option?
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 |
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
styles/themes/components/tab-nav.scss (1)
31-36: Consider using only:focus-visiblefor modern focus handling.The combined
&:focus-visible, &:focusselector applies focus styles on all focus events, including mouse clicks. This can create unwanted visual noise for mouse users. If the intent is to support older browsers, this is fine. Otherwise, using only:focus-visibleprovides better UX by showing focus rings only for keyboard navigation.- &:focus-visible, - &:focus { + &:focus-visible { color: var(--rad-ui-color-gray-1000); // step 11 — primary text box-shadow: var(--rad-ui-ring-shadow); outline: none; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles/themes/components/tab-nav.scss` around lines 31 - 36, The combined selector "&:focus-visible, &:focus" in tab-nav.scss applies focus styles on all focus events (including mouse), causing unnecessary visual noise; update the rule to use only "&:focus-visible" so the focus ring appears for keyboard navigation only, and if you need older-browser fallback keep a separate, explicitly documented fallback block (e.g., a comment or an opt-in variable) rather than applying the same styles to ":focus" by default—modify the selector used in the tab nav focus rule accordingly.styles/themes/components/spinner.scss (1)
40-45: Add trailing newline at end of file.SCSS files should end with a newline for POSIX compliance and cleaner diffs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles/themes/components/spinner.scss` around lines 40 - 45, Add a POSIX newline at the end of the file by ensuring the last line (the closing brace of the &[data-spinner-size="large"] block in spinner.scss) is followed by a single trailing newline character; update the file so it ends with that newline to satisfy POSIX and diff cleanliness..changeset/wet-corners-serve.md (1)
5-5: Consider a more descriptive changeset message.The current description "ui revamp for multiple comps" is vague. A more descriptive message would help consumers understand what changed, e.g., "Migrate Progress, Quote, Radio, Slider, Spinner, Splitter, and other components to Clarity design system tokens with improved transitions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/wet-corners-serve.md at line 5, Update the changeset summary string "ui revamp for multiple comps" to a more descriptive message that lists the affected components and the nature of the change (for example: "Migrate Progress, Quote, Radio, Slider, Spinner, Splitter and related components to Clarity design tokens and improve transition styles"); edit the content in the .changeset entry that currently contains "ui revamp for multiple comps" to this expanded description so consumers can understand what changed and why.styles/themes/components/switch.scss (1)
25-29: Transition timing differs from other components.The switch uses
0.2sand0.22stransitions while other components in this PR consistently use0.15s. This may be intentional to give the switch a slightly more pronounced feel, but consider aligning to0.15sfor consistency across the design system.Also applies to: 67-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles/themes/components/switch.scss` around lines 25 - 29, Update the switch component's transition durations to match the design system standard of 0.15s: change the transition declaration in switch.scss (the block that lists background-color, border-color, box-shadow, opacity) from 0.2s to 0.15s and also update the other transition block later in the file (the second transition occurrence) that currently uses 0.2s/0.22s so all switch transitions use 0.15s for consistency.styles/themes/components/select.scss (1)
160-176: Consider visual distinction between selected and selected+highlighted states.Both
[aria-selected="true"](line 160) and[aria-selected="true"][data-highlighted](line 166) usegray-300background. The only difference is the added inset border on the highlighted state. This subtle distinction may be intentional, but verify it provides sufficient visual feedback when navigating a selected item with keyboard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles/themes/components/select.scss` around lines 160 - 176, The selected state &[aria-selected="true"] and the selected+highlighted state &[aria-selected="true"][data-highlighted] currently share the same background (var(--rad-ui-color-gray-300)) making keyboard navigation feedback too subtle; update the highlighted state selector (&[aria-selected="true"][data-highlighted] and/or &[aria-selected="true"][data-active="true"]) to provide clearer visual distinction—for example use a slightly darker/lighter background token, stronger box-shadow, a distinct border color, or adjust color/opacity of .rad-ui-select-item-indicator—so the highlighted selected item is visually distinct from an un-highlighted selected item.styles/themes/components/radio-group.scss (1)
46-46: Consider addingbackground-colorto the transition.While the
border-colortransition provides smooth feedback, thebackground-coloralso changes on the checked state (line 59). Adding it to the transition would create a more polished visual experience.🎨 Proposed enhancement for smoother state transitions
- transition: border-color 0.15s ease; + transition: border-color 0.15s ease, background-color 0.15s ease;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@styles/themes/components/radio-group.scss` at line 46, Update the transition declaration in styles/themes/components/radio-group.scss to include background-color alongside border-color so the checked-state background changes animate smoothly; modify the existing "transition: border-color 0.15s ease;" entry (in the radio group option rule) to include "background-color 0.15s ease" (or a combined "transition: border-color 0.15s ease, background-color 0.15s ease;") to ensure both properties animate consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@styles/themes/components/progress.scss`:
- Around line 1-4: The header comment for .rad-ui-progress is inconsistent with
the CSS variable used: the comment says "step 10 (900)" but the indicator uses
--rad-ui-color-accent-800; either change the comment to "step 9 (800)" to match
the token, or update the indicator to use --rad-ui-color-accent-900 so the
comment remains "step 10 (900)"; update the header comment and/or the indicator
reference (--rad-ui-color-accent-800) accordingly to ensure they match.
In `@styles/themes/components/tab-nav.scss`:
- Line 7: The stylesheet comment wrongly references aria-current and lacks
styles for the tab active state; update styles/themes/components/tab-nav.scss to
add rules targeting [aria-selected="true"] (used by TabNavLink.tsx/TabNav
component) so selected tabs have distinct visual styles (e.g., different color,
background, border or underline) and remove or correct the misleading
"aria-current" comment; ensure the selector matches the element/type emitted by
TabNavLink (anchor/button) and mirrors the non-selected styling for
accessibility/contrast.
---
Nitpick comments:
In @.changeset/wet-corners-serve.md:
- Line 5: Update the changeset summary string "ui revamp for multiple comps" to
a more descriptive message that lists the affected components and the nature of
the change (for example: "Migrate Progress, Quote, Radio, Slider, Spinner,
Splitter and related components to Clarity design tokens and improve transition
styles"); edit the content in the .changeset entry that currently contains "ui
revamp for multiple comps" to this expanded description so consumers can
understand what changed and why.
In `@styles/themes/components/radio-group.scss`:
- Line 46: Update the transition declaration in
styles/themes/components/radio-group.scss to include background-color alongside
border-color so the checked-state background changes animate smoothly; modify
the existing "transition: border-color 0.15s ease;" entry (in the radio group
option rule) to include "background-color 0.15s ease" (or a combined
"transition: border-color 0.15s ease, background-color 0.15s ease;") to ensure
both properties animate consistently.
In `@styles/themes/components/select.scss`:
- Around line 160-176: The selected state &[aria-selected="true"] and the
selected+highlighted state &[aria-selected="true"][data-highlighted] currently
share the same background (var(--rad-ui-color-gray-300)) making keyboard
navigation feedback too subtle; update the highlighted state selector
(&[aria-selected="true"][data-highlighted] and/or
&[aria-selected="true"][data-active="true"]) to provide clearer visual
distinction—for example use a slightly darker/lighter background token, stronger
box-shadow, a distinct border color, or adjust color/opacity of
.rad-ui-select-item-indicator—so the highlighted selected item is visually
distinct from an un-highlighted selected item.
In `@styles/themes/components/spinner.scss`:
- Around line 40-45: Add a POSIX newline at the end of the file by ensuring the
last line (the closing brace of the &[data-spinner-size="large"] block in
spinner.scss) is followed by a single trailing newline character; update the
file so it ends with that newline to satisfy POSIX and diff cleanliness.
In `@styles/themes/components/switch.scss`:
- Around line 25-29: Update the switch component's transition durations to match
the design system standard of 0.15s: change the transition declaration in
switch.scss (the block that lists background-color, border-color, box-shadow,
opacity) from 0.2s to 0.15s and also update the other transition block later in
the file (the second transition occurrence) that currently uses 0.2s/0.22s so
all switch transitions use 0.15s for consistency.
In `@styles/themes/components/tab-nav.scss`:
- Around line 31-36: The combined selector "&:focus-visible, &:focus" in
tab-nav.scss applies focus styles on all focus events (including mouse), causing
unnecessary visual noise; update the rule to use only "&:focus-visible" so the
focus ring appears for keyboard navigation only, and if you need older-browser
fallback keep a separate, explicitly documented fallback block (e.g., a comment
or an opt-in variable) rather than applying the same styles to ":focus" by
default—modify the selector used in the tab nav focus rule accordingly.
🪄 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: 3f96b879-3cfe-45bc-8808-d847ccdfa463
📒 Files selected for processing (19)
.changeset/wet-corners-serve.mdstyles/themes/components/progress.scssstyles/themes/components/quote.scssstyles/themes/components/radio-group.scssstyles/themes/components/radio.scssstyles/themes/components/radiocards.scssstyles/themes/components/scroll-area.scssstyles/themes/components/select.scssstyles/themes/components/separator.scssstyles/themes/components/skeleton.scssstyles/themes/components/slider.scssstyles/themes/components/spinner.scssstyles/themes/components/splitter.scssstyles/themes/components/steps.scssstyles/themes/components/strong.scssstyles/themes/components/switch.scssstyles/themes/components/tab-nav.scssstyles/themes/components/text.scssstyles/themes/default.scss
| // Progress — Clarity design system aligned | ||
| // Track: step 3 (200) — subtle interactive surface, low visual weight | ||
| // Indicator: step 10 (900) — solid, accessible, high contrast fill | ||
| .rad-ui-progress { |
There was a problem hiding this comment.
Comment inconsistency: step number doesn't match token.
The header comment says "step 10 (900)" but the indicator uses --rad-ui-color-accent-800. Either update the comment to "step 9 (800)" or use --rad-ui-color-accent-900 if step 10 is intended.
Proposed fix
// Progress — Clarity design system aligned
// Track: step 3 (200) — subtle interactive surface, low visual weight
-// Indicator: step 10 (900) — solid, accessible, high contrast fill
+// Indicator: step 9 (800) — solid, accessible, high contrast fill📝 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.
| // Progress — Clarity design system aligned | |
| // Track: step 3 (200) — subtle interactive surface, low visual weight | |
| // Indicator: step 10 (900) — solid, accessible, high contrast fill | |
| .rad-ui-progress { | |
| // Progress — Clarity design system aligned | |
| // Track: step 3 (200) — subtle interactive surface, low visual weight | |
| // Indicator: step 9 (800) — solid, accessible, high contrast fill | |
| .rad-ui-progress { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@styles/themes/components/progress.scss` around lines 1 - 4, The header
comment for .rad-ui-progress is inconsistent with the CSS variable used: the
comment says "step 10 (900)" but the indicator uses --rad-ui-color-accent-800;
either change the comment to "step 9 (800)" to match the token, or update the
indicator to use --rad-ui-color-accent-900 so the comment remains "step 10
(900)"; update the header comment and/or the indicator reference
(--rad-ui-color-accent-800) accordingly to ensure they match.
| // Link text default: step 8 (700) — muted/supporting text | ||
| // Link hover bg: step 1 (50) — canvas bg (lifted feel) | ||
| // Link hover text: step 11 (1000) — primary text | ||
| // Link active/selected: handled by consumer via aria-current |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how other components handle aria-current/active styling to determine if this is intentional
# Search for aria-current patterns in component styles
rg -n 'aria-current' --type scss --type css -C3
# Search for data-active patterns
rg -n 'data-active' --type scss --type css -C3
# Check if TabNav component passes aria-current to links
rg -n 'aria-current' -g '*.tsx' -g '*.jsx' -C3Repository: rad-ui/ui
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Search for aria-current in all files (using glob patterns instead of --type)
echo "=== Searching for aria-current usage ==="
rg -n 'aria-current' -g '*.scss' -g '*.css' -g '*.tsx' -g '*.jsx' -C2
echo ""
echo "=== Searching for data-active usage ==="
rg -n 'data-active' -g '*.scss' -g '*.css' -g '*.tsx' -g '*.jsx' -C2
echo ""
echo "=== Checking TabNav component files ==="
fd -e tsx -e jsx 'tab' -i | head -20
echo ""
echo "=== Examining tab-nav.scss file ==="
cat -n styles/themes/components/tab-nav.scss
echo ""
echo "=== Examining navigation-menu.scss for comparison ==="
cat -n styles/themes/components/navigation-menu.scss 2>/dev/null || echo "File not found"Repository: rad-ui/ui
Length of output: 11073
🏁 Script executed:
#!/bin/bash
echo "=== TabNavLink.tsx - main link implementation ==="
cat -n src/components/ui/TabNav/fragments/TabNavLink.tsx
echo ""
echo "=== TabNav.tsx - main component ==="
cat -n src/components/ui/TabNav/TabNav.tsx
echo ""
echo "=== TabNav context to understand state management ==="
cat -n src/components/ui/TabNav/context/TabNav.context.tsx
echo ""
echo "=== TabNav tests to check expected behavior ==="
head -100 src/components/ui/TabNav/tests/TabNav.test.tsxRepository: rad-ui/ui
Length of output: 7264
Missing active state styling for selected tabs.
The TabNav component sets aria-selected={isActive} on links to indicate the active tab (line 48 in TabNavLink.tsx), but the tab-nav.scss stylesheet has no styling for this attribute. The comment on line 7 is misleading—it references "aria-current" but the component actually uses aria-selected. Without visual styling, users won't see which tab is currently selected.
Style the active tab state by adding:
Suggested fix
&:hover {
color: var(--rad-ui-color-gray-1000); // step 11 — primary text
background-color: var(--rad-ui-color-gray-50); // step 1 — canvas bg (lifted)
}
+
+ &[aria-selected="true"] {
+ background-color: var(--rad-ui-color-gray-50);
+ color: var(--rad-ui-color-gray-1000);
+ }
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@styles/themes/components/tab-nav.scss` at line 7, The stylesheet comment
wrongly references aria-current and lacks styles for the tab active state;
update styles/themes/components/tab-nav.scss to add rules targeting
[aria-selected="true"] (used by TabNavLink.tsx/TabNav component) so selected
tabs have distinct visual styles (e.g., different color, background, border or
underline) and remove or correct the misleading "aria-current" comment; ensure
the selector matches the element/type emitted by TabNavLink (anchor/button) and
mirrors the non-selected styling for accessibility/contrast.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Style