this is title pull request #210
Conversation
WalkthroughThis pull request introduces a comprehensive design system implementation for a project, focusing on creating a robust and flexible styling architecture. The changes include establishing a new design system package with scripts for generating SCSS and Tailwind CSS configurations, defining color palettes, and creating theme-specific color variables. The implementation supports both light and dark themes, provides utility scripts for color conversions, and sets up automated build processes through GitHub Actions. Changes
Sequence DiagramsequenceDiagram
participant Build Script
participant Token JSON
participant Color Generator
participant SCSS Generator
participant Tailwind Generator
Build Script->>Token JSON: Read color tokens
Token JSON-->>Color Generator: Provide color data
Color Generator->>SCSS Generator: Generate color styles
Color Generator->>Tailwind Generator: Generate color configurations
SCSS Generator-->>Build Script: Create SCSS files
Tailwind Generator-->>Build Script: Create Tailwind config
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: 13
🧹 Nitpick comments (15)
packages/design-system/src/styles/base/_colors-dark.scss (2)
1-1: Consider using an explicit namespace for palette imports.Using
as *makes variables globally available but can lead to naming conflicts. Consider using a descriptive namespace like@use "_palette.scss" as palette;for better maintainability and clarity.-@use "_palette.scss" as *; +@use "_palette.scss" as palette;
34-34: Avoid hardcoded opacity values.The table header uses a hardcoded rgba value. Consider using CSS custom properties for consistency.
- --table-header: rgba(39, 39, 42, 0.5000); + --table-header-opacity: 0.5; + --table-header: rgba(39, 39, 42, var(--table-header-opacity));packages/design-system/src/scripts/build-main-style.ts (1)
4-6: Consider making paths configurable.The hardcoded paths make the script less flexible. Consider extracting these paths to a configuration file or environment variables for better maintainability and reusability across different project structures.
+import { config } from '../config'; + // Paths for the output file -const outputDir = path.join(__dirname, "../styles"); -const outputFilePath = path.join(outputDir, "main.scss"); +const outputDir = path.join(__dirname, config.styles.outputDir); +const outputFilePath = path.join(outputDir, config.styles.mainFileName);packages/design-system/src/scripts/build.ts (1)
8-30: Enhance error handling and build process efficiency.The current implementation could benefit from:
- More detailed error handling
- Parallel processing for independent build steps
- Build validation and cleanup
Here's a suggested implementation:
const main = () => { try { console.log("🎨 Building design system..."); + // Clean up old build files + console.log("Cleaning up old build files..."); + cleanup(); + // Styles console.log("Building SCSS styles..."); - buildPaletteScss(); - buildColorsScss("light"); - buildColorsScss("dark"); - buildMainScss(); + await Promise.all([ + buildPaletteScss(), + buildColorsScss("light"), + buildColorsScss("dark"), + buildMainScss(), + ]); // Tailwind console.log("Building Tailwind configurations..."); - buildColorsTailwind(); - buildPaletteTailwind(); - buildTailwindConfig(); + await Promise.all([ + buildColorsTailwind(), + buildPaletteTailwind(), + buildTailwindConfig(), + ]); + + // Validate build output + console.log("Validating build output..."); + validateBuild(); console.log("✅ Design system built successfully!"); } catch (error) { - console.error("❌ Build failed:", error); + console.error("❌ Build failed:"); + if (error instanceof Error) { + console.error(`Error: ${error.message}`); + console.error(`Stack: ${error.stack}`); + } else { + console.error(error); + } process.exit(1); } };packages/design-system/src/tailwind/colors.ts (1)
1-35: Add TypeScript type definitions for better maintainability.Consider adding a type definition for the colors object to improve maintainability and provide better IDE support.
+type ColorToken = { + [key: string]: string; +}; + -export const colors = { +export const colors: ColorToken = { "accent": "hsl(var(--accent))", // ... rest of the colors };packages/design-system/src/scripts/build-palette-tailwind.ts (1)
53-54: Consider making the script execution conditional.The script automatically runs when imported. Consider making this optional to allow for testing and different execution contexts.
-// Run the script -buildPaletteTailwind(); +// Only run the script if this file is being executed directly +if (require.main === module) { + buildPaletteTailwind(); +}packages/design-system/src/scripts/build-colors-tailwind.ts (2)
13-30: Add error handling and type validation.Consider these improvements for robustness:
- Add type validation for the tokens parameter
- Handle malformed tokens gracefully
-const generateColorsTsContent = (tokens: Record<string, any>): string => { +interface ColorToken { + color: Record<string, { $value: string }>; +} + +const generateColorsTsContent = (tokens: ColorToken): string => { const colorTokens = tokens.color; + if (!colorTokens) { + throw new Error('Invalid token structure: missing color tokens'); + } const lines: string[] = [];
60-61: Consider conditional execution.The immediate execution at import time could cause issues in testing scenarios or when importing as a module. Consider making it conditional:
-// Build colors.ts -buildColorsTailwind(); +// Only execute if this is the main module +if (require.main === module) { + buildColorsTailwind(); +}packages/design-system/src/scripts/build-colors-style.ts (2)
7-9: Strengthen the token reference regex pattern.The current regex pattern
{(.+?)}is quite permissive. Consider using a more specific pattern to prevent potential security issues.- return value.replace(/{(.+?)}/g, (_, match) => `var(--${match.replace(".", "-")})`); + return value.replace(/{([a-zA-Z][a-zA-Z0-9]*(?:\.[a-zA-Z][a-zA-Z0-9]*)*)}/g, (_, match) => `var(--${match.replace(".", "-")})`);
77-79: Improve execution strategy and error handling.The immediate execution of both themes could cause issues in testing or when importing as a module. Also, consider adding error handling around the theme builds.
-// Reusable calls for light and dark themes -buildColorsScss("light"); -buildColorsScss("dark"); +if (require.main === module) { + try { + buildColorsScss("light"); + buildColorsScss("dark"); + console.log('Successfully built both themes'); + } catch (error) { + console.error('Failed to build themes:', error); + process.exit(1); + } +}packages/design-system/src/tailwind/palette.ts (1)
1-291: Consider using TypeScript enums or constants for color categories.To improve type safety and maintainability, consider defining the color categories and shade numbers as TypeScript enums or constants.
enum ColorShade { '50' = '50', '100' = '100', // ... other shades } enum ColorCategory { General = 'General', Amber = 'amber', // ... other categories } type ColorPalette = { [key in ColorCategory]: { [shade in ColorShade]?: string; } & (key extends ColorCategory.General ? { black: string; white: string; } : {}); };.github/workflows/build-design-system.yml (1)
1-34: Enhance the workflow with caching and artifacts.Consider these improvements to the workflow:
- Add Bun cache to speed up builds
- Upload build artifacts for verification
name: Build Design System on: push: paths: - "packages/design-system/src/tokens/**" pull_request: paths: - "packages/design-system/src/tokens/**" jobs: build-design-system: runs-on: ubuntu-latest steps: - name: Checkout repository uses: actions/checkout@v3 - name: Setup Bun uses: oven-sh/setup-bun@v1 with: bun-version: latest + - name: Cache Bun 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 - name: Run build script working-directory: ./packages/design-system run: bun run build + - name: Upload build artifacts + uses: actions/upload-artifact@v3 + with: + name: design-system-build + path: packages/design-system/distpackages/design-system/src/styles/base/_colors-light.scss (3)
1-1: Consider using a named import instead of namespace removal.Using
@use "_palette.scss" as *makes it less clear where the variables are coming from. Consider using a named import like@use "_palette.scss" as palettefor better maintainability and clarity.-@use "_palette.scss" as *; +@use "_palette.scss" as palette;
9-10: Maintain consistency in color palette usage.The card uses
zinc-100but its foreground usesslate-900. Consider using the same color family for better consistency:--card: var(--zinc-100); - --card-foreground: var(--slate-900); + --card-foreground: var(--zinc-900);
4-38: Add documentation for semantic color usage.Consider adding comments to document the intended usage of each semantic color. This will help maintain consistency as the design system grows.
Example:
.light { // Primary colors - Used for main actions and key UI elements --primary: var(--purple-500); --primary-foreground: var(--General-white); // Semantic colors - Used for status and feedback --success: var(--green-100); // ... etc }
📜 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 (9)
- packages/design-system/tsconfig.json
- packages/ui/src/styles/_tailinwd.scss
- packages/design-system/src/styles/main.scss
- apps/core/app/layout.tsx
- packages/design-system/src/tokens/token_Palette_primitive.json
- apps/core/app/auth/layout.tsx
- packages/design-system/tsconfig.lint.json
- packages/design-system/package.json
- packages/design-system/src/styles/base/_palette.scss
🧰 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: Verify the necessity of disabling typescript-config/consistent-casing.While disabling this rule might be necessary for your design system implementation, inconsistent casing in TypeScript configurations could lead to maintainability issues. Please ensure this is intentional and consider:
- Adding a comment explaining why this rule needs to be disabled
- Limiting the scope of this configuration if possible
Let's verify if there are any specific casing patterns in the TypeScript configurations that necessitate this change:
✅ Verification successful
Disabling typescript-config/consistent-casing is justified for this design system
The mixed casing patterns in the codebase are intentional and necessary to maintain consistency with:
- shadcn-ui and Radix UI conventions (kebab-case files)
- React component conventions (PascalCase)
- React hooks conventions (camelCase)
- Icon component naming patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for TypeScript configuration files and their casing patterns echo "Searching for tsconfig files and their content..." fd -e json -e jsonc --full-path . -x sh -c 'echo "=== {} ==="; cat {}' echo -e "\nSearching for specific casing patterns in TypeScript files..." rg -g '*.ts' -g '*.tsx' -i 'export|interface|type|class' -A 2Length of output: 113849
Script:
#!/bin/bash # Search for casing patterns in imports, exports, and type definitions echo "Searching for import/export statements with different casing patterns..." rg -g '*.ts' -g '*.tsx' -i '^import.*from.*' -A 1 echo -e "\nSearching for type and interface declarations..." rg -g '*.ts' -g '*.tsx' -i '^(export )?(type|interface).*' -A 1 echo -e "\nSearching for class declarations..." rg -g '*.ts' -g '*.tsx' -i '^(export )?class.*' -A 1Length of output: 66716
packages/design-system/src/styles/base/_colors-dark.scss (2)
3-4: Well-structured layer and selector usage!Good use of
@layer basefor fundamental styles and:rootfor global CSS custom properties.
1-39: Verify palette variable dependencies.Let's ensure all referenced color variables are defined in the palette file.
✅ Verification successful
All palette variables are properly defined ✅
All color variables referenced in
_colors-dark.scssare correctly defined in the palette file. The palette provides complete color scales and includes all necessary variables for the dark theme implementation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Extract all palette variable references echo "Checking for palette variable references..." palette_vars=$(cat packages/design-system/src/styles/base/_colors-dark.scss | grep -o 'var(--[^)]*)' | sort -u) # Check if palette file exists and contains these variables echo "Verifying variables in palette file..." for var in $palette_vars; do var=${var#var(} var=${var%)} rg -q "$var" "packages/design-system/src/styles/base/_palette.scss" || echo "Missing variable: $var" doneLength of output: 14324
Script:
#!/bin/bash # Read the colors dark file and extract variables echo "Checking palette file content first..." cat packages/design-system/src/styles/base/_palette.scss echo -e "\nNow checking for variable definitions..." palette_vars=$(cat packages/design-system/src/styles/base/_colors-dark.scss | grep -o 'var(--[^)]*)' | sed 's/var(//;s/)//' | sort -u) # Check each variable with proper rg syntax echo "Verifying variables in palette file..." for var in $palette_vars; do # Escape the variable name for proper searching escaped_var=$(echo "$var" | sed 's/[\/&]/\\&/g') rg -q "\\$escaped_var[^-]" "packages/design-system/src/styles/base/_palette.scss" || echo "Missing variable: $var" doneLength of output: 11286
packages/ui/postcss.config.mjs (1)
3-3: LGTM! Verify PostCSS SCSS setup.The addition of SCSS syntax support is correct and aligns with the design system implementation.
Let's verify the PostCSS SCSS setup is complete:
✅ Verification successful
✅ PostCSS SCSS setup is complete and correct
- Dependency
[email protected]is properly declared- SCSS files are present and being used in the UI package
- PostCSS configuration is properly set up with required plugins
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify PostCSS SCSS configuration and dependencies # Check if postcss-scss is properly declared in package.json echo "Checking postcss-scss dependency..." if ! grep -q '"postcss-scss"' package.json; then echo "Warning: postcss-scss dependency not found in package.json" exit 1 fi # Check for SCSS files in the project echo "Checking for SCSS files..." fd -e scssLength of output: 256
Script:
#!/bin/bash # Check UI package specifically echo "Checking UI package configuration..." cat packages/ui/package.json | grep -A 2 -B 2 "postcss-scss" echo -e "\nChecking for SCSS files in UI package..." fd -e scss . packages/ui/ echo -e "\nChecking complete PostCSS config..." cat packages/ui/postcss.config.mjsLength of output: 797
packages/ui/package.json (3)
66-72: LGTM! Required SCSS dependencies are properly configured.The addition of
postcss-scssandsassdependencies with pinned versions is correct for SCSS support.
81-81: Verify the new SCSS file structure.The export path has been updated to reflect the new SCSS file location. Let's ensure the file structure is correct and no imports are broken.
✅ Verification successful
SCSS file structure is correct and properly mapped
The file exists at
packages/ui/src/styles/globals.scssand is correctly mapped in the package.json exports.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify SCSS file structure and imports # Check if the SCSS file exists in the new location echo "Checking for globals.scss..." if [ ! -f "src/styles/globals.scss" ]; then echo "Warning: globals.scss not found at src/styles/globals.scss" exit 1 fi # Check for any remaining .css imports that should be .scss echo "Checking for remaining CSS imports..." rg -t tsx -t ts "from ['\"].*\.css['\"]"Length of output: 243
Script:
#!/bin/bash # Search for globals.scss and check SCSS imports echo "Searching for globals.scss in the repository..." fd globals.scss echo -e "\nChecking package.json location..." fd package.json --type f --exec cat {} \; | grep -A 1 -B 1 "globals.scss" echo -e "\nChecking for SCSS imports..." rg "from ['\"].*\.scss['\"]"Length of output: 540
27-27: Consider pinning the design system version.Using a wildcard version (
*) for@repo/design-systemcould lead to unexpected breaking changes. Consider pinning to a specific version or using a version range.packages/design-system/src/scripts/build-main-style.ts (2)
9-14: 🛠️ Refactor suggestionAdd validation for required base style files.
The function assumes the existence of base style files without verification. This could lead to silent failures or confusing build outputs if the files are missing.
const generateMainScssContent = (): string => { + const baseFiles = ['_colors-dark.scss', '_colors-light.scss']; + const basePath = path.join(outputDir, 'base'); + + // Verify base files exist + baseFiles.forEach(file => { + const filePath = path.join(basePath, file); + if (!fs.existsSync(filePath)) { + throw new Error(`Required base file ${file} not found at ${filePath}`); + } + }); + return ` @use "./base/_colors-dark.scss" as *; @use "./base/_colors-light.scss" as *; `.trim(); };
1-32: Verify integration with the design system package.Please ensure this script is properly integrated with the design system's build pipeline and package.json scripts.
✅ Verification successful
Build integration verified and confirmed working correctly
The script is properly integrated into the design system's build pipeline, executed in the correct order, and maintains a clean dependency structure with no circular imports.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check package.json scripts and dependencies echo "Checking package.json configuration..." fd --type f --glob "package.json" packages/design-system | xargs cat | jq '.scripts, .dependencies, .devDependencies' # Check for potential circular dependencies in imports echo "Checking for circular dependencies..." rg -l "@use|@import" packages/design-system/src/stylesLength of output: 625
Script:
#!/bin/bash # Check if build.ts includes our script echo "Checking build.ts content..." cat packages/design-system/src/scripts/build.ts # Check color files content echo -e "\nChecking color files content..." cat packages/design-system/src/styles/base/_colors-light.scss packages/design-system/src/styles/base/_colors-dark.scssLength of output: 3897
packages/ui/src/styles/globals.scss (3)
1-2: LGTM! Modern SCSS import syntax used correctly.The imports are well-structured using the modern
@usedirective instead of the deprecated@import.
212-219: LGTM! Clean implementation of base styles.The base layer correctly applies design tokens through Tailwind utilities for consistent styling.
221-232: LGTM! Well-implemented cross-browser scrollbar utility.The no-scrollbar utility is properly implemented with:
- Clear documentation
- Cross-browser compatibility
- Appropriate vendor prefixes
packages/design-system/src/tailwind/config.ts (1)
4-10: Verify potential color name conflicts between colors and palette.The implementation correctly extends Tailwind's theme with merged color objects. However, since both objects are spread into the same namespace, we should ensure there are no naming conflicts that could cause unintended overwrites.
✅ Verification successful
No naming conflicts detected between colors and palette objects ✅
The merge is safe because:
colorsuses semantic names (accent, background, etc.)paletteis nested under group names (General, amber, etc.), creating separate namespaces🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find potential naming conflicts between colors and palette echo "Checking for naming conflicts in color definitions..." # Search for color object definitions rg -A 10 "export const colors = {" ./packages/design-system/src rg -A 10 "export const palette = {" ./packages/design-system/srcLength of output: 471
Script:
#!/bin/bash # Find color-related imports and definitions echo "Searching for color-related imports..." rg "import.*\{.*(?:colors|palette).*\}" ./packages/design-system/src echo -e "\nSearching for color definitions..." rg -A 10 "export const colors = \{" ./packages/design-system/src rg -A 10 "export const palette = \{" ./packages/design-system/srcLength of output: 4587
packages/ui/tailwind.config.ts (1)
3-6: LGTM! Clean integration with the design system.Good job centralizing the color configuration in the design system package. This will help maintain consistency across the project.
packages/design-system/src/scripts/build-palette-tailwind.ts (1)
4-6:⚠️ Potential issueCritical: Required input file is missing.
The build is failing because
token_Palette_Primitive.jsonis missing. This file is required for generating the color palette.packages/design-system/src/scripts/build-colors-tailwind.ts (1)
1-6: LGTM! Clean and standard implementation.The file structure and path handling follow Node.js best practices.
packages/design-system/src/styles/base/_colors-light.scss (1)
1-1: Verify palette variable definitions.Ensure that all referenced color variables are defined in
_palette.scss.✅ Verification successful
All palette variables are properly defined ✓
The
_palette.scssfile exists and all color variables used in_colors-light.scssare properly defined in the palette file.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if all color variables are defined in _palette.scss echo "Checking for undefined color variables..." rg -o "var\((--[^)]+)\)" packages/design-system/src/styles/base/_colors-light.scss | cut -d'(' -f2 | tr -d ')' | sort > /tmp/used_vars.txt rg "^[[:space:]]*--" packages/design-system/src/styles/base/_palette.scss | cut -d':' -f1 | tr -d ' ' | sort > /tmp/defined_vars.txt comm -23 /tmp/used_vars.txt /tmp/defined_vars.txtLength of output: 468
Script:
#!/bin/bash # Check if palette file exists and show its content echo "Checking _palette.scss existence and content..." if [ -f "packages/design-system/src/styles/base/_palette.scss" ]; then echo "File exists. Content:" cat packages/design-system/src/styles/base/_palette.scss echo -e "\nChecking variable usage in _colors-light.scss:" cat packages/design-system/src/styles/base/_colors-light.scss else echo "Error: _palette.scss not found" fiLength of output: 8479
| --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.
🛠️ Refactor suggestion
Standardize color naming conventions and improve documentation.
-
The naming convention is inconsistent:
General-whitevszinc-800- Consider standardizing all color names
-
Some color combinations might have accessibility concerns:
--destructive-foreground: var(--General-black)on--destructive: var(--rose-500)--primary-foreground: var(--General-white)on--primary: var(--purple-500)
Consider adding color contrast validation and documentation:
// Example documentation format
:root {
/* Primary colors */
--primary: var(--purple-500); // Used for main CTAs
--primary-foreground: var(--General-white); // Ensures WCAG AA contrast
/* ... other colors ... */
}| // Run the script | ||
| buildMainScss(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve script execution handling.
The immediate script execution should handle errors and process exit properly, especially after converting to async operations.
-// Run the script
-buildMainScss();
+// Run the script with proper error handling
+if (require.main === module) {
+ buildMainScss()
+ .then(() => {
+ process.exit(0);
+ })
+ .catch((error) => {
+ console.error('Build failed:', 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.
| // Run the script | |
| buildMainScss(); | |
| // Run the script with proper error handling | |
| if (require.main === module) { | |
| buildMainScss() | |
| .then(() => { | |
| process.exit(0); | |
| }) | |
| .catch((error) => { | |
| console.error('Build failed:', error); | |
| process.exit(1); | |
| }); | |
| } |
| 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
Enhance error handling and consider async operations.
The current implementation uses synchronous operations and lacks comprehensive error handling. Consider these improvements:
- Use async/await for better performance
- Add error handling
- Backup existing file before overwriting
-export const buildMainScss = (): void => {
+export const buildMainScss = async (): Promise<void> => {
try {
// Generate the content
const scssContent = generateMainScssContent();
// Ensure the output directory exists
- if (!fs.existsSync(outputDir)) {
- fs.mkdirSync(outputDir, { recursive: true });
+ await fs.promises.mkdir(outputDir, { recursive: true });
+
+ // Backup existing file if it exists
+ if (await fs.promises.access(outputFilePath).then(() => true).catch(() => false)) {
+ const backupPath = `${outputFilePath}.backup`;
+ await fs.promises.copyFile(outputFilePath, backupPath);
}
// Write the content to the file
- fs.writeFileSync(outputFilePath, scssContent, "utf-8");
+ await fs.promises.writeFile(outputFilePath, scssContent, "utf-8");
console.log(`main.scss file created at: ${outputFilePath}`);
+ } catch (error) {
+ console.error('Failed to build main.scss:', error);
+ throw error;
}
};📝 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 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}`); | |
| }; | |
| export const buildMainScss = async (): Promise<void> => { | |
| try { | |
| // Generate the content | |
| const scssContent = generateMainScssContent(); | |
| // Ensure the output directory exists | |
| await fs.promises.mkdir(outputDir, { recursive: true }); | |
| // Backup existing file if it exists | |
| if (await fs.promises.access(outputFilePath).then(() => true).catch(() => false)) { | |
| const backupPath = `${outputFilePath}.backup`; | |
| await fs.promises.copyFile(outputFilePath, backupPath); | |
| } | |
| // Write the content to the file | |
| await fs.promises.writeFile(outputFilePath, scssContent, "utf-8"); | |
| console.log(`main.scss file created at: ${outputFilePath}`); | |
| } catch (error) { | |
| console.error('Failed to build main.scss:', error); | |
| throw error; | |
| } | |
| }; |
| // TODO: Delete | ||
| // @layer base { | ||
| // :root { | ||
| // /* primary */ | ||
| // --primary-50: #ecefff; | ||
| // --primary-100: #dde2ff; | ||
| // --primary-200: #c2caff; | ||
| // --primary-300: #9ca5ff; | ||
| // --primary-400: #7576ff; | ||
| // --primary-500: #6155ff; | ||
| // --primary-600: #553af5; | ||
| // --primary-700: #4629d9; | ||
| // --primary-800: #3925ae; | ||
| // --primary-900: #322689; | ||
|
|
||
| // /* Erorr */ | ||
| // --error-50: #fef2f2; | ||
| // --error-100: #fee2e2; | ||
| // --error-200: #fecaca; | ||
| // --error-300: #fca5a5; | ||
| // --error-400: #f87171; | ||
| // --error-500: #ef4444; | ||
| // --error-600: #dc2626; | ||
| // --error-700: #b91c1c; | ||
| // --error-800: #991b1b; | ||
| // --error-900: #7f1d1d; | ||
|
|
||
| // /* Warning */ | ||
| // --warning-50: #fff4ed; | ||
| // --warning-100: #ffe6d5; | ||
| // --warning-200: #feccaa; | ||
| // --warning-300: #fdac74; | ||
| // --warning-400: #fb8a3c; | ||
| // --warning-500: #f97316; | ||
| // --warning-600: #ea670c; | ||
| // --warning-700: #c2570c; | ||
| // --warning-800: #9a4a12; | ||
| // --warning-900: #7c3d12; | ||
|
|
||
| // /* gray */ | ||
| // --gray-50: #f9fafb; | ||
| // --gray-100: #f3f4f6; | ||
| // --gray-200: #e5e7eb; | ||
| // --gray-300: #d1d5db; | ||
| // --gray-400: #9ca3af; | ||
| // --gray-500: #6b7280; | ||
| // --gray-600: #4b5563; | ||
| // --gray-700: #374151; | ||
| // --gray-800: #1f2937; | ||
| // --gray-900: #111827; | ||
|
|
||
| // /* Success */ | ||
| // --success-50: #f0fdf4; | ||
| // --success-100: #f0fdf5; | ||
| // --success-200: #bbf7d1; | ||
| // --success-300: #86efad; | ||
| // --success-400: #4ade80; | ||
| // --success-500: #22c55e; | ||
| // --success-600: #16a34a; | ||
| // --success-700: #15803c; | ||
| // --success-800: #166533; | ||
| // --success-900: #14532b; | ||
|
|
||
| // /* Info */ | ||
| // --info-50: #f0f9ff; | ||
| // --info-100: #e0f2fe; | ||
| // --info-200: #bae5fd; | ||
| // --info-300: #7dd1fc; | ||
| // --info-400: #38bbf8; | ||
| // --info-500: #0ea2e9; | ||
| // --info-600: #0288d1; | ||
| // --info-700: #0367a1; | ||
| // --info-800: #075785; | ||
| // --info-900: #10C496E; | ||
|
|
||
| // --background: 0 0% 100%; | ||
| // --foreground: 240 10% 4%; | ||
| // --card: 240 5% 96%; | ||
| // --card-foreground: 222 47% 11%; | ||
| // --popover: 0 0% 100%; | ||
| // --popover-foreground: 222 47% 11%; | ||
| // --primary: 250 62% 58%; | ||
| // --primary-foreground: 210 40% 98%; | ||
| // --secondary: 240 5% 96%; | ||
| // --secondary-foreground: 240 6% 10%; | ||
| // --muted: 240 5% 96%; | ||
| // --muted-foreground: 240 4% 46%; | ||
| // --accent: 240 5% 96%; | ||
| // --accent-foreground: 240 6% 10%; | ||
| // --destructive: 0 72% 51%; | ||
| // --destructive-foreground: 0 0% 100%; | ||
| // --border: 240 6% 90%; | ||
| // --input: 240 6% 90%; | ||
| // --ring: 240 5% 65%; | ||
|
|
||
| // --table-header: 240 5% 96% · 50%; | ||
| // --warning-foreground: 26 90% 37%; | ||
| // --warning: 48 97% 77%; | ||
| // --warning-border: 48 97% 77%; | ||
| // --success-foreground: 142 72% 29%; | ||
| // --success: 145 80% 10%; | ||
| // --success-border: 141 79% 85%; | ||
|
|
||
| // --information-foreground: 224 76% 48%; | ||
| // --information: 226 57% 21%; | ||
| // --information-border: 213 97% 87%; | ||
|
|
||
| // --error: 0 93% 94%; | ||
| // --error-border: 0 96% 89%; | ||
| // --error-foreground: 0 72% 51%; | ||
|
|
||
| // --radius: 0.5rem; | ||
| // --chart-1: 12 76% 61%; | ||
| // --chart-2: 173 58% 39%; | ||
| // --chart-3: 197 37% 24%; | ||
| // --chart-4: 43 74% 66%; | ||
| // --chart-5: 27 87% 67%; | ||
| // --table-header: 240 5% 96% · 50%; | ||
| // } | ||
|
|
||
| // .dark { | ||
| // --background: 240 10% 4%; | ||
| // --foreground: 0 0% 98%; | ||
| // --card: 240 6% 10%; | ||
| // --card-foreground: 0 0% 100%; | ||
| // --popover: 240 10% 4%; | ||
| // --popover-foreground: 0 0% 100%; | ||
| // --primary: 250 62% 58%; | ||
| // --primary-foreground: 0 0% 100%; | ||
| // --secondary: 240 4% 16%; | ||
| // --secondary-foreground: 0 0% 100%; | ||
| // --muted: 240 5% 96%; | ||
| // --muted-foreground: 240 5% 65%; | ||
| // --accent: 240 4% 16%; | ||
| // --accent-foreground: 0 0% 100%; | ||
| // --destructive: 350 89% 60%; | ||
| // --destructive-foreground: 0 0% 0%; | ||
| // --border: 240 4% 16%; | ||
| // --input: 240 4% 16%; | ||
| // --ring: 240 5% 84%; | ||
|
|
||
| // --table-header: 240 4% 16% · 50%; | ||
| // --warning-foreground: 48 96% 89%; | ||
| // --warning: 22 78% 26%; | ||
| // --warning-border: 22 78% 26%; | ||
| // --success-foreground: 138 76% 97%; | ||
| // --success: 141 84% 93%; | ||
| // --success-border: 144 61% 20%; | ||
|
|
||
| // --information-foreground: 214 95% 93%; | ||
| // --information: 214 95% 93%; | ||
| // --information-border: 224 64% 33%; | ||
|
|
||
| // --error: 350 89% 60%; | ||
| // --error-border: 0 63% 31%; | ||
| // --error-foreground: 0 93% 94%; | ||
|
|
||
| // --chart-1: 220 70% 50%; | ||
| // --chart-2: 160 60% 45%; | ||
| // --chart-3: 30 80% 55%; | ||
| // --chart-4: 280 65% 60%; | ||
| // --chart-5: 340 75% 55%; | ||
| // --table-header: 240 4% 16% · 50%; | ||
| // } | ||
|
|
||
| // .light { | ||
| // --background: 0 0% 100%; | ||
| // --foreground: 240 10% 4%; | ||
| // --card: 240 5% 96%; | ||
| // --card-foreground: 222 47% 11%; | ||
| // --popover: 0 0% 100%; | ||
| // --popover-foreground: 222 47% 11%; | ||
| // --primary: 250 62% 58%; | ||
| // --primary-foreground: 210 40% 98%; | ||
| // --secondary: 240 5% 96%; | ||
| // --secondary-foreground: 240 6% 10%; | ||
| // --muted: 240 5% 96%; | ||
| // --muted-foreground: 240 4% 46%; | ||
| // --accent: 240 5% 96%; | ||
| // --accent-foreground: 240 6% 10%; | ||
| // --destructive: 0 72% 51%; | ||
| // --destructive-foreground: 0 0% 100%; | ||
| // --border: 240 6% 90%; | ||
| // --input: 240 6% 90%; | ||
| // --ring: 240 5% 65%; | ||
|
|
||
| // --table-header: 240 5% 96% · 50%; | ||
| // --warning-foreground: 26 90% 37%; | ||
| // --warning: 48 97% 77%; | ||
| // --warning-border: 48 97% 77%; | ||
| // --success-foreground: 142 72% 29%; | ||
| // --success: 145 80% 10%; | ||
| // --success-border: 141 79% 85%; | ||
| // --information-foreground: 224 76% 48%; | ||
| // --information: 226 57% 21%; | ||
| // --information-border: 213 97% 87%; | ||
| // --error: 0 93% 94%; | ||
| // --error-border: 0 96% 89%; | ||
| // --error-foreground: 0 72% 51%; | ||
| // --radius: 0.5rem; | ||
| // --chart-1: 12 76% 61%; | ||
| // --chart-2: 173 58% 39%; | ||
| // --chart-3: 197 37% 24%; | ||
| // --chart-4: 43 74% 66%; | ||
| // --chart-5: 27 87% 67%; | ||
| // } | ||
| // } |
There was a problem hiding this comment.
💡 Codebase verification
Remove old color system and update custom-cursor component
The old color system can be safely removed as the new design system implements a comprehensive color system with semantic tokens. However:
- Update
custom-cursor.tsxto use the new color token:
- color = "var(--primary-700)",
+ color = "var(--primary)",- Remove the commented color variables in
globals.scssas they've been replaced with a more maintainable system in the design system package.
🔗 Analysis chain
Verify migration of color system before deletion.
Before removing this commented code block:
- Ensure all color values are properly migrated to
@repo/design-system/styles - Note that there are some invalid values that should be corrected if migrated:
- Invalid hex color:
#10C496E - Invalid character
·in--table-headervalues - Duplicate declarations of
--table-header
- Invalid hex color:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify color system migration to design system package
# Search for color variables in design system styles
echo "Searching for color variables in design system package..."
rg -A 1 "(--(primary|error|warning|gray|success|info|background|foreground|card|popover|secondary|muted|accent|destructive|border|input|ring|table-header|chart-[1-5])" "@repo/design-system/styles"
# Check for any remaining usages of the old color system
echo "Checking for usages of old color system..."
rg "var\\(--((primary|error|warning|gray|success|info)-[0-9]{2,3}|chart-[1-5])\\)" "packages/ui/src"
Length of output: 807
Script:
#!/bin/bash
# Search for color variables in design system with fixed regex
echo "Searching in design system..."
rg "\-\-(primary|error|warning|gray|success|info|background|foreground|card|popover|secondary|muted|accent|destructive|border|input|ring|table-header|chart)" "@repo/design-system"
# Check for all potential uses of old color system
echo "Checking all potential color variable usages..."
rg "var\(--" "packages/ui/src"
# Look at custom-cursor implementation
echo "Checking custom-cursor implementation..."
cat "packages/ui/src/components/atoms/custom-cursor.tsx"
Length of output: 4065
Script:
#!/bin/bash
# Find the correct design system package location
echo "Looking for design system package..."
fd -t d "design-system"
# Once found, search for color tokens in all style files
echo "Searching for style files..."
fd -e css -e scss -e less . | grep -i "design"
Length of output: 456
Script:
#!/bin/bash
echo "Checking color system implementation..."
echo "=== Color Palette ==="
cat "packages/design-system/src/styles/base/_palette.scss"
echo -e "\n=== Light Theme Colors ==="
cat "packages/design-system/src/styles/base/_colors-light.scss"
echo -e "\n=== Dark Theme Colors ==="
cat "packages/design-system/src/styles/base/_colors-dark.scss"
# Check for any imports of these files
echo -e "\n=== Checking imports ==="
rg -l "@import.*colors-|@import.*palette" "packages/design-system"
Length of output: 9943
| // Utility to convert HEX to HSL | ||
| 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
Fix file name typo and enhance function implementation.
- The file name contains a typo: 'utills.ts' should be 'utils.ts'
- The function implementation could be improved with:
- Input validation
- JSDoc documentation
- Type safety
Here's the suggested implementation:
+/**
+ * Converts a hexadecimal color string to HSL format.
+ * @param hex - The hexadecimal color string (e.g., "#FF0000")
+ * @returns The color in HSL format (e.g., "0 100% 50%")
+ * @throws {Error} If the hex string is invalid
+ */
export const hexToHsl = (hex: string): string => {
+ // Validate hex format
+ if (!/^#[0-9A-Fa-f]{6}$/.test(hex)) {
+ throw new Error('Invalid hex color format. Expected "#RRGGBB"');
+ }
+
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.
| // Utility to convert HEX to HSL | |
| 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)}%`; | |
| }; | |
| /** | |
| * Converts a hexadecimal color string to HSL format. | |
| * @param hex - The hexadecimal color string (e.g., "#FF0000") | |
| * @returns The color in HSL format (e.g., "0 100% 50%") | |
| * @throws {Error} If the hex string is invalid | |
| */ | |
| export const hexToHsl = (hex: string): string => { | |
| // Validate hex format | |
| if (!/^#[0-9A-Fa-f]{6}$/.test(hex)) { | |
| throw new Error('Invalid hex color format. Expected "#RRGGBB"'); | |
| } | |
| 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)}%`; | |
| }; |
| // Function to build and write the palette.ts file | ||
| export const buildPaletteTailwind = (): void => { | ||
| // Read and parse the JSON file | ||
| const rawData = fs.readFileSync(inputFilePath, "utf-8"); | ||
| const tokens = JSON.parse(rawData); | ||
|
|
||
| // Generate TypeScript content | ||
| const tsContent = generatePaletteTsContent(tokens); | ||
|
|
||
| // Ensure the output directory exists | ||
| const outputDir = path.dirname(outputFilePath); | ||
| if (!fs.existsSync(outputDir)) { | ||
| fs.mkdirSync(outputDir, { recursive: true }); | ||
| } | ||
|
|
||
| // Write the TypeScript content to the file | ||
| fs.writeFileSync(outputFilePath, tsContent, "utf-8"); | ||
| console.log(`palette.ts file created at: ${outputFilePath}`); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and input validation.
Similar to build-palette-style.ts, add proper error handling and validation to prevent runtime failures.
export const buildPaletteTailwind = (): void => {
- // Read and parse the JSON file
- const rawData = fs.readFileSync(inputFilePath, "utf-8");
- const tokens = JSON.parse(rawData);
+ try {
+ if (!fs.existsSync(inputFilePath)) {
+ throw new Error(`Input file not found: ${inputFilePath}`);
+ }
+
+ const rawData = fs.readFileSync(inputFilePath, "utf-8");
+ const tokens = JSON.parse(rawData);
+
+ if (!tokens || typeof tokens !== 'object') {
+ throw new Error('Invalid token file structure');
+ }
- // Generate TypeScript content
- const tsContent = generatePaletteTsContent(tokens);
+ const tsContent = generatePaletteTsContent(tokens);
- // Ensure the output directory exists
- const outputDir = path.dirname(outputFilePath);
- if (!fs.existsSync(outputDir)) {
- fs.mkdirSync(outputDir, { recursive: true });
- }
+ const outputDir = path.dirname(outputFilePath);
+ if (!fs.existsSync(outputDir)) {
+ fs.mkdirSync(outputDir, { recursive: true });
+ }
- // Write the TypeScript content to the file
- fs.writeFileSync(outputFilePath, tsContent, "utf-8");
- console.log(`palette.ts file created at: ${outputFilePath}`);
+ fs.writeFileSync(outputFilePath, tsContent, "utf-8");
+ console.log(`palette.ts file created at: ${outputFilePath}`);
+ } catch (error) {
+ console.error('Failed to build palette Tailwind:', 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 build and write the palette.ts file | |
| export const buildPaletteTailwind = (): void => { | |
| // Read and parse the JSON file | |
| const rawData = fs.readFileSync(inputFilePath, "utf-8"); | |
| const tokens = JSON.parse(rawData); | |
| // Generate TypeScript content | |
| const tsContent = generatePaletteTsContent(tokens); | |
| // Ensure the output directory exists | |
| const outputDir = path.dirname(outputFilePath); | |
| if (!fs.existsSync(outputDir)) { | |
| fs.mkdirSync(outputDir, { recursive: true }); | |
| } | |
| // Write the TypeScript content to the file | |
| fs.writeFileSync(outputFilePath, tsContent, "utf-8"); | |
| console.log(`palette.ts file created at: ${outputFilePath}`); | |
| }; | |
| // Function to build and write the palette.ts file | |
| export const buildPaletteTailwind = (): void => { | |
| try { | |
| if (!fs.existsSync(inputFilePath)) { | |
| throw new Error(`Input file not found: ${inputFilePath}`); | |
| } | |
| const rawData = fs.readFileSync(inputFilePath, "utf-8"); | |
| const tokens = JSON.parse(rawData); | |
| if (!tokens || typeof tokens !== 'object') { | |
| throw new Error('Invalid token file structure'); | |
| } | |
| const tsContent = generatePaletteTsContent(tokens); | |
| const outputDir = path.dirname(outputFilePath); | |
| if (!fs.existsSync(outputDir)) { | |
| fs.mkdirSync(outputDir, { recursive: true }); | |
| } | |
| fs.writeFileSync(outputFilePath, tsContent, "utf-8"); | |
| console.log(`palette.ts file created at: ${outputFilePath}`); | |
| } catch (error) { | |
| console.error('Failed to build palette Tailwind:', error); | |
| process.exit(1); | |
| } | |
| }; |
🧰 Tools
🪛 GitHub Actions: Build Design System
[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.
| const generateScssContent = ( | ||
| theme: "dark" | "light", | ||
| tokens: Record<string, any>, | ||
| ): string => { | ||
| const lines: string[] = []; | ||
|
|
||
| const selector = theme === "dark" ? ":root" : ".light"; | ||
|
|
||
| // Add the @use directive | ||
| lines.push(`@use "_palette.scss" as *;\n`); | ||
|
|
||
| // Start the layer and class | ||
| lines.push(`@layer base {`); | ||
| lines.push(` ${selector} {`); | ||
|
|
||
| // Process each token in the "color" group | ||
| const colorTokens = tokens.color; | ||
| for (const key in colorTokens) { | ||
| const value = colorTokens[key].$value; | ||
|
|
||
| // Convert token reference (e.g., {zinc.200}) to CSS variable (e.g., var(--zinc.200)) | ||
| const resolvedValue = resolveTokenReference(value); | ||
| lines.push(` --${key}: ${resolvedValue};`); | ||
| } | ||
|
|
||
| // Close the class and layer | ||
| lines.push(` }`); | ||
| lines.push(`}`); | ||
|
|
||
| return lines.join("\n"); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for token processing.
The function should validate the token structure and handle potential errors gracefully.
const generateScssContent = (
theme: "dark" | "light",
tokens: Record<string, any>,
): string => {
+ if (!tokens?.color || typeof tokens.color !== 'object') {
+ throw new Error(`Invalid token structure for ${theme} theme: missing or invalid color tokens`);
+ }
const lines: string[] = [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const generateScssContent = ( | |
| theme: "dark" | "light", | |
| tokens: Record<string, any>, | |
| ): string => { | |
| const lines: string[] = []; | |
| const selector = theme === "dark" ? ":root" : ".light"; | |
| // Add the @use directive | |
| lines.push(`@use "_palette.scss" as *;\n`); | |
| // Start the layer and class | |
| lines.push(`@layer base {`); | |
| lines.push(` ${selector} {`); | |
| // Process each token in the "color" group | |
| const colorTokens = tokens.color; | |
| for (const key in colorTokens) { | |
| const value = colorTokens[key].$value; | |
| // Convert token reference (e.g., {zinc.200}) to CSS variable (e.g., var(--zinc.200)) | |
| const resolvedValue = resolveTokenReference(value); | |
| lines.push(` --${key}: ${resolvedValue};`); | |
| } | |
| // Close the class and layer | |
| lines.push(` }`); | |
| lines.push(`}`); | |
| return lines.join("\n"); | |
| }; | |
| const generateScssContent = ( | |
| theme: "dark" | "light", | |
| tokens: Record<string, any>, | |
| ): string => { | |
| if (!tokens?.color || typeof tokens.color !== 'object') { | |
| throw new Error(`Invalid token structure for ${theme} theme: missing or invalid color tokens`); | |
| } | |
| const lines: string[] = []; | |
| const selector = theme === "dark" ? ":root" : ".light"; | |
| // Add the @use directive | |
| lines.push(`@use "_palette.scss" as *;\n`); | |
| // Start the layer and class | |
| lines.push(`@layer base {`); | |
| lines.push(` ${selector} {`); | |
| // Process each token in the "color" group | |
| const colorTokens = tokens.color; | |
| for (const key in colorTokens) { | |
| const value = colorTokens[key].$value; | |
| // Convert token reference (e.g., {zinc.200}) to CSS variable (e.g., var(--zinc.200)) | |
| const resolvedValue = resolveTokenReference(value); | |
| lines.push(` --${key}: ${resolvedValue};`); | |
| } | |
| // Close the class and layer | |
| lines.push(` }`); | |
| lines.push(`}`); | |
| return lines.join("\n"); | |
| }; |
| "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 or rename the inconsistent color shade.
The ali shade in the gray category breaks the numerical naming convention used throughout the palette. Consider removing it or renaming it to follow the established pattern.
- "ali": "var(--gray-ali)",| --success: var(--green-100); | ||
| --success-border: var(--green-200); | ||
| --success-foreground: var(--green-700); | ||
| --table-header: rgba(244, 244, 245, 0.5000); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hardcoded rgba value with a palette variable.
The table header uses a hardcoded rgba value which breaks the pattern of using palette variables. Consider adding this color to your palette or deriving it from existing variables.
- --table-header: rgba(244, 244, 245, 0.5000);
+ --table-header: var(--zinc-100-50);📝 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.
| --table-header: rgba(244, 244, 245, 0.5000); | |
| --table-header: var(--zinc-100-50); |
| --warning: var(--amber-500); | ||
| --warning-border: var(--amber-200); | ||
| --warning-foreground: var(--General-white); |
There was a problem hiding this comment.
💡 Codebase verification
Contrast issue confirmed - white text on amber background needs adjustment
The codebase shows that warning messages use white text (HSL 0 0% 100%) on an amber-500 background (HSL 38 92% 50%). This medium-bright amber with white text likely fails WCAG contrast guidelines. The suggested change to use zinc-900 would provide better contrast, similar to how the dark theme uses amber-100 for better readability.
🔗 Analysis chain
Review warning color contrast.
The warning foreground (white) on warning background (amber-500) might have insufficient contrast for accessibility. Consider using a darker color for better readability:
--warning: var(--amber-500);
--warning-border: var(--amber-200);
- --warning-foreground: var(--General-white);
+ --warning-foreground: var(--zinc-900);🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the contrast ratio meets WCAG guidelines
# Install and use color-contrast-checker
npm install -g color-contrast-checker
echo "Checking contrast between amber-500 and white..."
color-contrast amber-500 white
Length of output: 1966
Script:
#!/bin/bash
# Search for amber-500 color definition
echo "Searching for amber-500 color definition..."
rg --type scss "amber-500" -B 2 -A 2
Length of output: 162
Script:
#!/bin/bash
# Find all scss files and search for color definitions
echo "Finding scss files and searching for color definitions..."
fd . -e scss -x cat {} | grep -A 5 -B 5 "amber-\|--warning"
Length of output: 3171
Scripts
this code change some code in scripts
Summary by CodeRabbit
Based on the comprehensive changes, here are the release notes:
New Features
Improvements
Chores