Test title pull request #209
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a comprehensive design system for the project, focusing on creating a robust and flexible styling infrastructure. The changes include the development of a new design system package with scripts for generating color tokens, SCSS styles, and Tailwind CSS configurations. A new GitHub Actions workflow is added to automate the build process, and the project's styling approach is migrated from CSS to SCSS, leveraging more advanced styling capabilities. Changes
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions
participant Build as Build Script
participant Tokens as Token Generator
participant Styles as Style Generator
participant Tailwind as Tailwind Config Generator
GHA->>Build: Trigger build process
Build->>Tokens: Generate color tokens
Tokens-->>Build: Return color token data
Build->>Styles: Generate SCSS styles
Styles-->>Build: Return style files
Build->>Tailwind: Generate Tailwind configuration
Tailwind-->>Build: Return config files
Build->>GHA: Complete build process
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (18)
packages/design-system/src/styles/base/_palette.scss (1)
76-76: Maintain consistent naming convention for color variables.The variable
--gray-alibreaks the established naming pattern of--{color}-{shade}. Consider renaming it to follow the same convention, e.g.,--gray-lightor including it in the numbered scale.- --gray-ali: 210 20% 98%; + --gray-light: 210 20% 98%;packages/design-system/src/scripts/build-main-style.ts (2)
1-6: Consider using path.resolve and adding path validation.While the current path construction works, consider these improvements for better reliability:
- Use
path.resolveinstead ofpath.joinfor absolute path resolution- Add path validation to ensure the base directory exists
-const outputDir = path.join(__dirname, "../styles"); -const outputFilePath = path.join(outputDir, "main.scss"); +const outputDir = path.resolve(__dirname, "../styles"); +const outputFilePath = path.resolve(outputDir, "main.scss"); + +// Validate base directory exists +const baseDir = path.resolve(__dirname, ".."); +if (!fs.existsSync(baseDir)) { + throw new Error(`Base directory does not exist: ${baseDir}`); +}
31-32: Improve script execution with proper module checks.The script is directly executed without checking if it's being run as a module or directly.
-// Run the script -buildMainScss(); +// Only run the script if it's being executed directly +if (require.main === module) { + buildMainScss(); +}packages/design-system/src/styles/base/_colors-light.scss (3)
5-11: Consider grouping related color variables together.The color variables could be better organized by grouping related properties together (e.g., all primary-related variables, all accent-related variables).
Here's a suggested reorganization for better maintainability:
.light { + // Brand colors + --primary: var(--purple-500); + --primary-foreground: var(--General-white); + --primary-light: var(--purple-100); + --secondary: var(--zinc-100); + --secondary-foreground: var(--zinc-900); + + // Base UI colors --accent: var(--zinc-100); --accent-foreground: var(--zinc-900); --background: var(--General-white); --border: var(--zinc-200); - --card: var(--zinc-100); - --card-foreground: var(--slate-900); - --destructive: var(--red-600); - --destructive-foreground: var(--General-white); - // ... rest of the variables - --primary: var(--purple-500); - --primary-foreground: var(--General-white); - --primary-light: var(--purple-100); - --secondary: var(--zinc-100); - --secondary-foreground: var(--zinc-900);Also applies to: 25-30
34-34: Consider using CSS color-mix() for table header transparency.Instead of using rgba, consider using the modern
color-mix()function for better maintainability and consistency with the design system.- --table-header: rgba(244, 244, 245, 0.5000); + --table-header: color-mix(in srgb, var(--zinc-100) 50%, transparent);
1-39: Consider adding documentation for color usage.While the variable names are descriptive, adding comments to document the intended usage of each color variable would help other developers understand when to use specific colors (e.g., when to use
--accentvs--secondary).Consider adding JSDoc-style comments above each logical group:
/** Brand colors - Used for primary actions and brand identity */ --primary: var(--purple-500); /** Interface colors - Used for general UI elements */ --accent: var(--zinc-100); /** State colors - Used for feedback and system status */ --error: var(--red-500);packages/design-system/src/tailwind/config.ts (1)
1-2: Consider adding type definitions for imported modules.Adding TypeScript interfaces for
colorsandpalettewould improve type safety and documentation.interface ColorDefinitions { [key: string]: string | Record<string, string>; } import { colors } from "./colors"; import { palette } from "./palette";packages/design-system/src/scripts/utills.ts (1)
1-1: Fix typo in filename: "utills.ts" should be "utils.ts".The filename contains a spelling error. Consider renaming it to maintain consistency with common naming conventions.
packages/design-system/src/scripts/build.ts (1)
32-32: Consider making the script more configurable.Add command-line arguments to:
- Select specific build steps to run
- Configure output paths
- Set build modes (development/production)
packages/ui/tailwind.config.ts (1)
54-57: Consider using import statements for plugins.Instead of using
require(), consider using ES imports for consistency with the rest of the file.- require("tailwind-scrollbar"), - require("tailwind-scrollbar-hide"), + import("tailwind-scrollbar"), + import("tailwind-scrollbar-hide"),packages/design-system/src/tailwind/colors.ts (1)
1-35: Consider grouping related colors using TypeScript types.While the color definitions are comprehensive, they could be better organized using TypeScript types to group related colors (e.g., primary colors, status colors, etc.).
type StatusColors = { error: string; "error-border": string; "error-foreground": string; warning: string; "warning-border": string; "warning-foreground": string; success: string; "success-border": string; "success-foreground": string; information: string; "information-border": string; "information-foreground": string; }; type ThemeColors = { primary: string; "primary-foreground": string; "primary-light": string; secondary: string; "secondary-foreground": string; // ... other theme colors }; export const colors: StatusColors & ThemeColors = { // ... color definitions };packages/design-system/src/scripts/build-colors-tailwind.ts (1)
13-30: Add TypeScript interfaces for better type safety.Define interfaces for the token structure to improve type safety and documentation.
interface ColorToken { $value: string; } interface TokenData { color: Record<string, ColorToken>; } const generateColorsTsContent = (tokens: TokenData): string => {packages/design-system/src/tailwind/palette.ts (1)
2-5: Use consistent casing for category names.The "General" category uses PascalCase while other categories use camelCase. Maintain consistency by using camelCase throughout.
- "General": { - "black": "var(--General-black)", - "white": "var(--General-white)", + "general": { + "black": "var(--general-black)", + "white": "var(--general-white)", },.github/workflows/build-design-system.yml (2)
26-30: Add dependency caching to improve workflow performance.Add caching for Bun dependencies to speed up the workflow execution.
+ # Add caching step before installing dependencies + - name: Cache dependencies + uses: actions/cache@v3 + with: + path: ~/.bun/install/cache + key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }} + restore-keys: | + ${{ runner.os }}-bun- + - name: Install dependencies working-directory: ./packages/design-system run: bun install
17-18: Update checkout action to latest version.The checkout action can be updated to v4 for latest features and security fixes.
- name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4packages/ui/src/styles/globals.scss (1)
222-232: Add documentation for the no-scrollbar utility class.Consider adding a brief comment explaining when and how to use this utility class.
@layer utilities { + /** + * Use the no-scrollbar class to hide scrollbars while maintaining scroll functionality. + * Example: <div class="no-scrollbar overflow-auto">...</div> + */ /* Hide scrollbar for Chrome, Safari and Opera */ .no-scrollbar::-webkit-scrollbar {packages/design-system/src/styles/base/_colors-dark.scss (2)
1-1: Consider using an explicit namespace for better maintainability.While using
@useis the correct modern approach, usingas *could lead to naming collisions. Consider using a descriptive namespace like@use "_palette.scss" as palette;for better clarity and maintainability.-@use "_palette.scss" as *; +@use "_palette.scss" as palette;
34-34: Maintain consistency in color value definitions.The
--table-headervalue uses a direct rgba color instead of referencing a palette variable like other properties. Consider using a palette variable for consistency.- --table-header: rgba(39, 39, 42, 0.5000); + --table-header: var(--zinc-900-50);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (31)
.github/workflows/build-design-system.yml(1 hunks).github/workflows/update-leaderboard.yml(0 hunks).github/workflows/updateLeaderboard.js(0 hunks).hintrc(1 hunks)apps/core/app/auth/layout.tsx(1 hunks)apps/core/app/layout.tsx(1 hunks)packages/design-system/package.json(1 hunks)packages/design-system/src/scripts/build-colors-style.ts(1 hunks)packages/design-system/src/scripts/build-colors-tailwind.ts(1 hunks)packages/design-system/src/scripts/build-config-tailwind.ts(1 hunks)packages/design-system/src/scripts/build-main-style.ts(1 hunks)packages/design-system/src/scripts/build-palette-style.ts(1 hunks)packages/design-system/src/scripts/build-palette-tailwind.ts(1 hunks)packages/design-system/src/scripts/build.ts(1 hunks)packages/design-system/src/scripts/utills.ts(1 hunks)packages/design-system/src/styles/base/_colors-dark.scss(1 hunks)packages/design-system/src/styles/base/_colors-light.scss(1 hunks)packages/design-system/src/styles/base/_palette.scss(1 hunks)packages/design-system/src/styles/main.scss(1 hunks)packages/design-system/src/tailwind/colors.ts(1 hunks)packages/design-system/src/tailwind/config.ts(1 hunks)packages/design-system/src/tailwind/palette.ts(1 hunks)packages/design-system/src/tokens/token_Palette_primitive.json(1 hunks)packages/design-system/tsconfig.json(1 hunks)packages/design-system/tsconfig.lint.json(1 hunks)packages/ui/package.json(4 hunks)packages/ui/postcss.config.mjs(1 hunks)packages/ui/src/globals.css(0 hunks)packages/ui/src/styles/_tailinwd.scss(1 hunks)packages/ui/src/styles/globals.scss(1 hunks)packages/ui/tailwind.config.ts(3 hunks)
💤 Files with no reviewable changes (3)
- .github/workflows/updateLeaderboard.js
- .github/workflows/update-leaderboard.yml
- packages/ui/src/globals.css
✅ Files skipped from review due to trivial changes (8)
- packages/design-system/tsconfig.json
- packages/design-system/src/styles/main.scss
- apps/core/app/auth/layout.tsx
- packages/design-system/tsconfig.lint.json
- packages/ui/src/styles/_tailinwd.scss
- packages/design-system/package.json
- packages/design-system/src/tokens/token_Palette_primitive.json
- apps/core/app/layout.tsx
🧰 Additional context used
🪛 GitHub Actions: Build Design System
packages/design-system/src/scripts/build-palette-tailwind.ts
[error] 36-36: File not found: Unable to read '/home/runner/work/pixel-client/pixel-client/packages/design-system/src/tokens/token_Palette_Primitive.json'. The required input file is missing.
🔇 Additional comments (17)
.hintrc (1)
5-7: Please clarify the reason for disabling consistent-casing.Disabling the
typescript-config/consistent-casingrule could lead to inconsistent naming conventions across the codebase. If there's a specific reason for disabling this rule, please document it in the PR description.Let's check the TypeScript files in the codebase for potential casing inconsistencies that might have prompted this change:
✅ Verification successful
Disabling consistent-casing rule is appropriate for this codebase.
The codebase follows standard TypeScript/React naming conventions:
- PascalCase for React components
- camelCase for variables, functions, and hooks
- kebab-case for file names
Disabling this rule allows these established conventions to coexist without triggering false positives.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for inconsistent casing in TypeScript files # This will help understand if there are legitimate cases where we need different casing styles # Search for different casing patterns in TypeScript files echo "Checking for different casing patterns in TypeScript files..." echo "PascalCase:" rg -g '*.ts' -g '*.tsx' '[A-Z][a-z0-9]+(?:[A-Z][a-z0-9]+)*' --count echo "\nCamelCase:" rg -g '*.ts' -g '*.tsx' '[a-z]+(?:[A-Z][a-z0-9]+)*' --count echo "\nKebab-case:" rg -g '*.ts' -g '*.tsx' '[a-z]+(?:-[a-z0-9]+)*' --count echo "\nSnake_case:" rg -g '*.ts' -g '*.tsx' '[a-z]+(?:_[a-z0-9]+)*' --count # Look for any existing comments about casing conventions echo "\nChecking for comments about casing conventions..." rg -g '*.ts' -g '*.tsx' -i 'case|naming|convention' -C 2 '//|/\*|\*'Length of output: 54319
packages/design-system/src/styles/base/_palette.scss (3)
1-4: LGTM! Well-structured base layer declaration.The use of @layer for base styles and :root scope is correct. The General colors are appropriately defined as the foundation.
1-270: LGTM! Well-organized color system structure.The color system demonstrates:
- Consistent alphabetical ordering
- Clear visual separation between color groups
- Uniform HSL format usage
1-270: Verify the build process integration.Since this file is generated by
build-palette-style.ts, let's verify the build process and integration.✅ Verification successful
Build process verification successful
The palette.scss file is correctly generated from design tokens through a well-structured build process. The implementation properly converts color values to HSL format and integrates with the design system's color architecture.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the build script and its dependencies echo "Checking build script and related files..." rg -l "build-palette-style" . fd "build-palette-style.ts" # Verify color token definitions echo "Checking color token definitions..." fd -e json -e ts "palette|colors|tokens" packages/design-system/srcLength of output: 799
Script:
#!/bin/bash # Check the token definitions and build script implementation echo "=== Token Definitions ===" cat packages/design-system/src/tokens/token_Palette_primitive.json echo -e "\n=== Build Script Implementation ===" cat packages/design-system/src/scripts/build-palette-style.ts echo -e "\n=== Related Build Scripts ===" cat packages/design-system/src/scripts/build-colors-style.tsLength of output: 21738
packages/design-system/src/scripts/build-main-style.ts (1)
8-14: Validate SCSS dependencies and consider explicit namespacing.The current implementation has two potential issues:
- No validation of required SCSS files
- Using
as *could lead to naming conflicts between dark and light themesConsider this safer approach:
const generateMainScssContent = (): string => { return ` -@use "./base/_colors-dark.scss" as *; -@use "./base/_colors-light.scss" as *; +@use "./base/_colors-dark.scss" as dark; +@use "./base/_colors-light.scss" as light; `.trim(); };packages/design-system/src/styles/base/_colors-light.scss (3)
3-4: LGTM! Good use of CSS layers.Using
@layer baseis a good practice for managing style specificity and organizing the CSS cascade.
11-16: Review the consistency of state colors.The state indicators (error, warning, success) follow different patterns:
- Error uses 500/200/600
- Success uses 100/200/700
- Warning uses 500/200/white
Consider standardizing the color intensity patterns across states for better consistency.
Run this script to analyze color usage patterns:
Also applies to: 31-37
✅ Verification successful
Color intensity patterns show intentional consistency
The analysis reveals that the color system follows a consistent pattern across states:
- Light variants (100-200) for backgrounds and borders
- Mid variants (500-600) for primary indicators
- Dark variants (700+) for emphasis and contrast
The slight variations in specific intensities (e.g., 500 vs 600) appear intentional for achieving proper contrast ratios and visual hierarchy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for consistent color intensity patterns across the codebase rg --no-filename -o "var\(--(red|green|amber)-[0-9]+\)" | sort | uniq -cLength of output: 874
1-1: Verify the palette import path and consider namespace.The import statement uses a relative path. While this works, consider using a namespace for better clarity when using imported values.
-@use "_palette.scss" as *; +@use "_palette.scss" as palette;Run this script to verify the import path:
packages/design-system/src/tailwind/config.ts (1)
7-7: Verify potential color name conflicts.When spreading both
colorsandpalette, ensure there are no naming conflicts that could cause unintended overwrites.✅ Verification successful
No color name conflicts found - spreading is safe ✅
The spread operation is safe as the objects use different namespacing:
colors: Semantic tokens (e.g.,accent,primary)palette: Color scales nested under color names (e.g.,amber.50,blue.100)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find potential color name conflicts between colors and palette ast-grep --pattern 'export const colors = { $$$ }' | grep -o '"[^"]*"' > /tmp/color_keys.txt ast-grep --pattern 'export const palette = { $$$ }' | grep -o '"[^"]*"' > /tmp/palette_keys.txt # Find duplicates sort /tmp/color_keys.txt /tmp/palette_keys.txt | uniq -dLength of output: 274
Script:
#!/bin/bash # Get the full content of both color objects echo "=== Colors Object ===" ast-grep --pattern 'export const colors = { $$$ }' echo -e "\n=== Palette Object ===" ast-grep --pattern 'export const palette = { $$$ }'Length of output: 26144
packages/ui/tailwind.config.ts (1)
3-6: LGTM! Good refactoring to use centralized design system config.The change to use the design system's Tailwind configuration as a preset helps maintain consistency across the project.
packages/design-system/src/scripts/build-palette-tailwind.ts (1)
4-6:⚠️ Potential issueFix the missing token file issue.
The pipeline is failing because
token_Palette_Primitive.jsonis missing. Ensure this file is committed to the repository.packages/ui/postcss.config.mjs (1)
3-3: LGTM! PostCSS configuration updated for SCSS support.The addition of SCSS syntax support aligns well with the project's migration to SCSS-based styling.
packages/ui/package.json (3)
27-27: LGTM! Dependencies updated for SCSS support.The addition of
@repo/design-systemandpostcss-scssaligns with the new styling infrastructure.Also applies to: 66-66
81-81: LGTM! Export path updated for SCSS.The export path has been correctly updated to reflect the migration from CSS to SCSS.
72-72: Verify the sass package version.The specified version
1.83.4is beyond my knowledge cutoff date. Please verify that this is the latest stable version without known security vulnerabilities.✅ Verification successful
The sass package version 1.83.4 is verified as secure and up-to-date
The specified version is confirmed to be the latest available release and has no known security vulnerabilities in the npm advisory database.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for the latest version of sass and any security advisories npm view sass versions --json | jq -r '.[-1]' npm audit [email protected]Length of output: 427
Script:
#!/bin/bash # Query npm advisory database for sass package vulnerabilities curl -s "https://registry.npmjs.org/-/npm/v1/security/advisories/search?package=sass" | jq '.advisories'Length of output: 109
packages/ui/src/styles/globals.scss (1)
212-219: LGTM! Base styles properly configured.The base layer correctly applies border styles and sets background/text colors using Tailwind's @apply directive.
packages/design-system/src/styles/base/_colors-dark.scss (1)
3-4: Well-structured base layer implementation!Good use of
@layer basefor proper cascade layering and:rootfor global scope definition.
| --purple-50: 251 62% 95%; | ||
| --purple-100: 250 61% 90%; | ||
| --purple-200: 250 63% 80%; | ||
| --purple-300: 250 62% 70%; | ||
| --purple-400: 250 62% 60%; | ||
| --purple-500: 250 62% 58%; | ||
| --purple-600: 250 62% 40%; | ||
| --purple-700: 250 62% 30%; | ||
| --purple-800: 250 63% 20%; | ||
| --purple-900: 250 61% 10%; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing shade to complete the purple scale.
The purple color scale is missing the 950 shade that is present in all other color scales. This inconsistency could affect the design system's completeness.
--purple-800: 250 63% 20%;
--purple-900: 250 61% 10%;
+ --purple-950: 250 62% 5%;📝 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.
| --purple-50: 251 62% 95%; | |
| --purple-100: 250 61% 90%; | |
| --purple-200: 250 63% 80%; | |
| --purple-300: 250 62% 70%; | |
| --purple-400: 250 62% 60%; | |
| --purple-500: 250 62% 58%; | |
| --purple-600: 250 62% 40%; | |
| --purple-700: 250 62% 30%; | |
| --purple-800: 250 63% 20%; | |
| --purple-900: 250 61% 10%; | |
| --purple-50: 251 62% 95%; | |
| --purple-100: 250 61% 90%; | |
| --purple-200: 250 63% 80%; | |
| --purple-300: 250 62% 70%; | |
| --purple-400: 250 62% 60%; | |
| --purple-500: 250 62% 58%; | |
| --purple-600: 250 62% 40%; | |
| --purple-700: 250 62% 30%; | |
| --purple-800: 250 63% 20%; | |
| --purple-900: 250 61% 10%; | |
| --purple-950: 250 62% 5%; |
| // Function to create the main.scss file | ||
| export const buildMainScss = (): void => { | ||
| // Generate the content | ||
| const scssContent = generateMainScssContent(); | ||
|
|
||
| // Ensure the output directory exists | ||
| if (!fs.existsSync(outputDir)) { | ||
| fs.mkdirSync(outputDir, { recursive: true }); | ||
| } | ||
|
|
||
| // Write the content to the file | ||
| fs.writeFileSync(outputFilePath, scssContent, "utf-8"); | ||
| console.log(`main.scss file created at: ${outputFilePath}`); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add robust error handling and improved logging.
The current implementation lacks proper error handling and logging capabilities.
export const buildMainScss = (): void => {
+ try {
// Generate the content
const scssContent = generateMainScssContent();
// Ensure the output directory exists
if (!fs.existsSync(outputDir)) {
+ console.log(`Creating directory: ${outputDir}`);
fs.mkdirSync(outputDir, { recursive: true });
}
+ // Check if file exists
+ if (fs.existsSync(outputFilePath)) {
+ console.log(`Warning: Overwriting existing file: ${outputFilePath}`);
+ }
+
// Write the content to the file
fs.writeFileSync(outputFilePath, scssContent, "utf-8");
console.log(`main.scss file created at: ${outputFilePath}`);
+ } catch (error) {
+ console.error('Failed to build main.scss:', error);
+ process.exit(1);
+ }
};📝 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.
| // Function to create the main.scss file | |
| export const buildMainScss = (): void => { | |
| // Generate the content | |
| const scssContent = generateMainScssContent(); | |
| // Ensure the output directory exists | |
| if (!fs.existsSync(outputDir)) { | |
| fs.mkdirSync(outputDir, { recursive: true }); | |
| } | |
| // Write the content to the file | |
| fs.writeFileSync(outputFilePath, scssContent, "utf-8"); | |
| console.log(`main.scss file created at: ${outputFilePath}`); | |
| }; | |
| // Function to create the main.scss file | |
| export const buildMainScss = (): void => { | |
| try { | |
| // Generate the content | |
| const scssContent = generateMainScssContent(); | |
| // Ensure the output directory exists | |
| if (!fs.existsSync(outputDir)) { | |
| console.log(`Creating directory: ${outputDir}`); | |
| fs.mkdirSync(outputDir, { recursive: true }); | |
| } | |
| // Check if file exists | |
| if (fs.existsSync(outputFilePath)) { | |
| console.log(`Warning: Overwriting existing file: ${outputFilePath}`); | |
| } | |
| // Write the content to the file | |
| fs.writeFileSync(outputFilePath, scssContent, "utf-8"); | |
| console.log(`main.scss file created at: ${outputFilePath}`); | |
| } catch (error) { | |
| console.error('Failed to build main.scss:', error); | |
| process.exit(1); | |
| } | |
| }; |
| export const hexToHsl = (hex: string): string => { | ||
| const r = parseInt(hex.slice(1, 3), 16) / 255; | ||
| const g = parseInt(hex.slice(3, 5), 16) / 255; | ||
| const b = parseInt(hex.slice(5, 7), 16) / 255; | ||
|
|
||
| const max = Math.max(r, g, b); | ||
| const min = Math.min(r, g, b); | ||
| const delta = max - min; | ||
|
|
||
| let h = 0; | ||
| if (delta !== 0) { | ||
| if (max === r) { | ||
| h = ((g - b) / delta + (g < b ? 6 : 0)) * 60; | ||
| } else if (max === g) { | ||
| h = ((b - r) / delta + 2) * 60; | ||
| } else { | ||
| h = ((r - g) / delta + 4) * 60; | ||
| } | ||
| } | ||
|
|
||
| const l = (max + min) / 2; | ||
| const s = delta === 0 ? 0 : delta / (1 - Math.abs(2 * l - 1)); | ||
|
|
||
| return `${Math.round(h)} ${Math.round(s * 100)}% ${Math.round(l * 100)}%`; | ||
| }; No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and support for different hex formats.
The current implementation has several potential issues:
- No validation for hex color format
- Doesn't support shorthand hex (#RGB)
- No error handling for invalid inputs
Consider this improved implementation:
export const hexToHsl = (hex: string): string => {
+ // Validate hex format
+ const hexRegex = /^#([A-Fa-f0-9]{3}|[A-Fa-f0-9]{6})$/;
+ if (!hexRegex.test(hex)) {
+ throw new Error('Invalid hex color format');
+ }
+
+ // Normalize hex to 6 digits
+ const normalizedHex = hex.length === 4
+ ? `#${hex[1]}${hex[1]}${hex[2]}${hex[2]}${hex[3]}${hex[3]}`
+ : hex;
+
const r = parseInt(hex.slice(1, 3), 16) / 255;
const g = parseInt(hex.slice(3, 5), 16) / 255;
const b = parseInt(hex.slice(5, 7), 16) / 255;📝 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 const hexToHsl = (hex: string): string => { | |
| const r = parseInt(hex.slice(1, 3), 16) / 255; | |
| const g = parseInt(hex.slice(3, 5), 16) / 255; | |
| const b = parseInt(hex.slice(5, 7), 16) / 255; | |
| const max = Math.max(r, g, b); | |
| const min = Math.min(r, g, b); | |
| const delta = max - min; | |
| let h = 0; | |
| if (delta !== 0) { | |
| if (max === r) { | |
| h = ((g - b) / delta + (g < b ? 6 : 0)) * 60; | |
| } else if (max === g) { | |
| h = ((b - r) / delta + 2) * 60; | |
| } else { | |
| h = ((r - g) / delta + 4) * 60; | |
| } | |
| } | |
| const l = (max + min) / 2; | |
| const s = delta === 0 ? 0 : delta / (1 - Math.abs(2 * l - 1)); | |
| return `${Math.round(h)} ${Math.round(s * 100)}% ${Math.round(l * 100)}%`; | |
| }; | |
| export const hexToHsl = (hex: string): string => { | |
| // Validate hex format | |
| const hexRegex = /^#([A-Fa-f0-9]{3}|[A-Fa-f0-9]{6})$/; | |
| if (!hexRegex.test(hex)) { | |
| throw new Error('Invalid hex color format'); | |
| } | |
| // Normalize hex to 6 digits | |
| const normalizedHex = hex.length === 4 | |
| ? `#${hex[1]}${hex[1]}${hex[2]}${hex[2]}${hex[3]}${hex[3]}` | |
| : hex; | |
| const r = parseInt(normalizedHex.slice(1, 3), 16) / 255; | |
| const g = parseInt(normalizedHex.slice(3, 5), 16) / 255; | |
| const b = parseInt(normalizedHex.slice(5, 7), 16) / 255; | |
| const max = Math.max(r, g, b); | |
| const min = Math.min(r, g, b); | |
| const delta = max - min; | |
| let h = 0; | |
| if (delta !== 0) { | |
| if (max === r) { | |
| h = ((g - b) / delta + (g < b ? 6 : 0)) * 60; | |
| } else if (max === g) { | |
| h = ((b - r) / delta + 2) * 60; | |
| } else { | |
| h = ((r - g) / delta + 4) * 60; | |
| } | |
| } | |
| const l = (max + min) / 2; | |
| const s = delta === 0 ? 0 : delta / (1 - Math.abs(2 * l - 1)); | |
| return `${Math.round(h)} ${Math.round(s * 100)}% ${Math.round(l * 100)}%`; | |
| }; |
| const main = () => { | ||
| try { | ||
| console.log("🎨 Building design system..."); | ||
|
|
||
| // Styles | ||
| console.log("Building SCSS styles..."); | ||
| buildPaletteScss(); | ||
| buildColorsScss("light"); | ||
| buildColorsScss("dark"); | ||
| buildMainScss(); | ||
|
|
||
| // Tailwind | ||
| console.log("Building Tailwind configurations..."); | ||
| buildColorsTailwind(); | ||
| buildPaletteTailwind(); | ||
| buildTailwindConfig(); | ||
|
|
||
| console.log("✅ Design system built successfully!"); | ||
| } catch (error) { | ||
| console.error("❌ Build failed:", error); | ||
| process.exit(1); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and build process tracking.
The current implementation could benefit from:
- More detailed error reporting
- Build step status tracking
- Cleanup on failure
Consider this improved structure:
interface BuildStep {
name: string;
action: () => void;
}
const buildSteps: BuildStep[] = [
{ name: 'palette-scss', action: buildPaletteScss },
{ name: 'colors-scss-light', action: () => buildColorsScss("light") },
// ... other steps
];
const main = async () => {
const results = new Map<string, boolean>();
try {
console.log("🎨 Building design system...");
for (const step of buildSteps) {
console.log(`Running ${step.name}...`);
await step.action();
results.set(step.name, true);
}
console.log("✅ Design system built successfully!");
} catch (error) {
console.error("❌ Build failed:", error);
console.log("Build status:", Object.fromEntries(results));
process.exit(1);
}
};| // Paths for the output file | ||
| const outputDir = path.join(__dirname, "../tailwind"); | ||
| const outputFilePath = path.join(outputDir, "config.ts"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve path handling and configuration.
The current implementation uses hardcoded paths. Consider:
- Making paths configurable through environment variables
- Using a configuration file for paths
- Adding path validation
import { config } from '../config';
const getOutputPaths = () => {
const baseDir = process.env.DESIGN_SYSTEM_OUTPUT_DIR || config.outputDir;
return {
dir: path.resolve(baseDir, 'tailwind'),
file: path.resolve(baseDir, 'tailwind/config.ts')
};
};| "gray": { | ||
| "100": "var(--gray-100)", | ||
| "200": "var(--gray-200)", | ||
| "300": "var(--gray-300)", | ||
| "400": "var(--gray-400)", | ||
| "500": "var(--gray-500)", | ||
| "600": "var(--gray-600)", | ||
| "700": "var(--gray-700)", | ||
| "800": "var(--gray-800)", | ||
| "900": "var(--gray-900)", | ||
| "950": "var(--gray-950)", | ||
| "ali": "var(--gray-ali)", | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove inconsistent "ali" shade from gray category.
The "gray" category includes a non-standard shade name "ali" which breaks the numerical shade pattern used throughout the palette.
"gray": {
"100": "var(--gray-100)",
// ... other shades ...
"900": "var(--gray-900)",
"950": "var(--gray-950)",
- "ali": "var(--gray-ali)",
},📝 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.
| "gray": { | |
| "100": "var(--gray-100)", | |
| "200": "var(--gray-200)", | |
| "300": "var(--gray-300)", | |
| "400": "var(--gray-400)", | |
| "500": "var(--gray-500)", | |
| "600": "var(--gray-600)", | |
| "700": "var(--gray-700)", | |
| "800": "var(--gray-800)", | |
| "900": "var(--gray-900)", | |
| "950": "var(--gray-950)", | |
| "ali": "var(--gray-ali)", | |
| }, | |
| "gray": { | |
| "100": "var(--gray-100)", | |
| "200": "var(--gray-200)", | |
| "300": "var(--gray-300)", | |
| "400": "var(--gray-400)", | |
| "500": "var(--gray-500)", | |
| "600": "var(--gray-600)", | |
| "700": "var(--gray-700)", | |
| "800": "var(--gray-800)", | |
| "900": "var(--gray-900)", | |
| "950": "var(--gray-950)", | |
| }, |
| "purple": { | ||
| "50": "var(--purple-50)", | ||
| "100": "var(--purple-100)", | ||
| "200": "var(--purple-200)", | ||
| "300": "var(--purple-300)", | ||
| "400": "var(--purple-400)", | ||
| "500": "var(--purple-500)", | ||
| "600": "var(--purple-600)", | ||
| "700": "var(--purple-700)", | ||
| "800": "var(--purple-800)", | ||
| "900": "var(--purple-900)", | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Missing "950" shade in purple color category confirmed
The review comment is correct. All color categories in the design system consistently include the "950" shade, but the purple category is missing it. This is verified in both the palette implementation and token definitions.
🔗 Analysis chain
Add missing "950" shade to purple category.
The "purple" category is missing the "950" shade that exists in all other color categories.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if purple-950 is defined in token files
fd --type f --extension json . packages/design-system/src/tokens/ --exec grep -l "purple-950" {} \;
Length of output: 102
Script:
#!/bin/bash
# Check for all color token definitions and 950 shades
echo "=== Checking palette implementation ==="
cat packages/design-system/src/tailwind/palette.ts
echo -e "\n=== Searching for any *-950 definitions ==="
rg -l ".*-950.*" packages/design-system/src/
echo -e "\n=== Checking token files for color definitions ==="
fd . packages/design-system/src/tokens/ -e json -e ts --exec cat {}
Length of output: 43233
| - name: Setup Bun | ||
| uses: oven-sh/setup-bun@v1 | ||
| with: | ||
| bun-version: latest |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin Bun version for stability.
Using "latest" for bun-version could lead to unexpected issues if a new version introduces breaking changes.
- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
- bun-version: latest
+ bun-version: 1.0.20 # Pin to a specific version📝 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 Bun | |
| uses: oven-sh/setup-bun@v1 | |
| with: | |
| bun-version: latest | |
| - name: Setup Bun | |
| uses: oven-sh/setup-bun@v1 | |
| with: | |
| bun-version: 1.0.20 # Pin to a specific version |
| @use "./_tailinwd.scss" as *; | ||
| @use "@repo/design-system/styles" as *; | ||
|
|
||
| // TODO: Delete |
There was a problem hiding this comment.
💡 Codebase verification
Do not delete color tokens until properly migrated to design system
The commented-out color tokens in globals.scss are still actively used throughout the codebase via Tailwind classes and CSS variables. These tokens must be properly migrated to the design system package before deletion to prevent breaking existing components.
- Migrate color tokens to
/packages/design-system/src/styles/base/_colors-light.scssand_colors-dark.scss - Update component styles to use design system tokens
- Remove globals.scss tokens only after migration is complete and verified
🔗 Analysis chain
Track the TODO comment for code cleanup.
The commented-out color tokens and theme variables should be reviewed before deletion to ensure all values are properly migrated to the design system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these color tokens exist in the design system
echo "Checking for color token definitions in design system..."
rg --type scss -A 5 "primary-[0-9]+"
rg --type scss -A 5 "error-[0-9]+"
rg --type scss -A 5 "warning-[0-9]+"
rg --type scss -A 5 "success-[0-9]+"
rg --type scss -A 5 "info-[0-9]+"
Length of output: 457
Script:
#!/bin/bash
# Find design system related files
echo "Locating design system files..."
fd -e scss -e css -e js -e json -e ts
# Search for color tokens in various formats
echo -e "\nSearching for color token definitions..."
rg -i "(--color-primary-|primary-[0-9]+|error-[0-9]+|warning-[0-9]+|success-[0-9]+|info-[0-9]+)" -A 3
Length of output: 25356
| --accent: var(--zinc-800); | ||
| --accent-foreground: var(--General-white); | ||
| --background: var(--zinc-950); | ||
| --border: var(--zinc-800); | ||
| --card: var(--zinc-900); | ||
| --card-foreground: var(--General-white); | ||
| --destructive: var(--rose-500); | ||
| --destructive-foreground: var(--General-black); | ||
| --error: var(--rose-500); | ||
| --error-border: var(--red-900); | ||
| --error-foreground: var(--red-100); | ||
| --foreground: var(--zinc-50); | ||
| --information: var(--sky-500); | ||
| --information-border: var(--blue-900); | ||
| --information-foreground: var(--blue-100); | ||
| --input: var(--zinc-800); | ||
| --muted: var(--zinc-800); | ||
| --muted-foreground: var(--zinc-400); | ||
| --popover: var(--zinc-950); | ||
| --popover-foreground: var(--General-white); | ||
| --primary: var(--purple-500); | ||
| --primary-foreground: var(--General-white); | ||
| --primary-light: var(--purple-100); | ||
| --ring: var(--zinc-600); | ||
| --secondary: var(--zinc-800); | ||
| --secondary-foreground: var(--General-white); | ||
| --success: var(--emerald-500); | ||
| --success-border: var(--green-900); | ||
| --success-foreground: var(--green-50); | ||
| --table-header: rgba(39, 39, 42, 0.5000); | ||
| --warning: var(--amber-500); | ||
| --warning-border: var(--amber-900); | ||
| --warning-foreground: var(--amber-100); |
There was a problem hiding this comment.
💡 Codebase verification
Multiple color combinations fail to meet WCAG AA standards (minimum 4.5:1 ratio):
- Primary text: white on purple-500 (3.96:1)
- Error text: red-100 on rose-500 (3.01:1)
- Success text: green-50 on emerald-500 (2.42:1)
- Warning text: amber-100 on amber-500 (1.93:1)
Consider using darker background colors or lighter text colors to achieve the minimum 4.5:1 contrast ratio required for WCAG AA compliance.
🔗 Analysis chain
Verify color contrast ratios for accessibility.
Please ensure that the color combinations meet WCAG accessibility standards, particularly for:
- Text colors against backgrounds (--foreground on --background)
- Primary/Secondary text on their backgrounds
- Error/Success/Warning text on their respective backgrounds
Run the following script to generate a color contrast report:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Install and use color-contrast-checker
npm install color-contrast-checker
cat << 'EOF' > check_contrast.js
const ColorContrastChecker = require('color-contrast-checker');
const ccc = new ColorContrastChecker();
// Define color pairs to check (example pairs)
const colorPairs = [
['var(--foreground)', 'var(--background)'],
['var(--primary-foreground)', 'var(--primary)'],
['var(--error-foreground)', 'var(--error)'],
['var(--success-foreground)', 'var(--success)'],
['var(--warning-foreground)', 'var(--warning)']
];
colorPairs.forEach(([fg, bg]) => {
const passes = ccc.isLevelAA(fg, bg);
console.log(`Contrast check for ${fg} on ${bg}: ${passes ? 'PASSES' : 'FAILS'} WCAG AA`);
});
EOF
node check_contrast.js
Length of output: 1203
Script:
#!/bin/bash
# First, find all color definitions
echo "=== Searching for color definitions ==="
rg --type scss -g '!node_modules' -g '!dist' '(--zinc-|--rose-|--purple-|--emerald-|--amber-|--red-|--green-|--blue-|--General-).*:.*#'
# Then use Python to calculate contrast ratios
pip install wcag-contrast-ratio
cat << 'EOF' > check_contrast.py
from wcag_contrast_ratio import rgb, contrast
def hex_to_rgb(hex_color):
hex_color = hex_color.lstrip('#')
return tuple(int(hex_color[i:i+2], 16) for i in (0, 2, 4))
# Define your color mappings based on the found values
# We'll update these after finding the actual values
colors = {
'zinc-950': '#09090b',
'zinc-50': '#fafafa',
'purple-500': '#a855f7',
'white': '#ffffff',
'rose-500': '#f43f5e',
'red-100': '#fee2e2',
'emerald-500': '#10b981',
'green-50': '#f0fdf4',
'amber-500': '#f59e0b',
'amber-100': '#fef3c7'
}
# Check contrast ratios
pairs = [
('zinc-50', 'zinc-950', 'Text on Background'),
('white', 'purple-500', 'Text on Primary'),
('red-100', 'rose-500', 'Error Text'),
('green-50', 'emerald-500', 'Success Text'),
('amber-100', 'amber-500', 'Warning Text')
]
print("\n=== Contrast Ratio Results ===")
for fg_key, bg_key, name in pairs:
fg = hex_to_rgb(colors[fg_key])
bg = hex_to_rgb(colors[bg_key])
ratio = contrast(rgb(fg[0], fg[1], fg[2]), rgb(bg[0], bg[1], bg[2]))
passes_aa = ratio >= 4.5
print(f"{name}: {ratio:.2f}:1 - {'PASSES' if passes_aa else 'FAILS'} WCAG AA")
EOF
python check_contrast.py
Length of output: 1532
Script:
#!/bin/bash
# First, find all color definitions
echo "=== Searching for color definitions ==="
rg --type-add 'scss:*.scss' --type scss -g '!node_modules' -g '!dist' '(--zinc-|--rose-|--purple-|--emerald-|--amber-|--red-|--green-|--blue-|--General-).*:.*#'
# Then use Python to calculate contrast ratios
pip install wcag-contrast-ratio
cat << 'EOF' > check_contrast.py
from wcag_contrast_ratio import rgb, contrast
def hex_to_rgb(hex_color):
hex_color = hex_color.lstrip('#')
return tuple(int(hex_color[i:i+2], 16) for i in (0, 2, 4))
# Define color mappings
colors = {
'zinc-950': '#09090b',
'zinc-50': '#fafafa',
'purple-500': '#a855f7',
'white': '#ffffff',
'rose-500': '#f43f5e',
'red-100': '#fee2e2',
'emerald-500': '#10b981',
'green-50': '#f0fdf4',
'amber-500': '#f59e0b',
'amber-100': '#fef3c7'
}
# Check contrast ratios
pairs = [
('zinc-50', 'zinc-950', 'Text on Background'),
('white', 'purple-500', 'Text on Primary'),
('red-100', 'rose-500', 'Error Text'),
('green-50', 'emerald-500', 'Success Text'),
('amber-100', 'amber-500', 'Warning Text')
]
print("\n=== Contrast Ratio Results ===")
for fg_key, bg_key, name in pairs:
fg = hex_to_rgb(colors[fg_key])
bg = hex_to_rgb(colors[bg_key])
ratio = contrast(rgb(fg), rgb(bg))
passes_aa = ratio >= 4.5
print(f"{name}: {ratio:.2f}:1 - {'PASSES' if passes_aa else 'FAILS'} WCAG AA")
EOF
python check_contrast.py
Length of output: 774
Script:
#!/bin/bash
# Install colormath for reliable color calculations
pip install colormath
cat << 'EOF' > check_contrast.py
from colormath.color_objects import sRGBColor, LabColor
from colormath.color_conversions import convert_color
def hex_to_rgb(hex_str):
hex_str = hex_str.lstrip('#')
r = int(hex_str[0:2], 16) / 255.0
g = int(hex_str[2:4], 16) / 255.0
b = int(hex_str[4:6], 16) / 255.0
return (r, g, b)
def get_relative_luminance(r, g, b):
def linearize(v):
if v <= 0.03928:
return v / 12.92
return ((v + 0.055) / 1.055) ** 2.4
rl, gl, bl = map(linearize, (r, g, b))
return 0.2126 * rl + 0.7152 * gl + 0.0722 * bl
def calculate_contrast_ratio(fg_hex, bg_hex):
fg_rgb = hex_to_rgb(fg_hex)
bg_rgb = hex_to_rgb(bg_hex)
l1 = get_relative_luminance(*fg_rgb)
l2 = get_relative_luminance(*bg_rgb)
lighter = max(l1, l2)
darker = min(l1, l2)
return (lighter + 0.05) / (darker + 0.05)
# Define color mappings
colors = {
'zinc-950': '#09090b',
'zinc-50': '#fafafa',
'purple-500': '#a855f7',
'white': '#ffffff',
'rose-500': '#f43f5e',
'red-100': '#fee2e2',
'emerald-500': '#10b981',
'green-50': '#f0fdf4',
'amber-500': '#f59e0b',
'amber-100': '#fef3c7'
}
# Check contrast ratios
pairs = [
('zinc-50', 'zinc-950', 'Text on Background'),
('white', 'purple-500', 'Text on Primary'),
('red-100', 'rose-500', 'Error Text'),
('green-50', 'emerald-500', 'Success Text'),
('amber-100', 'amber-500', 'Warning Text')
]
print("\n=== Contrast Ratio Results ===")
for fg_key, bg_key, name in pairs:
ratio = calculate_contrast_ratio(colors[fg_key], colors[bg_key])
passes_aa = ratio >= 4.5
print(f"{name}: {ratio:.2f}:1 - {'PASSES' if passes_aa else 'FAILS'} WCAG AA")
EOF
python check_contrast.py
Length of output: 1707
body
Summary by CodeRabbit
Based on the comprehensive changes, here are the release notes:
New Features
Improvements
Chores