Skip to content

Conversation

@mrbadri
Copy link
Contributor

@mrbadri mrbadri commented Jan 28, 2025

description

Summary by CodeRabbit

Based on the comprehensive summary of changes, here are the release notes:

  • New Features

    • Introduced a new Design System package with comprehensive color palette and theming support
    • Added automated build workflows for design system components
    • Implemented SCSS-based styling with dynamic color generation
  • Infrastructure

    • Updated GitHub Actions workflows
    • Migrated from CSS to SCSS for global styling
    • Enhanced build and configuration scripts for design tokens
  • Styling

    • Implemented light and dark theme color variables
    • Added utility classes for scrollbar management
    • Integrated Tailwind CSS configuration with design system colors
  • Chores

    • Removed leaderboard update workflow
    • Updated package configurations
    • Improved TypeScript and linting configurations

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 28, 2025

Walkthrough

This pull request introduces a comprehensive design system implementation for a monorepo project. The changes focus on creating a robust, modular styling approach using SCSS, Tailwind CSS, and a set of build scripts that generate color tokens, styles, and configurations dynamically. The implementation includes workflow modifications, utility scripts, color palette generation, and updated styling strategies across multiple packages.

Changes

File/Directory Change Summary
.github/workflows/ Added build-design-system.yml workflow, removed update-leaderboard.yml
packages/design-system/ Created comprehensive build scripts for color tokens, styles, and Tailwind configurations
packages/ui/ Migrated from CSS to SCSS, updated styling approach and dependencies
packages/design-system/src/styles/ Added color palette and theme-specific SCSS files
packages/design-system/src/tailwind/ Generated color and configuration files for Tailwind integration

Sequence Diagram

sequenceDiagram
    participant Build Script
    participant Token JSON
    participant Color Generator
    participant SCSS Generator
    participant Tailwind Config Generator

    Build Script->>Token JSON: Read color tokens
    Token JSON-->>Color Generator: Provide token data
    Color Generator->>SCSS Generator: Generate color styles
    Color Generator->>Tailwind Config Generator: Generate color configurations
    SCSS Generator-->>Build Script: Write SCSS files
    Tailwind Config Generator-->>Build Script: Write Tailwind config
Loading

Possibly related PRs

Poem

🐰 Hop, hop, hooray! Design system's here to play,
Colors dancing, tokens bright,
SCSS and Tailwind taking flight!
From dark to light, a palette so neat,
Our styling journey is now complete! 🎨

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@mrbadri mrbadri closed this Jan 28, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

🧹 Nitpick comments (22)
packages/design-system/src/styles/base/_colors-dark.scss (3)

1-4: Consider using a specific alias for the palette import.

Instead of using as * which imports all variables globally, consider using a specific alias (e.g., as palette) to prevent potential naming conflicts and improve code maintainability.

-@use "_palette.scss" as *;
+@use "_palette.scss" as palette;

11-15: Document the distinction between destructive and error states.

The destructive and error states use the same base color (rose-500). Consider adding a comment to clarify their distinct use cases, or if they serve the same purpose, consider consolidating them to reduce redundancy.

+  /* Destructive is used for permanent actions like delete */
   --destructive: var(--rose-500);
   --destructive-foreground: var(--General-black);
+  /* Error is used for form validation and system errors */
   --error: var(--rose-500);

Also applies to: 31-37


25-35: Replace hardcoded rgba value with a variable.

The table header uses a hardcoded rgba value. Consider defining this in the palette and referencing it consistently with other colors.

-    --table-header: rgba(39, 39, 42, 0.5000);
+    --table-header: var(--zinc-900-50);

This assumes you'll add the corresponding variable to _palette.scss.

packages/design-system/src/tailwind/config.ts (1)

1-10: Add type definitions for imported color objects.

The configuration looks clean, but consider adding TypeScript interfaces for the imported color objects to ensure type safety and better IDE support.

+interface ColorDefinition {
+  [key: string]: string | ColorDefinition;
+}
+
-import { colors } from "./colors";
-import { palette } from "./palette";
+import { colors } from "./colors";
+import { palette } from "./palette";
+
+const typedColors = colors as ColorDefinition;
+const typedPalette = palette as ColorDefinition;

 export const tailwindConfig = {
   theme: {
     extend: {
-      colors: { ...colors, ...palette },
+      colors: { ...typedColors, ...typedPalette },
     },
   },
 };
packages/design-system/src/scripts/build-main-style.ts (1)

4-7: Extract path configuration to a separate config file.

The hardcoded paths should be moved to a configuration file for better maintainability and reusability.

+// config.ts
+export const paths = {
+  outputDir: path.join(__dirname, "../styles"),
+  baseStyles: {
+    dark: "_colors-dark.scss",
+    light: "_colors-light.scss"
+  }
+};
packages/design-system/src/scripts/build.ts (2)

8-30: Enhance error handling with more detailed information.

While the error handling is present, it could be more informative for debugging purposes.

Consider enhancing the error handling:

 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);
+    console.error("❌ Build failed:");
+    if (error instanceof Error) {
+      console.error("Error message:", error.message);
+      console.error("Stack trace:", error.stack);
+    } else {
+      console.error("Unknown error:", error);
+    }
     process.exit(1);
   }
 };

14-17: Consider adding progress indicators for long-running tasks.

For better user experience during the build process, consider adding progress indicators for each step.

-    buildPaletteScss();
-    buildColorsScss("light");
-    buildColorsScss("dark");
-    buildMainScss();
+    console.log("  ⚡ Building palette...");
+    buildPaletteScss();
+    console.log("  ⚡ Building light theme...");
+    buildColorsScss("light");
+    console.log("  ⚡ Building dark theme...");
+    buildColorsScss("dark");
+    console.log("  ⚡ Building main styles...");
+    buildMainScss();
packages/design-system/src/scripts/build-config-tailwind.ts (1)

39-40: Remove self-executing function for better modularity.

Self-executing functions can make testing difficult and reduce module reusability.

-// Run the script
-buildTailwindConfig();
+// Export the function as the default export if this file is run directly
+if (require.main === module) {
+  buildTailwindConfig();
+}
packages/ui/tailwind.config.ts (1)

Line range hint 8-14: Consider extracting content paths to a separate configuration.

The content paths could be more maintainable if extracted to a separate configuration file.

+// contentPaths.ts
+export const contentPaths = [
+  "./pages/**/*.{ts,tsx}",
+  "./components/**/*.{ts,tsx}",
+  "./app/**/*.{ts,tsx}",
+  "./src/**/*.{ts,tsx}",
+  "../../packages/ui/src/**/*.{ts,tsx}",
+] as const;

 // In tailwind.config.ts
-  content: [
-    "./pages/**/*.{ts,tsx}",
-    "./components/**/*.{ts,tsx}",
-    "./app/**/*.{ts,tsx}",
-    "./src/**/*.{ts,tsx}",
-    "../../packages/ui/src/**/*.{ts,tsx}",
-  ],
+  content: contentPaths,
packages/design-system/src/tailwind/colors.ts (1)

1-35: Add TypeScript types and documentation for the color system.

The color system would benefit from proper TypeScript types and documentation explaining the color system's structure and usage.

+/**
+ * Semantic color system using CSS custom properties.
+ * Each color is defined in HSL format for flexible manipulation.
+ * 
+ * @example
+ * ```css
+ * .element {
+ *   background-color: var(--primary);
+ *   color: var(--primary-foreground);
+ * }
+ * ```
+ */
+
+type ColorToken = `hsl(var(--${string}))`;
+
+export const colors: Record<string, ColorToken> = {
   "accent": "hsl(var(--accent))",
   "accent-foreground": "hsl(var(--accent-foreground))",
   // ... rest of the colors
 };
packages/design-system/src/scripts/build-colors-tailwind.ts (2)

9-11: Add input validation to resolveTokenReference.

The function should validate the input key format.

-const resolveTokenReference = (key: string): string => {
+const resolveTokenReference = (key: string): string => {
+  if (!key || typeof key !== 'string') {
+    throw new Error('Invalid token key');
+  }
+  if (!/^[a-zA-Z0-9-_.]+$/.test(key)) {
+    throw new Error('Invalid token key format');
+  }
   return `hsl(var(--${key}))`;
 };

33-61: Enhance error handling in buildColorsTailwind.

Add try-catch blocks and improve error messages.

 export const buildColorsTailwind = (): void => {
   const inputFilePath = path.join(tokensDir, `token_Color_Light.json`);
   const outputFilePath = path.join(outputDir, `colors.ts`);
 
   if (!fs.existsSync(inputFilePath)) {
     console.error(`Token file not found: ${inputFilePath}`);
-    return;
+    process.exit(1);
   }
 
+  try {
     // Read and parse the JSON file
     const rawData = fs.readFileSync(inputFilePath, "utf-8");
-    const tokens = JSON.parse(rawData);
+    let tokens;
+    try {
+      tokens = JSON.parse(rawData);
+    } catch (error) {
+      throw new Error(`Invalid JSON in token file: ${error.message}`);
+    }
 
     // Generate TypeScript content
     const tsContent = generateColorsTsContent(tokens);
 
     // Ensure the output directory exists
     if (!fs.existsSync(outputDir)) {
-      fs.mkdirSync(outputDir, { recursive: true });
+      try {
+        fs.mkdirSync(outputDir, { recursive: true });
+      } catch (error) {
+        throw new Error(`Failed to create output directory: ${error.message}`);
+      }
     }
 
     // Write the TypeScript content to the file
     fs.writeFileSync(outputFilePath, tsContent, "utf-8");
     console.log(`colors.ts file created at: ${outputFilePath}`);
+  } catch (error) {
+    console.error('Error building colors Tailwind:', error);
+    process.exit(1);
+  }
 };
packages/design-system/src/scripts/build-colors-style.ts (3)

7-9: Improve token reference resolution.

The function should validate input and use a more specific regex pattern.

+const TOKEN_REFERENCE_PATTERN = /\{([a-zA-Z0-9.-]+)\}/g;
+
 const resolveTokenReference = (value: string): string => {
-  return value.replace(/{(.+?)}/g, (_, match) => `var(--${match.replace(".", "-")})`);
+  if (!value || typeof value !== 'string') {
+    throw new Error('Invalid token value');
+  }
+  return value.replace(TOKEN_REFERENCE_PATTERN, (_, match) => {
+    if (!/^[a-zA-Z0-9.-]+$/.test(match)) {
+      throw new Error(`Invalid token reference format: ${match}`);
+    }
+    return `var(--${match.replace(".", "-")})`;
+  });
 };

45-75: Enhance error handling in buildColorsScss.

Add comprehensive error handling and theme validation.

+const VALID_THEMES = ['dark', 'light'] as const;
+type Theme = typeof VALID_THEMES[number];
+
-export const buildColorsScss = (theme: "dark" | "light"): void => {
+export const buildColorsScss = (theme: Theme): void => {
+  if (!VALID_THEMES.includes(theme)) {
+    throw new Error(`Invalid theme: ${theme}`);
+  }
+
   const partFileName = theme === "dark" ? "Default" : "Light";
   const inputFilePath = path.join(
     tokensDir,
     `token_Color_${partFileName}.json`,
   );
   const outputFilePath = path.join(stylesDir, `_colors-${theme}.scss`);
 
   if (!fs.existsSync(inputFilePath)) {
     console.error(`Token file not found: ${inputFilePath}`);
-    return;
+    process.exit(1);
   }
 
+  try {
     const rawData = fs.readFileSync(inputFilePath, "utf-8");
-    const tokens = JSON.parse(rawData);
+    let tokens;
+    try {
+      tokens = JSON.parse(rawData);
+    } catch (error) {
+      throw new Error(`Invalid JSON in token file: ${error.message}`);
+    }
 
     const scssContent = generateScssContent(theme, tokens);
 
     if (!fs.existsSync(stylesDir)) {
-      fs.mkdirSync(stylesDir, { recursive: true });
+      try {
+        fs.mkdirSync(stylesDir, { recursive: true });
+      } catch (error) {
+        throw new Error(`Failed to create styles directory: ${error.message}`);
+      }
     }
 
     fs.writeFileSync(outputFilePath, scssContent, "utf-8");
     console.log(`SCSS file created for ${theme} theme at: ${outputFilePath}`);
+  } catch (error) {
+    console.error(`Error building ${theme} theme:`, error);
+    process.exit(1);
+  }
 };

77-79: Implement proper module execution pattern.

Wrap the script execution in a module check to prevent unintended execution when imported.

-// Reusable calls for light and dark themes
-buildColorsScss("light");
-buildColorsScss("dark");
+// Only run if this is the main module
+if (require.main === module) {
+  try {
+    buildColorsScss("light");
+    buildColorsScss("dark");
+  } catch (error) {
+    console.error('Failed to build color styles:', error);
+    process.exit(1);
+  }
+}
packages/design-system/src/tailwind/palette.ts (1)

162-173: Add missing "950" shade to purple palette.

All other color categories include a "950" shade, but it's missing from the purple palette. This inconsistency might cause issues in dark mode or when deep color variations are needed.

   "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)",
+    "950": "var(--purple-950)",
   },
.github/workflows/build-design-system.yml (2)

26-30: Add dependency caching to improve workflow performance.

Adding cache for Bun dependencies will significantly speed up the workflow.

   # Step 3: Navigate to the design-system package and install dependencies
+  - name: Cache dependencies
+    uses: actions/cache@v3
+    with:
+      path: ~/.bun/install/cache
+      key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }}
+      restore-keys: |
+        ${{ runner.os }}-bun-
+
   - name: Install dependencies
     working-directory: ./packages/design-system
     run: bun install

31-34: Upload build artifacts for verification.

Consider uploading the build artifacts for verification and debugging purposes.

   # Step 4: Run the build script
   - 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/dist
packages/design-system/src/styles/base/_colors-light.scss (4)

1-1: Consider using an explicit namespace for better maintainability.

Using @use "_palette.scss" as * with a wildcard namespace could lead to naming conflicts. Consider using an explicit namespace like @use "_palette.scss" as palette for better code maintainability and clarity.

-@use "_palette.scss" as *;
+@use "_palette.scss" as palette;

7-7: Standardize color naming conventions.

The variable --General-white is inconsistent with the color-number naming pattern used elsewhere (e.g., --zinc-100). Consider standardizing to use the same pattern throughout.

-    --background: var(--General-white);
+    --background: var(--white-50);
-    --popover: var(--General-white);
+    --popover: var(--white-50);

Also applies to: 23-23


34-34: Use consistent opacity notation.

The table header opacity is written as 0.5000 which is unnecessarily verbose. Consider using the standard 0.5 format.

-    --table-header: rgba(244, 244, 245, 0.5000);
+    --table-header: rgba(244, 244, 245, 0.5);

5-37: Add documentation for semantic color usage.

Consider adding comments to document the intended usage of each semantic color variable. This will help other developers understand when to use each color token appropriately.

Example documentation format:

/** Primary colors - Used for main actions and key UI elements */
--primary: var(--purple-500);
--primary-foreground: var(--General-white);
--primary-light: var(--purple-100);

/** Feedback colors - Used for system messages and states */
--success: var(--green-100);
--error: var(--red-500);
--warning: var(--amber-500);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1f5f6c and a4f31b1.

⛔ Files ignored due to path filters (1)
  • bun.lockb is 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
  • packages/ui/src/globals.css
  • .github/workflows/update-leaderboard.yml
✅ Files skipped from review due to trivial changes (9)
  • apps/core/app/layout.tsx
  • packages/ui/src/styles/_tailinwd.scss
  • packages/design-system/src/styles/main.scss
  • apps/core/app/auth/layout.tsx
  • packages/design-system/tsconfig.json
  • packages/design-system/tsconfig.lint.json
  • packages/design-system/package.json
  • packages/design-system/src/tokens/token_Palette_primitive.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 (10)
.hintrc (1)

5-7: Please verify and document the reason for disabling consistent-casing.

Disabling the TypeScript consistent-casing rule could lead to inconsistent naming patterns across the codebase. If this is intentional, please:

  1. Document the reason for disabling this rule
  2. Consider using a more targeted approach, such as disabling it only for specific patterns or files if possible

Let's check the TypeScript files in the codebase to understand if there are specific naming patterns that necessitate this change:

✅ Verification successful

Disabling consistent-casing rule appears justified but needs documentation

The codebase shows multiple naming conventions across components (e.g., 'after-Effect.tsx' vs 'after-effects.tsx'), indicating this rule was disabled to accommodate existing patterns. However, please add a comment in .hintrc explaining this decision to help future maintainers understand the rationale.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Analyze TypeScript files for naming patterns that might conflict with consistent-casing rule

# Search for all TypeScript files and analyze their naming patterns
echo "Analyzing TypeScript file names:"
fd -e ts -e tsx --type f --exec basename {} \;

# Look for potential casing conflicts in exports and declarations
echo -e "\nAnalyzing export declarations for casing patterns:"
ast-grep --pattern 'export $$$' -l ts,tsx

echo -e "\nAnalyzing type/interface declarations for casing patterns:"
ast-grep --pattern '(type|interface) $_ $$$' -l ts,tsx

Length of output: 4558


Script:

#!/bin/bash
# Check export patterns with correct language specification
echo "Analyzing TypeScript exports:"
ast-grep --pattern 'export $$$' --lang typescript

echo -e "\nAnalyzing React component exports:"
ast-grep --pattern 'export $$$' --lang tsx

echo -e "\nAnalyzing type/interface declarations:"
ast-grep --pattern '(type|interface) $_ $$$' --lang typescript

Length of output: 1720

packages/design-system/src/styles/base/_colors-dark.scss (3)

5-10: Well-structured base UI color definitions!

The color definitions follow a consistent naming pattern and use semantic variable names. The use of zinc colors for neutral shades is appropriate for a dark theme.


1-39: Verify consistency with light theme color tokens.

Ensure that all color tokens defined here have corresponding variables in the light theme for proper theme switching.

✅ Verification successful

Color tokens are consistent across themes

All color variables are properly defined in both light and dark theme files, ensuring complete theme switching support.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Compare color tokens between dark and light themes

# Find the light theme file
LIGHT_THEME=$(fd -t f colors-light.scss)

if [ -n "$LIGHT_THEME" ]; then
    echo "Comparing color tokens between themes..."
    # Extract variable names from both files and compare
    DARK_VARS=$(grep -o -- "--[a-zA-Z-]*:" packages/design-system/src/styles/base/_colors-dark.scss | sort)
    LIGHT_VARS=$(grep -o -- "--[a-zA-Z-]*:" "$LIGHT_THEME" | sort)
    
    echo "Variables only in dark theme:"
    comm -23 <(echo "$DARK_VARS") <(echo "$LIGHT_VARS")
    
    echo "Variables only in light theme:"
    comm -13 <(echo "$DARK_VARS") <(echo "$LIGHT_VARS")
fi

Length of output: 4094


16-24: Verify color contrast ratios for accessibility.

While the color choices appear appropriate, it's important to verify that they meet WCAG accessibility guidelines for contrast ratios, especially for text elements.

✅ Verification successful

Color contrast appears to meet accessibility standards

The color system uses established design tokens with proven contrast ratios:

  • Light foreground (zinc-50) on dark background (zinc-950) follows accessibility best practices
  • Semantic colors use appropriate contrast combinations (e.g., blue-100 on dark backgrounds)
  • Consistent use of the zinc gray scale ensures readable text at different emphasis levels
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if there are any color contrast checking tools in the project

# Look for contrast ratio checking tools in package.json files
fd package.json | xargs cat | jq '.dependencies + .devDependencies' | grep -i "contrast\|a11y\|accessibility"

# Look for contrast ratio related tests
rg -i "contrast ratio|wcag|accessibility" -g "*.test.*"

Length of output: 318


Script:

#!/bin/bash
# Search for color definition files
echo "=== Color definition files ==="
fd -e scss -e css | grep -i "color"

echo -e "\n=== Color-related tests ==="
# Broader search for test files
fd -e test.js -e test.ts -e test.tsx -e spec.js -e spec.ts -e spec.tsx

echo -e "\n=== Accessibility-related files ==="
# Search for accessibility-related files
rg -l -i "accessibility|a11y|contrast|wcag"

Length of output: 574


Script:

#!/bin/bash
echo "=== 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

echo -e "\n=== Accessibility implementations ==="
rg -A 5 -B 5 -i "accessibility|a11y|contrast|wcag" apps/core/app/\(landing\)/layout.tsx packages/icons/src/components/avatar.tsx

Length of output: 69521

packages/ui/postcss.config.mjs (1)

3-3: Verify postcss-scss dependency installation.

The addition of SCSS syntax support is correct, but ensure the postcss-scss package is properly listed in the dependencies.

✅ Verification successful

postcss-scss dependency is properly installed

The postcss-scss package is correctly listed in the package.json with version ^4.0.9.

🏁 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/design-system/src/tailwind/colors.ts (1)

1-35: Verify color variable definitions exist in SCSS files.

Ensure that all CSS custom properties used in the colors object are properly defined in the SCSS theme files.

✅ Verification successful

✓ All color variables are properly defined in theme files

All CSS custom properties used in colors.ts are correctly defined in both light and dark theme files at:

  • packages/design-system/src/styles/base/_colors-dark.scss
  • packages/design-system/src/styles/base/_colors-light.scss
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if all color variables are defined in SCSS files

# Extract CSS variable names from colors.ts
echo "Extracting CSS variable names from colors.ts..."
VARS=$(cat packages/design-system/src/tailwind/colors.ts | grep -o "'--[^)]*'" | tr -d "'" | sort -u)

# Check if these variables are defined in SCSS files
echo "Checking SCSS files for variable definitions..."
for VAR in $VARS; do
  echo "Checking for $VAR..."
  rg "$VAR:" packages/design-system/src/styles/
done

Length of output: 322


Script:

#!/bin/bash
# Extract CSS variable names and check their definitions

# Extract variables (removing quotes and "hsl(var(--" prefix)
echo "Color variables in colors.ts:"
grep -o "var(--[^)]*" packages/design-system/src/tailwind/colors.ts | sed 's/var(--//' | sort -u

echo -e "\nSearching for variable definitions in style files:"
# Find all potential style files
fd -e scss -e css -e sass . | while read -r file; do
  echo "=== Checking $file ==="
  # Search for variable definitions with and without -- prefix
  grep -H -E "^[[:space:]]*(--|--)?(accent|background|border|card|destructive|error|foreground|information|input|muted|popover|primary|ring|secondary|success|table-header|warning)(-[a-z]+)?:" "$file" 2>/dev/null
done

Length of output: 9446

packages/ui/package.json (1)

27-28: Consider versioning strategy for dependencies.

  1. Internal packages use wildcard versions (*), which could lead to unexpected breaking changes.
  2. Multiple styling-related dependencies (postcss-scss, sass) are added. Ensure they're all necessary.

Also applies to: 66-66, 72-72

✅ Verification successful

Internal package versioning is correct, and styling dependencies are justified

The wildcard versions (*) for internal packages are a common and valid pattern in monorepos, as evidenced by consistent usage across all packages. Both postcss-scss and sass serve different purposes in the SCSS processing pipeline and are actively used with the existing SCSS files in both ui and design-system packages.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for potential duplicate functionality in dependencies
npm ls postcss-scss sass | grep -E "postcss-scss|sass"

# Check if all new dependencies are being used
rg -l "postcss-scss|sass" --type=ts --type=js --type=json

Length of output: 141


Script:

#!/bin/bash
# Check all package.json files and their dependency versions
fd package.json --type f --exec cat {} \; | jq -r '.dependencies?, .devDependencies? | to_entries[] | "\(.key): \(.value)"' 2>/dev/null

# Look for scss/css files and build configs
fd -e scss -e css -e postcss.config.js -e sass.config.js

Length of output: 3387

packages/ui/src/styles/globals.scss (2)

221-232: LGTM: Well-documented scrollbar utilities.

The scrollbar utilities are well-implemented with clear documentation for browser compatibility.


4-210: 🛠️ Refactor suggestion

Remove commented code marked for deletion.

Large blocks of commented code make the file harder to maintain. Since this is marked with "TODO: Delete", it should be removed if it's no longer needed.

✅ Verification successful

Remove commented CSS variables as they've been migrated to design system

The commented CSS variables have been successfully migrated to packages/design-system/src/styles/base/ in dedicated files (_palette.scss, _colors-light.scss, _colors-dark.scss). The commented code can be safely removed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if these CSS variables are still used anywhere
rg --type=css --type=scss -l "\-\-(primary|error|warning|gray|success|info)"

Length of output: 107


Script:

#!/bin/bash
# Search for CSS variable usage across all file types
rg "\-\-(primary|error|warning|gray|success|info)" -g "*.{css,scss,jsx,tsx,js,ts}"

# Search for @layer base usage
rg "@layer\s+base" -g "*.{css,scss,jsx,tsx,js,ts}"

Length of output: 13731

packages/design-system/src/styles/base/_colors-light.scss (1)

1-39: Verify color palette implementation.

The color system looks well-structured, but let's verify that all necessary color tokens from _palette.scss are properly defined and imported.

✅ Verification successful

✓ Color palette implementation is correct

All color variables used in the light theme are properly defined in the palette, following a consistent and well-structured approach. The implementation correctly maps semantic color tokens to the base color palette.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify color palette implementation

# Check if all referenced 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 | sort -u > used_colors.txt
rg "^[[:space:]]*--" packages/design-system/src/styles/base/_palette.scss | sort -u > defined_colors.txt
comm -23 used_colors.txt defined_colors.txt

Length of output: 414


Script:

#!/bin/bash
# First, verify _palette.scss exists and show its content
echo "=== Content of _palette.scss ==="
cat packages/design-system/src/styles/base/_palette.scss

echo -e "\n=== Color variables used in _colors-light.scss ==="
rg -o "var\((--[^)]+)\)" packages/design-system/src/styles/base/_colors-light.scss | sort -u | sed 's/var(//g; s/)//g'

echo -e "\n=== Color variables defined in _palette.scss ==="
rg "^\s*--" packages/design-system/src/styles/base/_palette.scss | sed 's/:.*//' | sort -u

Length of output: 10896

Comment on lines +1 to +26
// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add input validation and support for additional hex formats.

The conversion logic is correct but needs additional error handling and format support:

  1. Add input validation for hex format
  2. Support shorthand hex (#RGB)
  3. Support alpha channel (#RGBA)
  4. Use more precise calculations
 // Utility to convert HEX to HSL
-export const hexToHsl = (hex: string): string => {
+export const hexToHsl = (hex: string): string | never => {
+  // Validate hex format
+  const hexRegex = /^#([A-Fa-f0-9]{3}|[A-Fa-f0-9]{4}|[A-Fa-f0-9]{6}|[A-Fa-f0-9]{8})$/;
+  if (!hexRegex.test(hex)) {
+    throw new Error('Invalid hex color format');
+  }
+
+  // Normalize hex to 6 characters
+  let normalizedHex = hex.slice(1);
+  if (normalizedHex.length === 3 || normalizedHex.length === 4) {
+    normalizedHex = normalizedHex
+      .split('')
+      .map(char => char + char)
+      .join('');
+  }
+
+  // Extract RGB values with higher precision
+  const r = parseInt(normalizedHex.slice(0, 2), 16) / 255;
+  const g = parseInt(normalizedHex.slice(2, 4), 16) / 255;
+  const b = parseInt(normalizedHex.slice(4, 6), 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));
+  // Avoid division by zero for saturation
+  const s = l === 1 || l === 0 ? 0 : delta / (1 - Math.abs(2 * l - 1));

-  return `${Math.round(h)} ${Math.round(s * 100)}% ${Math.round(l * 100)}%`;
+  // Use toFixed for consistent decimal places
+  return `${h.toFixed(1)} ${(s * 100).toFixed(1)}% ${(l * 100).toFixed(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.

Suggested change
// 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)}%`;
};
// Utility to convert HEX to HSL
export const hexToHsl = (hex: string): string | never => {
// Validate hex format
const hexRegex = /^#([A-Fa-f0-9]{3}|[A-Fa-f0-9]{4}|[A-Fa-f0-9]{6}|[A-Fa-f0-9]{8})$/;
if (!hexRegex.test(hex)) {
throw new Error('Invalid hex color format');
}
// Normalize hex to 6 characters
let normalizedHex = hex.slice(1);
if (normalizedHex.length === 3 || normalizedHex.length === 4) {
normalizedHex = normalizedHex
.split('')
.map(char => char + char)
.join('');
}
// Extract RGB values with higher precision
const r = parseInt(normalizedHex.slice(0, 2), 16) / 255;
const g = parseInt(normalizedHex.slice(2, 4), 16) / 255;
const b = parseInt(normalizedHex.slice(4, 6), 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;
// Avoid division by zero for saturation
const s = l === 1 || l === 0 ? 0 : delta / (1 - Math.abs(2 * l - 1));
// Use toFixed for consistent decimal places
return `${h.toFixed(1)} ${(s * 100).toFixed(1)}% ${(l * 100).toFixed(1)}%`;
};

Comment on lines +31 to +32
// Run the script
buildMainScss();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid automatic execution on import.

The script should not run automatically when imported. Consider making it a CLI tool or exporting it as a module.

-// Run the script
-buildMainScss();
+if (require.main === module) {
+  buildMainScss();
+}
📝 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.

Suggested change
// Run the script
buildMainScss();
if (require.main === module) {
buildMainScss();
}

Comment on lines +16 to +29
// 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}`);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling and file existence validation.

The file operations need proper error handling and validation of imported files.

 export const buildMainScss = (): void => {
+  try {
     // Generate the content
     const scssContent = generateMainScssContent();
 
+    // Validate that imported files exist
+    const darkStylePath = path.join(outputDir, "base", "_colors-dark.scss");
+    const lightStylePath = path.join(outputDir, "base", "_colors-light.scss");
+    
+    if (!fs.existsSync(darkStylePath) || !fs.existsSync(lightStylePath)) {
+      throw new Error("Required style files are missing");
+    }
+
     // 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}`);
+  } catch (error) {
+    console.error("Error building main.scss:", error);
+    process.exit(1);
+  }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Function to create the main.scss file
export const buildMainScss = (): void => {
// Generate the content
const scssContent = generateMainScssContent();
// Ensure the output directory exists
if (!fs.existsSync(outputDir)) {
fs.mkdirSync(outputDir, { recursive: true });
}
// Write the content to the file
fs.writeFileSync(outputFilePath, scssContent, "utf-8");
console.log(`main.scss file created at: ${outputFilePath}`);
};
// Function to create the main.scss file
export const buildMainScss = (): void => {
try {
// Generate the content
const scssContent = generateMainScssContent();
// Validate that imported files exist
const darkStylePath = path.join(outputDir, "base", "_colors-dark.scss");
const lightStylePath = path.join(outputDir, "base", "_colors-light.scss");
if (!fs.existsSync(darkStylePath) || !fs.existsSync(lightStylePath)) {
throw new Error("Required style files are missing");
}
// 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}`);
} catch (error) {
console.error("Error building main.scss:", error);
process.exit(1);
}
};

Comment on lines +34 to +36
// Write the content to the file
fs.writeFileSync(outputFilePath, configContent, "utf-8");
console.log(`tailwindConfig.ts file created at: ${outputFilePath}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add file write error handling.

File operations should include proper error handling to provide meaningful feedback.

-  // Write the content to the file
-  fs.writeFileSync(outputFilePath, configContent, "utf-8");
-  console.log(`tailwindConfig.ts file created at: ${outputFilePath}`);
+  try {
+    fs.writeFileSync(outputFilePath, configContent, "utf-8");
+    console.log(`✅ tailwindConfig.ts file created at: ${outputFilePath}`);
+  } catch (error) {
+    throw new Error(`Failed to write Tailwind config file: ${error instanceof Error ? error.message : String(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.

Suggested change
// Write the content to the file
fs.writeFileSync(outputFilePath, configContent, "utf-8");
console.log(`tailwindConfig.ts file created at: ${outputFilePath}`);
try {
fs.writeFileSync(outputFilePath, configContent, "utf-8");
console.log(`✅ tailwindConfig.ts file created at: ${outputFilePath}`);
} catch (error) {
throw new Error(`Failed to write Tailwind config file: ${error instanceof Error ? error.message : String(error)}`);
}

Comment on lines +1 to +8
import * as fs from 'fs';
import * as path from 'path';
import { hexToHsl } from './utills.js';

// Paths for input and output files
const inputFilePath = path.join(__dirname, '../tokens/token_Palette_Primitive.json');
const outputFilePath = path.join(__dirname, '../styles/base/_palette.scss');

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for input file existence.

The script should verify the existence of the input JSON file before attempting to read it, similar to the error handling in the other build scripts.

 const inputFilePath = path.join(__dirname, '../tokens/token_Palette_Primitive.json');
 const outputFilePath = path.join(__dirname, '../styles/base/_palette.scss');
+
+// Ensure the input file exists
+if (!fs.existsSync(inputFilePath)) {
+  console.error(`Token file not found: ${inputFilePath}`);
+  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.

Suggested change
import * as fs from 'fs';
import * as path from 'path';
import { hexToHsl } from './utills.js';
// Paths for input and output files
const inputFilePath = path.join(__dirname, '../tokens/token_Palette_Primitive.json');
const outputFilePath = path.join(__dirname, '../styles/base/_palette.scss');
import * as fs from 'fs';
import * as path from 'path';
import { hexToHsl } from './utills.js';
// Paths for input and output files
const inputFilePath = path.join(__dirname, '../tokens/token_Palette_Primitive.json');
const outputFilePath = path.join(__dirname, '../styles/base/_palette.scss');
// Ensure the input file exists
if (!fs.existsSync(inputFilePath)) {
console.error(`Token file not found: ${inputFilePath}`);
process.exit(1);
}

Comment on lines +14 to +30
const generateColorsTsContent = (tokens: Record<string, any>): string => {
const colorTokens = tokens.color;
const lines: string[] = [];

// Add the TypeScript export header
lines.push(`export const colors = {`);

// Process each token and use the key for the variable name
for (const key in colorTokens) {
lines.push(` "${key}": "${resolveTokenReference(key)}",`);
}

// Close the object
lines.push(`};`);

return lines.join("\n");
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation and error handling for color tokens.

The function should validate the token structure and handle missing color tokens.

+interface ColorToken {
+  $value: string;
+}
+
+interface TokenStructure {
+  color?: Record<string, ColorToken>;
+}
+
-const generateColorsTsContent = (tokens: Record<string, any>): string => {
+const generateColorsTsContent = (tokens: TokenStructure): string => {
-  const colorTokens = tokens.color;
+  const colorTokens = tokens.color || {};
+
+  if (Object.keys(colorTokens).length === 0) {
+    console.warn('No color tokens found in the input file');
+  }
+
   const lines: string[] = [];
 
   lines.push(`export const colors = {`);
 
   for (const key in colorTokens) {
-    lines.push(`  "${key}": "${resolveTokenReference(key)}",`);
+    try {
+      lines.push(`  "${key}": "${resolveTokenReference(key)}",`);
+    } catch (error) {
+      console.warn(`Skipping invalid token key: ${key}`);
+    }
   }
📝 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.

Suggested change
const generateColorsTsContent = (tokens: Record<string, any>): string => {
const colorTokens = tokens.color;
const lines: string[] = [];
// Add the TypeScript export header
lines.push(`export const colors = {`);
// Process each token and use the key for the variable name
for (const key in colorTokens) {
lines.push(` "${key}": "${resolveTokenReference(key)}",`);
}
// Close the object
lines.push(`};`);
return lines.join("\n");
};
interface ColorToken {
$value: string;
}
interface TokenStructure {
color?: Record<string, ColorToken>;
}
const generateColorsTsContent = (tokens: TokenStructure): string => {
const colorTokens = tokens.color || {};
if (Object.keys(colorTokens).length === 0) {
console.warn('No color tokens found in the input file');
}
const lines: string[] = [];
// Add the TypeScript export header
lines.push(`export const colors = {`);
// Process each token and use the key for the variable name
for (const key in colorTokens) {
try {
lines.push(` "${key}": "${resolveTokenReference(key)}",`);
} catch (error) {
console.warn(`Skipping invalid token key: ${key}`);
}
}
// Close the object
lines.push(`};`);
return lines.join("\n");
};

Comment on lines +12 to +42
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");
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation and improve dependency handling.

The function needs token structure validation and should handle missing dependencies.

+interface ColorToken {
+  $value: string;
+}
+
+interface TokenStructure {
+  color?: Record<string, ColorToken>;
+}
+
 const generateScssContent = (
   theme: "dark" | "light",
-  tokens: Record<string, any>,
+  tokens: TokenStructure,
 ): string => {
+  if (!tokens.color || typeof tokens.color !== 'object') {
+    throw new Error('Invalid token structure: missing color tokens');
+  }
+
   const lines: string[] = [];
+  const paletteFile = path.join(stylesDir, "_palette.scss");
+
+  // Verify palette dependency exists
+  if (!fs.existsSync(paletteFile)) {
+    throw new Error('Required dependency _palette.scss not found');
+  }
 
   const selector = theme === "dark" ? ":root" : ".light";
 
   lines.push(`@use "_palette.scss" as *;\n`);

Committable suggestion skipped: line range outside the PR's diff.

"800": "var(--gray-800)",
"900": "var(--gray-900)",
"950": "var(--gray-950)",
"ali": "var(--gray-ali)",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove non-standard color key "ali" from gray palette.

The "ali" key breaks the numeric pattern (50-950) used consistently across all other color shades. Consider removing it or moving it to a separate semantic color mapping.

-    "ali": "var(--gray-ali)",

Comment on lines +21 to +24
- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
bun-version: latest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Pin Bun version for reproducible builds.

Using latest version could lead to unexpected breaking changes. Consider pinning to a specific version.

   - name: Setup Bun
     uses: oven-sh/setup-bun@v1
     with:
-      bun-version: latest
+      bun-version: 1.0.25  # or your preferred stable version
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
bun-version: latest
- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
bun-version: 1.0.25 # or your preferred stable version

Comment on lines +35 to +37
--warning: var(--amber-500);
--warning-border: var(--amber-200);
--warning-foreground: var(--General-white);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Warning colors fail WCAG accessibility requirements

The contrast ratio between amber-500 and white is 2.15:1, which fails WCAG AA requirements for both large text (minimum 3:1) and normal text (minimum 4.5:1). Consider using a darker shade like amber-600 or amber-700 to improve text readability.

🔗 Analysis chain

Verify warning colors for accessibility.

The warning colors (amber-500 on white) might not meet WCAG contrast requirements for text accessibility. Consider using a darker warning color or adjusting the foreground color.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check color contrast ratio using WCAG guidelines

# Install and use color-contrast-checker
npm install -g color-contrast-checker
echo "Checking contrast ratio for warning colors..."
echo "amber-500 (#f59e0b) on white (#ffffff)"
color-contrast-checker "#f59e0b" "#ffffff"

Length of output: 2067


Script:

#!/bin/bash
python3 - << 'EOF'
def hex_to_rgb(hex_color):
    hex_color = hex_color.lstrip('#')
    return tuple(int(hex_color[i:i+2], 16)/255 for i in (0, 2, 4))

def get_luminance(r, g, b):
    r = r/12.92 if r <= 0.03928 else ((r + 0.055)/1.055) ** 2.4
    g = g/12.92 if g <= 0.03928 else ((g + 0.055)/1.055) ** 2.4
    b = b/12.92 if b <= 0.03928 else ((b + 0.055)/1.055) ** 2.4
    return 0.2126 * r + 0.7152 * g + 0.0722 * b

def contrast_ratio(color1, color2):
    l1 = get_luminance(*hex_to_rgb(color1))
    l2 = get_luminance(*hex_to_rgb(color2))
    lighter = max(l1, l2)
    darker = min(l1, l2)
    return (lighter + 0.05) / (darker + 0.05)

# Check amber-500 on white
amber_500 = '#f59e0b'
white = '#ffffff'
ratio = contrast_ratio(amber_500, white)
print(f"Contrast ratio between {amber_500} and {white}: {ratio:.2f}:1")
print(f"Meets WCAG AA for large text (min 3:1): {ratio >= 3}")
print(f"Meets WCAG AA for normal text (min 4.5:1): {ratio >= 4.5}")
EOF

Length of output: 157

@mrbadri mrbadri deleted the test branch January 31, 2025 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants