-
-
Notifications
You must be signed in to change notification settings - Fork 53
[New Component] Theme #1048
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[New Component] Theme #1048
Conversation
## Walkthrough
This update introduces a new `Theme` React component to manage theming and accent color via data attributes, replacing previous class-based and attribute-based selectors. The CSS and token generation scripts are updated to use `[data-rad-ui-theme="dark"]` for dark theme and `[data-rad-ui-accent-color=colorName]` for accent colors, standardizing attribute selectors across the codebase. The `SandboxEditor` now leverages the new `Theme` component for dynamic theming. Additionally, HSL color values in CSS are reformatted for consistency. Multiple UI components and their tests are updated to use the new `data-rad-ui-accent-color` attribute instead of the older `data-accent-color`. A new hook `useCreateDataAccentColorAttribute` is introduced to generate this attribute.
## Changes
| File(s) | Change Summary |
|-------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `src/components/ui/Theme/Theme.tsx` | Introduces a new `Theme` React component that manages theme and accent color via data attributes, supporting 'light', 'dark', and 'system' appearance modes. |
| `src/components/tools/SandboxEditor/SandboxEditor.tsx` | Refactors the root container to use the new `Theme` component, passing `appearance` and `accentColor` props, and removes previous class-based theming logic. |
| `scripts/tokenGen/generateAccentTokens.js`, `scripts/tokenGen/index.js` | Updates token generation scripts to use `[data-rad-ui-accent-color=...]` and `[data-rad-ui-theme="dark"]` attribute selectors instead of previous attribute/class-based selectors. |
| `styles/cssTokens/base.tokens.css` | Changes CSS accent color selectors to `[data-rad-ui-accent-color=colorName]`, updates dark theme selector to `[data-rad-ui-theme="dark"]`, and reformats HSL color values to use comma-separated parameters without degree units. |
| `src/core/hooks/createDataAttribute/index.ts` | Adds a new hook `useCreateDataAccentColorAttribute` that returns a memoized callback producing the `data-rad-ui-accent-color` attribute object based on the given color string. |
| Multiple UI components and their tests (e.g., `Avatar`, `Badge`, `BlockQuote`, `Button`, `Callout`, `Code`, `Progress`, `Separator`, `Switch`, `Tabs`, `Toggle`, `ToggleGroup`) | Change the data attribute key for accent colors from `data-accent-color` to `data-rad-ui-accent-color`. Components update imports to use the new hook `useCreateDataAccentColorAttribute` where applicable. Corresponding tests are updated to expect the new attribute name. |
| `src/core/hooks/createDataAttribute/createDataAttribute.test.tsx` | Updates tests to use `useCreateDataAccentColorAttribute` and expect the `data-rad-ui-accent-color` attribute key instead of the older `data-accent-color`. |
| `styles/themes/components/separator.scss` | Updates the CSS selector for accent color from `[data-accent-color]` to `[data-rad-ui-accent-color]` in the `.rad-ui-separator` style rule. |
| `package.json` | Adds export entry for the new `Theme` component: `"./Theme": "./dist/components/Theme.js"`. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant App
participant SandboxEditor
participant Theme
participant DOM
App->>SandboxEditor: Render SandboxEditor
SandboxEditor->>Theme: Render Theme(appearance, accentColor)
Theme->>Theme: Determine theme (light/dark/system)
Theme->>DOM: Set data-rad-ui-theme and data-rad-ui-accent-color attributes
Theme->>Theme: Render children inside themed containerPossibly related PRs
Suggested labels
Poem
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/tokenGen/generateAccentTokens.js (1)
4-19: Update comment documentation to match new selectorThe comment block describing how accent colors are generated still references the old
data-accent-colorattribute instead of the newdata-rad-ui-accent-colorattribute./** * This file generates base css files for accent colors like this -* [data-accent-color=purple]{ +* [data-rad-ui-accent-color=purple]{ --rad-ui-color-accent-50: var(--rad-ui-color-purple-50); --rad-ui-color-accent-100: var(--rad-ui-color-purple-100); --rad-ui-color-accent-200: var(--rad-ui-color-purple-200); --rad-ui-color-accent-300: var(--rad-ui-color-purple-300); --rad-ui-color-accent-400: var(--rad-ui-color-purple-400); --rad-ui-color-accent-500: var(--rad-ui-color-purple-500); --rad-ui-color-accent-600: var(--rad-ui-color-purple-600); --rad-ui-color-accent-700: var(--rad-ui-color-purple-700); --rad-ui-color-accent-800: var(--rad-ui-color-purple-800); --rad-ui-color-accent-900: var(--rad-ui-color-purple-900); --rad-ui-color-accent-950: var(--rad-ui-color-purple-950); --rad-ui-color-accent-1000: var(--rad-ui-color-purple-1000); } */styles/cssTokens/base.tokens.css (1)
377-810: Update accent attribute selectors todata-rad-ui-accent-colorwithout quotes
All accent-color selectors were renamed from[data-accent-color="…"]to[data-rad-ui-accent-color=…], dropping the quotes around values to match the new attribute name generated bygenerateAccentTokens.js. This aligns the CSS with the updated ReactThemecomponent’s data attributes.
Consider standardizing quoting (quoted vs. unquoted) across both accent and theme attribute selectors for consistent style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
scripts/tokenGen/generateAccentTokens.js(1 hunks)scripts/tokenGen/index.js(1 hunks)src/components/tools/SandboxEditor/SandboxEditor.tsx(3 hunks)src/components/ui/Theme/Theme.tsx(1 hunks)styles/cssTokens/base.tokens.css(33 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/components/tools/SandboxEditor/SandboxEditor.tsx (1)
docs/components/layout/Theme.js (1)
Theme(2-8)
scripts/tokenGen/generateAccentTokens.js (1)
scripts/tokenGen/generateCSSTokens.js (1)
cssVariableName(15-15)
src/components/ui/Theme/Theme.tsx (1)
docs/components/layout/Theme.js (1)
Theme(2-8)
🪛 GitHub Check: lint
src/components/ui/Theme/Theme.tsx
[warning] 12-12:
'setThemeAccentColor' is assigned a value but never used
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (8)
scripts/tokenGen/index.js (1)
28-28: Good change to standardize theming via data attributesThe selector change from
.rad-ui-dark-themeto[data-rad-ui-theme="dark"]improves consistency in the theming system by using data attributes instead of classes. This aligns with modern web component practices and makes the relationship between React props and CSS selectors more explicit.scripts/tokenGen/generateAccentTokens.js (1)
31-31: Consistent naming pattern for theming attributesChanging from
[data-accent-color=${colorObj}]to[data-rad-ui-accent-color=${colorObj}]provides a more consistent naming convention with thedata-rad-ui-themeattribute. Using a common prefix (data-rad-ui-) for related functionality makes the codebase more maintainable.src/components/tools/SandboxEditor/SandboxEditor.tsx (3)
9-10: Clean import of the new Theme componentGood job importing the Theme component from the UI components directory.
53-56: Excellent use of the Theme component for consistent themingReplacing the container div with the new Theme component and passing the appropriate appearance and accent color props is a good refactoring. This centralizes theming logic and makes the component's behavior more declarative.
101-101: Proper closure of the Theme componentThe closing tag has been correctly updated to match the opening tag.
src/components/ui/Theme/Theme.tsx (1)
1-26: Well-implemented theming component with system preference supportThe Theme component is well structured with a clear API. I especially like the support for system color preference detection. The use of data attributes for theme and accent color makes it easy to target styles using CSS.
🧰 Tools
🪛 GitHub Check: lint
[warning] 12-12:
'setThemeAccentColor' is assigned a value but never usedstyles/cssTokens/base.tokens.css (2)
3-158: Reformat HSL function syntax in :root for broad CSS compatibility
Allhsl()declarations have been converted from the CSS Color 4 space-separated syntax (e.g.,hsl(0deg 0% 99.0%)) to the legacy comma-separated syntax (hsl(0, 0%, 99.0%)). This ensures consistent rendering across all browsers without altering any color values.
813-969: Reformat HSL syntax for dark theme section
The dark-theme variables under[data-rad-ui-theme="dark"]have been updated to the comma-separated HSL syntax, mirroring the root scope changes and ensuring uniform CSS support across browsers.
…CreateDataAccentColorAttribute hook- Add color tokens for various shades- Update color tokens for different shades
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/ui/Callout/fragments/CalloutRoot.tsx (1)
32-32: Remove redundant data attribute assignmentThe accent color data attribute is being set in two places: once in the
data_attributesobject (line 29) and again directly on the div element. This causes duplicate attribute assignment when color is defined.- return <div className={clsx(rootClass, className)} data-rad-ui-accent-color={color ?? undefined} {...data_attributes} {...props}> + return <div className={clsx(rootClass, className)} {...data_attributes} {...props}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
src/components/ui/Avatar/Avatar.tsx(2 hunks)src/components/ui/Avatar/tests/Avatar.test.tsx(1 hunks)src/components/ui/AvatarGroup/tests/AvatarGroup.test.js(1 hunks)src/components/ui/Badge/fragments/BadgeRoot.tsx(1 hunks)src/components/ui/Badge/tests/Badge.test.tsx(1 hunks)src/components/ui/BlockQuote/BlockQuote.tsx(1 hunks)src/components/ui/BlockQuote/tests/BlockQuote.test.tsx(1 hunks)src/components/ui/Button/Button.tsx(2 hunks)src/components/ui/Button/tests/Button.test.tsx(1 hunks)src/components/ui/Callout/fragments/CalloutRoot.tsx(1 hunks)src/components/ui/Code/Code.tsx(1 hunks)src/components/ui/Code/tests/Code.test.tsx(1 hunks)src/components/ui/Progress/fragments/ProgressIndicator.tsx(1 hunks)src/components/ui/Progress/tests/Progress.test.tsx(1 hunks)src/components/ui/Separator/Separator.tsx(1 hunks)src/components/ui/Separator/tests/Separator.test.tsx(1 hunks)src/components/ui/Switch/Switch.tsx(2 hunks)src/components/ui/Tabs/fragments/TabRoot.tsx(1 hunks)src/components/ui/Theme/Theme.tsx(1 hunks)src/components/ui/Toggle/Toggle.tsx(1 hunks)src/components/ui/Toggle/tests/Toggle.test.js(1 hunks)src/components/ui/ToggleGroup/fragments/ToggleGroupRoot.tsx(1 hunks)src/components/ui/ToggleGroup/tests/ToggleGroup.test.js(1 hunks)src/core/hooks/createDataAttribute/createDataAttribute.test.tsx(4 hunks)src/core/hooks/createDataAttribute/index.ts(1 hunks)src/core/primitives/Avatar/fragments/AvatarPrimitiveFallback.tsx(1 hunks)styles/themes/components/separator.scss(1 hunks)
✅ Files skipped from review due to trivial changes (9)
- src/components/ui/ToggleGroup/fragments/ToggleGroupRoot.tsx
- src/components/ui/Badge/fragments/BadgeRoot.tsx
- src/components/ui/Code/tests/Code.test.tsx
- src/components/ui/Separator/Separator.tsx
- src/components/ui/Code/Code.tsx
- src/components/ui/Toggle/Toggle.tsx
- src/components/ui/Tabs/fragments/TabRoot.tsx
- src/core/primitives/Avatar/fragments/AvatarPrimitiveFallback.tsx
- src/components/ui/AvatarGroup/tests/AvatarGroup.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ui/Theme/Theme.tsx
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/components/ui/Switch/Switch.tsx (1)
src/core/hooks/createDataAttribute/index.ts (1)
useCreateDataAccentColorAttribute(48-55)
src/components/ui/Button/Button.tsx (1)
src/core/hooks/createDataAttribute/index.ts (1)
useCreateDataAccentColorAttribute(48-55)
src/components/ui/Avatar/Avatar.tsx (1)
src/core/hooks/createDataAttribute/index.ts (2)
useCreateDataAttribute(10-25)useCreateDataAccentColorAttribute(48-55)
src/core/hooks/createDataAttribute/createDataAttribute.test.tsx (1)
src/core/hooks/createDataAttribute/index.ts (1)
useCreateDataAccentColorAttribute(48-55)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (24)
src/core/hooks/createDataAttribute/index.ts (1)
42-55: Well-implemented hook for standardized accent color attributeThe new
useCreateDataAccentColorAttributehook follows best practices with proper memoization, clear naming, and comprehensive documentation. This addition supports the standardization of accent color attribution across components.src/components/ui/Callout/fragments/CalloutRoot.tsx (1)
29-29: LGTM: Data attribute name updated correctlyThe data attribute has been properly renamed to follow the new standardized naming convention.
src/components/ui/Progress/tests/Progress.test.tsx (1)
35-35: Test correctly updated for new attribute naming conventionThe test has been properly updated to check for the new standardized data attribute name.
src/components/ui/Avatar/tests/Avatar.test.tsx (1)
38-38: Test correctly updated for new attribute naming conventionThe test has been properly updated to check for the new standardized data attribute name.
src/components/ui/BlockQuote/tests/BlockQuote.test.tsx (1)
32-32: Attribute name updated correctly todata-rad-ui-accent-colorThe test has been properly updated to verify the new standardized attribute name for accent colors. This change aligns with the PR objective of standardizing attribute selectors across the codebase.
src/components/ui/Badge/tests/Badge.test.tsx (1)
32-32: Attribute name updated correctly todata-rad-ui-accent-colorThe test has been properly updated to verify the new standardized attribute name for accent colors. This change is consistent with the updates in other component tests.
src/components/ui/Button/tests/Button.test.tsx (1)
22-22: Attribute name updated correctly todata-rad-ui-accent-colorThe test has been properly updated to verify the new standardized attribute name for accent colors. This change maintains consistency with the new theming approach across components.
src/components/ui/BlockQuote/BlockQuote.tsx (1)
31-31: Attribute name updated correctly todata-rad-ui-accent-colorThe component implementation has been updated to use the new standardized data attribute for accent colors. This change ensures the component works properly with the new Theme component and maintains consistency with other UI components.
src/components/ui/Toggle/tests/Toggle.test.js (1)
82-84: Correctly updated data attribute name for themingThe test now checks for the new standardized
data-rad-ui-accent-colorattribute instead of the olddata-accent-color, aligning with the broader update to standardize attribute selectors across the codebase.src/components/ui/Progress/fragments/ProgressIndicator.tsx (1)
26-28: Correctly updated data attribute name for color themingThe component now uses the standardized
data-rad-ui-accent-colorattribute instead of the olddata-accent-color, maintaining the same conditional logic but ensuring consistency with the new attribute naming convention.src/components/ui/Separator/tests/Separator.test.tsx (1)
26-28: Test updated to use standardized data attribute nameThe test now correctly checks for the new
data-rad-ui-accent-colorattribute instead of the olddata-accent-color, keeping the test aligned with the component implementation changes.src/components/ui/ToggleGroup/tests/ToggleGroup.test.js (1)
96-100: Test updated to use standardized data attribute nameThe test now correctly checks for the new
data-rad-ui-accent-colorattribute instead of the olddata-accent-color, ensuring the test remains in sync with the component's implementation changes.src/components/ui/Switch/Switch.tsx (2)
5-5: Import updated to use specialized accent color attribute hookUpdated to include the new
useCreateDataAccentColorAttributehook for creating standardized accent color data attributes.
33-33: Standardized accent color attribute implementationChanged from generic
useCreateDataAttribute('accent', { color })to the specializeduseCreateDataAccentColorAttribute(color)hook, which generates the standardizeddata-rad-ui-accent-colorattribute instead ofdata-accent-color.styles/themes/components/separator.scss (1)
7-7: Updated CSS selector to use standardized attribute nameThe CSS selector has been updated from
[data-accent-color]to[data-rad-ui-accent-color]to align with the standardized data attribute naming convention being implemented across the codebase.src/components/ui/Button/Button.tsx (2)
6-6: Import updated to use specialized accent color attribute hookUpdated to include the new
useCreateDataAccentColorAttributehook for creating standardized accent color data attributes.
23-23: Standardized accent color attribute implementationChanged from generic
useCreateDataAttribute('accent', { color })to the specializeduseCreateDataAccentColorAttribute(color)hook, which generates the standardizeddata-rad-ui-accent-colorattribute instead ofdata-accent-color.src/components/ui/Avatar/Avatar.tsx (3)
4-4: Import updated to use specialized accent color attribute hookUpdated to include the new
useCreateDataAccentColorAttributehook for creating standardized accent color data attributes.
22-22: Added default value for color propAdded an empty string as the default value for the
colorprop to ensure consistent behavior when no color is provided.
24-24: Standardized accent color attribute implementationChanged from generic
useCreateDataAttribute('accent', { color })to the specializeduseCreateDataAccentColorAttribute(color)hook, which generates the standardizeddata-rad-ui-accent-colorattribute instead ofdata-accent-color.src/core/hooks/createDataAttribute/createDataAttribute.test.tsx (4)
2-2: Clean import addition.Good job adding the new
useCreateDataAccentColorAttributehook import while maintaining the existing imports.
14-16: Good replacement of generic hook with specialized hook.Nice work replacing the generic
useCreateDataAttributewith the more specificuseCreateDataAccentColorAttribute. This change aligns with the standardization efforts and makes the intent clearer.
27-27: Correct update of attribute name.The expected attribute name has been properly updated to
'data-rad-ui-accent-color'to match the new naming convention, maintaining test validity.
41-43: Consistent usage of new hook for edge cases.Good job updating the empty string test case to use the new specialized hook. This ensures that edge cases continue to be properly tested with the new implementation.
…ist/components/Theme.js"
Summary by CodeRabbit
New Features
Style
Refactor
Chores