Skip to content

Update "PixelGenius Platform" from zeroheight #207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alikashef wants to merge 4 commits into from
Closed

Update "PixelGenius Platform" from zeroheight #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c3aabaf
Update PixelGenius Platform from zeroheight
Jan 27, 2025
1fb0cec
Update PixelGenius Platform from zeroheight
Jan 27, 2025
5bdf83b
Update PixelGenius Platform from zeroheight
Jan 27, 2025
d3d617e
chore: update design system tokens
alikashef Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/design-system/src/styles/base/_colors-dark.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
:root {
--accent: var(--zinc-800);
--accent-foreground: var(--general-white);
--aliblack: var(--general-black);
--background: var(--zinc-950);
--black: var(--general-black);
--border: var(--zinc-800);
--card: var(--zinc-900);
--card-foreground: var(--general-white);
Expand Down
2 changes: 1 addition & 1 deletion packages/design-system/src/styles/base/_colors-light.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
.light {
--accent: var(--zinc-100);
--accent-foreground: var(--zinc-900);
--aliblack: var(--general-black);
--background: var(--general-white);
--black: var(--general-black);
--border: var(--zinc-200);
--card: var(--zinc-100);
--card-foreground: var(--slate-900);
Expand Down
2 changes: 1 addition & 1 deletion packages/design-system/src/styles/base/_palette.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@
--yellow-900: 28 73% 26%;
--yellow-950: 26 83% 14%;

--zinc-50: 0 0% 98%;
--zinc-100: 240 5% 96%;
--zinc-200: 240 6% 90%;
--zinc-300: 240 5% 84%;
Expand All @@ -265,6 +264,7 @@
--zinc-800: 240 4% 16%;
--zinc-900: 240 6% 10%;
--zinc-950: 240 10% 4%;
--zinc-ali2: 0 0% 98%;
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 27, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Maintain consistent numeric scale for zinc color variants.

The new variable --zinc-ali2 breaks the numeric scale pattern (50, 100, 200, ...) used across all color palettes. Consider keeping the numeric scale for consistency:

  1. If this is a new shade, place it appropriately in the numeric scale
  2. If this replaces zinc-50, keep the original naming
-  --zinc-ali2: 0 0% 98%;
+  --zinc-50: 0 0% 98%;
📝 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
--zinc-ali2: 0 0% 98%;
--zinc-50: 0 0% 98%;


}
}
2 changes: 1 addition & 1 deletion packages/design-system/src/tailwind/colors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export const colors = {
"accent": "hsl(var(--accent))",
"accent-foreground": "hsl(var(--accent-foreground))",
"aliblack": "hsl(var(--aliblack))",
"background": "hsl(var(--background))",
"black": "hsl(var(--black))",
"border": "hsl(var(--border))",
"card": "hsl(var(--card))",
"card-foreground": "hsl(var(--card-foreground))",
Expand Down
2 changes: 1 addition & 1 deletion packages/design-system/src/tailwind/palette.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ export const palette = {
"950": "var(--yellow-950)",
},
"zinc": {
"50": "var(--zinc-50)",
"100": "var(--zinc-100)",
"200": "var(--zinc-200)",
"300": "var(--zinc-300)",
Expand All @@ -287,5 +286,6 @@ export const palette = {
"800": "var(--zinc-800)",
"900": "var(--zinc-900)",
"950": "var(--zinc-950)",
"ali2": "var(--zinc-ali2)",
},
};
8 changes: 4 additions & 4 deletions packages/design-system/src/tokens/token_color_default.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
"$value": "{general.white}",
"$description": "Used for accents such as hover effects on <DropdownMenuItem>, <SelectItem>...etc"
},
"aliblack": {
"$type": "color",
"$value": "{general.black}"
},
"background": {
"$type": "color",
"$value": "{zinc.950}",
"$description": "Default background color of <body />...etc"
},
"black": {
"$type": "color",
"$value": "{general.black}"
},
"border": {
"$type": "color",
"$value": "{zinc.800}",
Expand Down
8 changes: 4 additions & 4 deletions packages/design-system/src/tokens/token_color_light.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
"$value": "{zinc.900}",
"$description": "Used for accents such as hover effects on <DropdownMenuItem>, <SelectItem>...etc"
},
"aliblack": {
"$type": "color",
"$value": "{general.black}"
},
Comment on lines +13 to +16
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 27, 2025

Choose a reason for hiding this comment

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

💡 Codebase verification

The aliblack token name deviates from the design system's naming convention

The color token naming in the design system follows a clear semantic pattern (e.g., primary, secondary, background, error). The prefix "ali" in aliblack breaks this convention and doesn't convey its purpose. Consider renaming it to better reflect its intended use, following the established semantic naming pattern.

Examples of existing patterns:

  • Semantic: primary, secondary, accent
  • UI elements: card, input, popover
  • Status: error, warning, success
  • Variants: primary-light, primary-foreground
🔗 Analysis chain

Verify the naming convention for the new color token.

The prefix ali in aliblack appears to deviate from the established naming convention in the design system. Consider using a more descriptive and consistent name that reflects its purpose.

Run this script to check the naming patterns across the design system:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other color tokens with similar prefixes
rg -g '*.{json,scss,ts}' '"ali[a-z]+'

Length of output: 318

Script:

#!/bin/bash
# Check color token names in token files
echo "=== Color token names in token files ==="
rg -g '**/token_color_*.json' '"\$type": "color"' -B1

echo -e "\n=== Check for documentation ==="
fd "README|CONTRIBUTING" --extension md --exec cat {}

Length of output: 19340

"background": {
"$type": "color",
"$value": "{general.white}",
"$description": "Default background color of <body />...etc"
},
"black": {
"$type": "color",
"$value": "{general.black}"
},
"border": {
"$type": "color",
"$value": "{zinc.200}",
Expand Down
2 changes: 1 addition & 1 deletion packages/design-system/src/tokens/token_palette_primitive.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,7 @@
"$type": "color",
"$value": "#09090b"
},
"50": {
"ali2": {
"$type": "color",
"$value": "#fafafa"
}
Comment on lines +1015 to 1018
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 27, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Maintain consistent numeric scale in primitive color tokens.

The addition of ali2 in the zinc palette breaks the established numeric scale pattern used across all color palettes in the primitive tokens. This inconsistency could lead to confusion and maintenance issues.

-    "ali2": {
+    "50": {
       "$type": "color",
       "$value": "#fafafa"
     }
📝 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
"ali2": {
"$type": "color",
"$value": "#fafafa"
}
"50": {
"$type": "color",
"$value": "#fafafa"
}

Expand Down
Loading