Test #211
Test #211
Conversation
WalkthroughThis pull request introduces a comprehensive redesign of the design system's styling and build infrastructure. It replaces the existing CSS-based styling with a more modular SCSS approach, adds new build scripts for generating color tokens and Tailwind configurations, and updates the GitHub Actions workflow to automate the design system build process. The changes include creating a new design system package with scripts to generate color palettes, Tailwind configurations, and SCSS styles, while removing the previous leaderboard update workflow. Changes
Sequence DiagramsequenceDiagram
participant DS as Design System Build
participant Tokens as Token Generator
participant SCSS as SCSS Generator
participant Tailwind as Tailwind Config Generator
DS->>Tokens: Read color tokens
Tokens-->>DS: Generate color data
DS->>SCSS: Create color stylesheets
SCSS-->>DS: Generate SCSS files
DS->>Tailwind: Create Tailwind configurations
Tailwind-->>DS: Generate config files
DS->>DS: Compile and output
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: 15
🧹 Nitpick comments (15)
packages/design-system/src/tailwind/config.ts (1)
4-10: Add TypeScript types for better type safety and documentation.Consider adding explicit types for the configuration and color objects to improve maintainability and catch potential issues at compile time.
+import type { Config } from 'tailwindcss' + -export const tailwindConfig = { +export const tailwindConfig: Partial<Config> = { theme: { extend: { - colors: { ...colors, ...palette }, + colors: { ...colors, ...palette } as Record<string, string>, }, }, };packages/design-system/src/scripts/utills.ts (1)
1-1: Fix typo in filename: "utills.ts" should be "utils.ts"The filename contains a typo which should be corrected for consistency and maintainability.
packages/design-system/src/scripts/build-main-style.ts (1)
4-7: Consider making paths configurable through environment variables or configuration file.Hardcoded paths make the script less flexible and harder to maintain.
+import dotenv from 'dotenv'; +dotenv.config(); + // Paths for the output file -const outputDir = path.join(__dirname, "../styles"); -const outputFilePath = path.join(outputDir, "main.scss"); +const outputDir = path.join(__dirname, process.env.STYLES_OUTPUT_DIR || "../styles"); +const outputFilePath = path.join(outputDir, process.env.MAIN_SCSS_FILENAME || "main.scss");packages/design-system/src/scripts/build.ts (1)
32-32: Handle process termination signals.Add proper handling of process termination signals to ensure cleanup is performed.
+// Handle termination signals +process.on('SIGINT', async () => { + console.log('\n🛑 Build interrupted by user'); + await cleanup([]); + process.exit(1); +}); + -main(); +main().catch(error => { + console.error('Unhandled error:', error); + process.exit(1); +});packages/design-system/src/scripts/build-config-tailwind.ts (1)
4-6: Consider using path.resolve for more reliable path resolution.The current path resolution using
path.join(__dirname, ...)might lead to issues in different environments. Usingpath.resolvewould ensure more reliable path resolution.-const outputDir = path.join(__dirname, "../tailwind"); -const outputFilePath = path.join(outputDir, "config.ts"); +const outputDir = path.resolve(__dirname, "../tailwind"); +const outputFilePath = path.resolve(outputDir, "config.ts");packages/ui/tailwind.config.ts (1)
54-57: Consider using import statements for plugins.Using
require()for plugins is inconsistent with the TypeScript module system. Consider using import statements for better type safety.plugins: [ tailwindcssAnimate, - require("tailwind-scrollbar"), - require("tailwind-scrollbar-hide"), + import("tailwind-scrollbar"), + import("tailwind-scrollbar-hide"), ],packages/design-system/src/tailwind/colors.ts (1)
1-35: Add TypeScript types and documentation for color tokens.The color tokens would benefit from type safety and documentation explaining their intended usage.
+/** Color token definitions for the design system */ +type ColorToken = { + [key: string]: string; +}; + -export const colors = { +export const colors: ColorToken = { + /** Primary accent color for interactive elements */ "accent": "hsl(var(--accent))", + /** Contrasting text color for accent backgrounds */ "accent-foreground": "hsl(var(--accent-foreground))", // ... add JSDoc comments for other colors };Also, consider grouping related colors using nested objects for better organization:
export const colors: ColorToken = { accent: { DEFAULT: "hsl(var(--accent))", foreground: "hsl(var(--accent-foreground))", }, // ... group other related colors };packages/design-system/src/scripts/build-palette-style.ts (1)
53-54: Consider making the script more flexible.The script could be more reusable if it accepted command-line arguments for input/output paths.
+import { program } from 'commander'; + +program + .option('-i, --input <path>', 'Input JSON file path', inputFilePath) + .option('-o, --output <path>', 'Output SCSS file path', outputFilePath) + .parse(process.argv); + +const options = program.opts(); + // Run the script -buildPaletteScss(); +buildPaletteScss(options.input, options.output);packages/design-system/src/scripts/build-colors-tailwind.ts (1)
14-14: Add type definitions for token structure.Define an interface for the expected token structure to improve type safety and maintainability.
+interface ColorToken { + $type: string; + $value: string; +} + +interface ColorTokens { + color: Record<string, ColorToken>; +} + -const generateColorsTsContent = (tokens: Record<string, any>): string => { +const generateColorsTsContent = (tokens: ColorTokens): string => {packages/design-system/src/scripts/build-colors-style.ts (1)
12-15: Improve theme handling with type-safe enums.Replace magic strings with a type-safe enum for better maintainability and type checking.
+enum Theme { + Dark = 'dark', + Light = 'light' +} + +const getThemeSelector = (theme: Theme): string => { + switch (theme) { + case Theme.Dark: + return ':root'; + case Theme.Light: + return '.light'; + default: + throw new Error(`Invalid theme: ${theme}`); + } +}; + const generateScssContent = ( - theme: "dark" | "light", + theme: Theme, tokens: Record<string, any>, ): string => { const lines: string[] = []; - const selector = theme === "dark" ? ":root" : ".light"; + const selector = getThemeSelector(theme);Also applies to: 18-18
packages/design-system/src/styles/main.scss (1)
1-2: Consider using namespaced imports for better maintainability.Using
as *could lead to naming conflicts. Consider using namespaces:-@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;.github/workflows/build-design-system.yml (1)
1-34: Enhance the workflow with caching and artifacts.Consider the following improvements:
- Add dependency caching to speed up builds
- Upload build artifacts for verification
- Add error notifications
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 dependencies + uses: actions/cache@v3 + with: + path: packages/design-system/node_modules + key: ${{ runner.os }}-bun-${{ hashFiles('packages/design-system/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 + continue-on-error: false + + - name: Upload build artifacts + if: success() + uses: actions/upload-artifact@v3 + with: + name: design-system-build + path: packages/design-system/dist + + - name: Notify on failure + if: failure() + uses: actions/github-script@v6 + with: + script: | + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.name, + body: '❌ Design system build failed. Please check the workflow logs.' + })packages/design-system/src/styles/base/_colors-light.scss (1)
4-38: Add documentation for semantic color roles.The color tokens are well-organized, but adding documentation would help developers understand their intended usage.
@layer base { .light { + /* Core UI Colors */ --accent: var(--zinc-100); --accent-foreground: var(--zinc-900); --background: var(--General-white); --border: var(--zinc-200); + + /* Component-specific Colors */ --card: var(--zinc-100); --card-foreground: var(--slate-900); --input: var(--zinc-200); --popover: var(--General-white); --popover-foreground: var(--slate-900); + + /* State Colors */ --destructive: var(--red-600); --destructive-foreground: var(--General-white); --error: var(--red-500); --error-border: var(--red-200); --error-foreground: var(--red-600); /* ... rest of the colors ... */ } }packages/design-system/src/styles/base/_palette.scss (1)
1-270: Add documentation for the color scale system.The color palette is well-organized but would benefit from documentation explaining the scale system (50-950).
@layer base { + /* Color Scale System + * 50: Lightest shade, used for backgrounds + * 100-400: Light shades, used for hover states and borders + * 500: Primary shade, used for main UI elements + * 600-900: Dark shades, used for text and emphasis + * 950: Darkest shade, used for high contrast elements + */ :root { --General-black: 0 0% 0%; --General-white: 0 0% 100%; /* ... rest of the colors ... */ } }packages/ui/package.json (1)
66-66: Ensure proper SCSS build setup.The addition of
postcss-scssandsassdependencies indicates a migration from CSS to SCSS. Please ensure:
- PostCSS configuration is updated to handle SCSS files
- Build scripts are modified to process SCSS files
- Development environment is configured for SCSS hot-reloading
Also applies to: 72-72
📜 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 (7)
- apps/core/app/auth/layout.tsx
- packages/ui/src/styles/_tailinwd.scss
- packages/design-system/tsconfig.lint.json
- packages/design-system/tsconfig.json
- packages/design-system/src/tokens/token_Palette_primitive.json
- apps/core/app/layout.tsx
- packages/design-system/package.json
🧰 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 (9)
.hintrc (1)
5-7: Verify TypeScript casing conventions across the project.The addition of this configuration disables consistent casing checks for TypeScript. While this change is valid, we should ensure it aligns with the project's TypeScript guidelines and doesn't conflict with other configurations.
Let's verify the TypeScript configuration setup:
✅ Verification successful
Disabling consistent-casing hint is safe to proceed
After thorough verification of all TypeScript and ESLint configurations across the monorepo, no conflicting casing rules were found. The change in
.hintrcwon't create inconsistencies with other tools or configurations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for TypeScript configuration files and their casing rules echo "Checking for TypeScript configuration files..." fd -e json -e jsonc tsconfig fd -e json -e js -e yaml -e yml -g '*eslint*' echo -e "\nChecking for any casing-related rules in configurations..." rg -l --type json "casing" rg -l --type yaml "casing" echo -e "\nChecking for any style guide documentation..." fd -g 'STYLE*' -g 'CONTRIBUTING*' -g 'README*'Length of output: 899
Script:
#!/bin/bash # Check contents of TypeScript and ESLint configurations echo "Checking TypeScript configurations for casing rules..." for config in $(fd -e json -e jsonc tsconfig); do echo -e "\n=== $config ===" cat "$config" done echo -e "\nChecking for ESLint configurations in common locations..." for file in .eslintrc.json .eslintrc.js .eslintrc.yaml .eslintrc.yml .eslintrc; do if [ -f "$file" ]; then echo -e "\n=== $file ===" cat "$file" fi done echo -e "\nChecking package.json files for ESLint configurations..." for pkg in $(fd package.json); do echo -e "\n=== $pkg ===" cat "$pkg" | jq '.eslintConfig' doneLength of output: 5510
Script:
#!/bin/bash # Check shared TypeScript and ESLint configurations echo "Checking shared TypeScript configurations..." if [ -d "packages/typescript-config" ]; then echo -e "\n=== TypeScript shared configs ===" fd . packages/typescript-config -t f -e json -e js for file in $(fd . packages/typescript-config -t f -e json -e js); do echo -e "\n--- $file ---" cat "$file" done fi echo -e "\nChecking shared ESLint configurations..." if [ -d "packages/eslint-config" ]; then echo -e "\n=== ESLint shared configs ===" fd . packages/eslint-config -t f -e js for file in $(fd . packages/eslint-config -t f -e js); do echo -e "\n--- $file ---" cat "$file" done fiLength of output: 5499
packages/ui/postcss.config.mjs (1)
3-3: LGTM! Verify postcss-scss dependency.The addition of SCSS syntax support is correct for the CSS to SCSS transition.
Let's verify the postcss-scss dependency is properly added:
✅ Verification successful
✅ postcss-scss dependency verified
The postcss-scss dependency is properly added with version ^4.0.9 in package.json.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if postcss-scss is listed in package.json cat packages/ui/package.json | jq '.dependencies["postcss-scss"] // .devDependencies["postcss-scss"]'Length of output: 112
packages/ui/src/styles/globals.scss (4)
212-219: LGTM! Base styles follow Tailwind best practices.The base layer correctly applies global styles using Tailwind's @apply directive.
221-232: LGTM! Well-implemented cross-browser scrollbar utility.The no-scrollbar utility properly handles scrollbar hiding across different browsers.
1-2: LGTM! Verify design system styles package.The imports look correct for the new SCSS structure.
Let's verify the design system styles package exists:
✅ Verification successful
✅ Design system styles package verified and correctly imported
The import
@repo/design-system/stylesis valid and maps tosrc/styles/main.scssin the design system package. The package structure and exports are properly configured.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if design system styles exist fd -t f . "packages/design-system/styles"Length of output: 158
Script:
#!/bin/bash # Check if design-system package exists and list its contents echo "=== Checking for design-system package ===" fd -t d "design-system" packages/ echo -e "\n=== Looking for style files in packages ===" fd -e scss -e css . packages/design-system/ echo -e "\n=== Checking package.json for style references ===" fd package.json packages/design-system/ -x cat {}Length of output: 1011
4-210: Verify migration of color tokens before deletion.The commented section contains a comprehensive set of color tokens and theme variables marked for deletion. Before removing this code, ensure all these values have been properly migrated to the new design system.
Let's verify the migration of these values:
✅ Verification successful
✓ Color system successfully migrated to design system
The commented-out color tokens have been properly migrated to a more structured design system implementation, with comprehensive coverage across JSON tokens, Tailwind configuration, and SCSS variables. The code can be safely deleted.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for color tokens in design system echo "Checking for color tokens in design system..." fd -t f . "packages/design-system" -x grep -l "primary-\|error-\|warning-\|gray-\|success-\|info-" # Check for theme variables echo "Checking for theme variables..." fd -t f . "packages/design-system" -x grep -l "background\|foreground\|card\|popover\|muted\|accent"Length of output: 1027
Script:
#!/bin/bash # Check content of token files echo "=== Color Token Files ===" cat packages/design-system/src/tokens/token_Color_*.json echo -e "\n=== Tailwind Color Configuration ===" cat packages/design-system/src/tailwind/colors.ts echo -e "\n=== SCSS Color Variables ===" cat packages/design-system/src/styles/base/_colors-*.scssLength of output: 15649
packages/ui/tailwind.config.ts (1)
3-3: Verify the import path resolution.Ensure that
@repo/design-system/tailwindis properly configured in your build system and package.json.✅ Verification successful
Import path is correctly configured ✅
The
@repo/design-systemexists as a workspace package inpackages/design-system/with proper tailwind configuration files. The import will resolve correctly in your monorepo setup.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the package and path are properly configured cat package.json | jq '.dependencies["@repo/design-system"] || .devDependencies["@repo/design-system"]'Length of output: 336
Script:
#!/bin/bash # Check package.json content echo "=== Package.json dependencies ===" cat package.json | jq '."dependencies"' echo -e "\n=== Package.json devDependencies ===" cat package.json | jq '."devDependencies"' # Check if the path exists in workspace echo -e "\n=== Searching for tailwind config in design-system ===" fd -t f "tailwind" --full-path "*design-system*"Length of output: 666
Script:
#!/bin/bash # Check workspace packages echo "=== Workspace packages ===" fd -t f "package.json" -d 2 # List all tailwind configs echo -e "\n=== All tailwind configs ===" fd -t f "tailwind" # Check if design-system package exists echo -e "\n=== Design system package ===" fd -t d "design-system"Length of output: 559
packages/design-system/src/styles/base/_palette.scss (1)
76-76: Remove or document the purpose of --gray-ali.The
--gray-alivariable seems inconsistent with the naming convention and lacks context.packages/ui/package.json (1)
72-72: Verify the sass version.The sass version
1.83.4seems unusually high. Let's verify this is the correct version:✅ Verification successful
The sass version 1.83.4 is correct and up to date
This is the current latest stable version of sass from the official maintainers, published on npm.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the latest version of sass npm view sass versions --json | jq -r '.[-1]' # Check for any known issues or breaking changes npm view [email protected] 2>/dev/null || echo "Version does not exist"Length of output: 822
| 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.
Add input validation and improve error handling in hexToHsl function.
The function needs input validation and better error handling for robustness.
-export const hexToHsl = (hex: string): string => {
+export const hexToHsl = (hex: string): string => {
+ // Validate hex color format
+ if (!/^#[0-9A-Fa-f]{6}$/.test(hex)) {
+ throw new Error('Invalid hex color format. Expected format: #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;Also, consider adding JSDoc documentation explaining the mathematical formula:
/**
* Converts a hexadecimal color to HSL format.
*
* The conversion formula:
* 1. Convert hex to RGB (0-1 range)
* 2. Find max/min values to calculate delta
* 3. Calculate hue based on which RGB component is max
* 4. Calculate lightness as average of max and min
* 5. Calculate saturation using delta and lightness
*
* @param hex - The hex color string (format: #RRGGBB)
* @returns HSL values as "H S% L%" string
* @throws {Error} If hex color format is invalid
*/| // 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
Use async/await for file operations and add proper error handling.
The synchronous file operations could block the event loop. Also, file operations should have proper error handling.
// Function to create the main.scss file
-export const buildMainScss = (): void => {
+export const buildMainScss = async (): Promise<void> => {
// Generate the content
const scssContent = generateMainScssContent();
// Ensure the output directory exists
if (!fs.existsSync(outputDir)) {
- fs.mkdirSync(outputDir, { recursive: true });
+ try {
+ await fs.promises.mkdir(outputDir, { recursive: true });
+ } catch (error) {
+ throw new Error(`Failed to create directory ${outputDir}: ${error.message}`);
+ }
}
// Write the content to the file
- fs.writeFileSync(outputFilePath, scssContent, "utf-8");
- console.log(`main.scss file created at: ${outputFilePath}`);
+ try {
+ await fs.promises.writeFile(outputFilePath, scssContent, "utf-8");
+ console.log(`main.scss file created at: ${outputFilePath}`);
+ } catch (error) {
+ throw new Error(`Failed to write file ${outputFilePath}: ${error.message}`);
+ }
};📝 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 = async (): Promise<void> => { | |
| // Generate the content | |
| const scssContent = generateMainScssContent(); | |
| // Ensure the output directory exists | |
| if (!fs.existsSync(outputDir)) { | |
| try { | |
| await fs.promises.mkdir(outputDir, { recursive: true }); | |
| } catch (error) { | |
| throw new Error(`Failed to create directory ${outputDir}: ${error.message}`); | |
| } | |
| } | |
| // Write the content to the file | |
| try { | |
| await fs.promises.writeFile(outputFilePath, scssContent, "utf-8"); | |
| console.log(`main.scss file created at: ${outputFilePath}`); | |
| } catch (error) { | |
| throw new Error(`Failed to write file ${outputFilePath}: ${error.message}`); | |
| } | |
| }; |
| 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 build process with async operations and better error handling.
The build process should be asynchronous and include proper cleanup on failure.
-const main = () => {
+const main = async () => {
+ const completedSteps: string[] = [];
try {
console.log("🎨 Building design system...");
// Styles
console.log("Building SCSS styles...");
- buildPaletteScss();
- buildColorsScss("light");
- buildColorsScss("dark");
- buildMainScss();
+ await buildPaletteScss();
+ completedSteps.push('palette-scss');
+ await buildColorsScss("light");
+ completedSteps.push('colors-light-scss');
+ await buildColorsScss("dark");
+ completedSteps.push('colors-dark-scss');
+ await buildMainScss();
+ completedSteps.push('main-scss');
// Tailwind
console.log("Building Tailwind configurations...");
- buildColorsTailwind();
- buildPaletteTailwind();
- buildTailwindConfig();
+ await buildColorsTailwind();
+ completedSteps.push('colors-tailwind');
+ await buildPaletteTailwind();
+ completedSteps.push('palette-tailwind');
+ await buildTailwindConfig();
+ completedSteps.push('tailwind-config');
console.log("✅ Design system built successfully!");
} catch (error) {
- console.error("❌ Build failed:", error);
+ console.error(`❌ Build failed at step ${completedSteps.length + 1}:`, error);
+ await cleanup(completedSteps);
process.exit(1);
}
};
+
+const cleanup = async (completedSteps: string[]) => {
+ console.log("🧹 Cleaning up failed build...");
+ // Add cleanup logic here based on completed steps
+};Committable suggestion skipped: line range outside the PR's diff.
| const generateTailwindConfigContent = (): string => { | ||
| return ` | ||
| import { colors } from "./colors"; | ||
| import { palette } from "./palette"; | ||
| export const tailwindConfig = { | ||
| theme: { | ||
| extend: { | ||
| colors: { ...colors, ...palette }, | ||
| }, | ||
| }, | ||
| }; | ||
| `.trim(); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety to the configuration template.
The generated configuration lacks type checking which could lead to runtime errors.
-export const tailwindConfig = {
+import type { Config } from "tailwindcss";
+
+export const tailwindConfig: Partial<Config["theme"]> = {
theme: {
extend: {
colors: { ...colors, ...palette },
},
},
};📝 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 generateTailwindConfigContent = (): string => { | |
| return ` | |
| import { colors } from "./colors"; | |
| import { palette } from "./palette"; | |
| export const tailwindConfig = { | |
| theme: { | |
| extend: { | |
| colors: { ...colors, ...palette }, | |
| }, | |
| }, | |
| }; | |
| `.trim(); | |
| }; | |
| const generateTailwindConfigContent = (): string => { | |
| return ` | |
| import { colors } from "./colors"; | |
| import { palette } from "./palette"; | |
| import type { Config } from "tailwindcss"; | |
| export const tailwindConfig: Partial<Config["theme"]> = { | |
| theme: { | |
| extend: { | |
| colors: { ...colors, ...palette }, | |
| }, | |
| }, | |
| }; | |
| `.trim(); | |
| }; |
| export const buildTailwindConfig = (): void => { | ||
| // Generate the content | ||
| const configContent = generateTailwindConfigContent(); | ||
|
|
||
| // Ensure the output directory exists | ||
| if (!fs.existsSync(outputDir)) { | ||
| fs.mkdirSync(outputDir, { recursive: true }); | ||
| } | ||
|
|
||
| // Write the content to the file | ||
| fs.writeFileSync(outputFilePath, configContent, "utf-8"); | ||
| console.log(`tailwindConfig.ts file created at: ${outputFilePath}`); | ||
| }; |
There was a problem hiding this comment.
Add error handling for file operations.
The file operations could fail for various reasons (permissions, disk space, etc.) but there's no error handling.
export const buildTailwindConfig = (): void => {
- // Generate the content
- const configContent = generateTailwindConfigContent();
+ try {
+ // Generate the content
+ const configContent = generateTailwindConfigContent();
- // Ensure the output directory exists
- if (!fs.existsSync(outputDir)) {
- fs.mkdirSync(outputDir, { recursive: true });
- }
+ // Ensure the output directory exists
+ if (!fs.existsSync(outputDir)) {
+ fs.mkdirSync(outputDir, { recursive: true });
+ }
- // Write the content to the file
- fs.writeFileSync(outputFilePath, configContent, "utf-8");
- console.log(`tailwindConfig.ts file created at: ${outputFilePath}`);
+ // Write the content to the file
+ fs.writeFileSync(outputFilePath, configContent, "utf-8");
+ console.log(`tailwindConfig.ts file created at: ${outputFilePath}`);
+ } catch (error) {
+ console.error('Failed to build Tailwind config:', 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.
| export const buildTailwindConfig = (): void => { | |
| // Generate the content | |
| const configContent = generateTailwindConfigContent(); | |
| // Ensure the output directory exists | |
| if (!fs.existsSync(outputDir)) { | |
| fs.mkdirSync(outputDir, { recursive: true }); | |
| } | |
| // Write the content to the file | |
| fs.writeFileSync(outputFilePath, configContent, "utf-8"); | |
| console.log(`tailwindConfig.ts file created at: ${outputFilePath}`); | |
| }; | |
| export const buildTailwindConfig = (): void => { | |
| try { | |
| // Generate the content | |
| const configContent = generateTailwindConfigContent(); | |
| // Ensure the output directory exists | |
| if (!fs.existsSync(outputDir)) { | |
| fs.mkdirSync(outputDir, { recursive: true }); | |
| } | |
| // Write the content to the file | |
| fs.writeFileSync(outputFilePath, configContent, "utf-8"); | |
| console.log(`tailwindConfig.ts file created at: ${outputFilePath}`); | |
| } catch (error) { | |
| console.error('Failed to build Tailwind config:', error); | |
| process.exit(1); | |
| } | |
| }; |
| const resolveTokenReference = (value: string): string => { | ||
| return value.replace(/{(.+?)}/g, (_, match) => `var(--${match.replace(".", "-")})`); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve token reference resolution safety.
The current regex pattern might match unintended patterns. Consider using a more specific pattern and add input validation.
const resolveTokenReference = (value: string): string => {
+ if (typeof value !== 'string') {
+ throw new Error('Token value must be a string');
+ }
- return value.replace(/{(.+?)}/g, (_, match) => `var(--${match.replace(".", "-")})`);
+ // More specific pattern that only matches alphanumeric and dot characters
+ return value.replace(/{([a-zA-Z0-9.-]+)}/g, (_, match) => `var(--${match.replace(".", "-")})`);
};📝 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 resolveTokenReference = (value: string): string => { | |
| return value.replace(/{(.+?)}/g, (_, match) => `var(--${match.replace(".", "-")})`); | |
| }; | |
| const resolveTokenReference = (value: string): string => { | |
| if (typeof value !== 'string') { | |
| throw new Error('Token value must be a string'); | |
| } | |
| // More specific pattern that only matches alphanumeric and dot characters | |
| return value.replace(/{([a-zA-Z0-9.-]+)}/g, (_, match) => `var(--${match.replace(".", "-")})`); | |
| }; |
| "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 non-standard color name.
The color name "gray-ali" breaks the naming convention of the color system. Consider removing it or using standard numeric shade values.
- "ali": "var(--gray-ali)",| "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 group confirmed
The purple color group is indeed missing the "950" shade while all other standard color groups (19 in total) include it. This creates an inconsistency in the color system.
- Add
"950": "var(--purple-950)"to the purple color group inpackages/design-system/src/tailwind/palette.ts
🔗 Analysis chain
Add missing shade to purple color group.
The purple color group is missing the "950" shade that is present in all other color groups. This inconsistency should be addressed in the token file.
Run this script to verify the consistency of color shades across all groups:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistency in color shade definitions
# Find all token files
fd 'token_.*\.json' packages/design-system/src/tokens -x cat {} \; | jq -r '
# Extract all color groups and their shades
.. | objects | select(has("$type") and .["$type"] == "color") | path | .[0]
' | sort | uniq -c
Length of output: 445
Script:
#!/bin/bash
# First, let's find and examine the token files
fd 'token.*\.json$' packages/design-system/src/tokens -x cat {} \; | grep -A 1 -B 1 '"950":'
# Also check the palette.ts file directly to see all color groups
cat packages/design-system/src/tailwind/palette.ts
Length of output: 9068
| :root { | ||
| --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
Use consistent theme implementation approach.
The dark theme is defined on :root while light theme uses a class. This inconsistency could cause issues with theme switching.
@layer base {
- :root {
+ .dark {
--accent: var(--zinc-800);
--accent-foreground: var(--General-white);
/* ... rest of the colors ... */
}
}📝 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.
| :root { | |
| --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); | |
| } | |
| .dark { | |
| --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); | |
| } |
| }, | ||
| "exports": { | ||
| "./globals.css": "./src/globals.css", | ||
| "./globals.scss": "./src/styles/globals.scss", |
There was a problem hiding this comment.
💡 Codebase verification
Update shadcn configuration to use correct globals.scss path
The migration to globals.scss is complete in the codebase imports, but the shadcn configuration in packages/ui/components.json still references the old CSS file. Update the tailwind.css configuration to point to src/styles/globals.scss.
packages/ui/components.json: Change"css": "src/globals.css"to"css": "src/styles/globals.scss"
🔗 Analysis chain
Verify updates to globals.scss imports.
The path for globals has changed from root to src/styles/. Please ensure all imports of this file have been updated accordingly across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining imports of the old globals.css
rg -l "globals\.css"
# Search for imports of the new globals.scss to verify migration
rg -l "globals\.scss"
Length of output: 154
Script:
#!/bin/bash
# Check the content of components.json to understand the globals.css reference
cat packages/ui/components.json
Length of output: 471
desc
Summary by CodeRabbit
Based on the comprehensive changes, here are the release notes:
Release Notes
New Features
Improvements
Chores
Documentation