Skip to content

Commit ebeee4f

Browse files
spartan-vutrannguyenspartan-vutrannguyen
authored
fix: error when add new color to group (onlook-dev#1683)
* fix: error when add new color to group * update variable in css file * fix display in color picker
1 parent 2ba62ff commit ebeee4f

File tree

6 files changed

+127
-35
lines changed

6 files changed

+127
-35
lines changed

‎apps/studio/electron/main/assets/styles.ts

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ export async function updateTailwindColorConfig(
5656
}
5757

5858
const camelCaseName = newName === DEFAULT_COLOR_NAME ? newName : camelCase(newName);
59+
5960
return originalKey
6061
? updateTailwindColorVariable(colorUpdate, originalKey, newColor, camelCaseName, theme)
6162
: createTailwindColorVariable(colorUpdate, newColor, camelCaseName, parentName);
@@ -217,7 +218,7 @@ function updateTailwindConfigFile(
217218
colorProp.key.name = newName;
218219
keyUpdated = true;
219220

220-
// Then we need to update the child css variables
221+
// Then we need to update the child css variables or direct color values
221222
if (colorProp.value.type === 'ObjectExpression') {
222223
colorProp.value.properties.forEach((nestedProp) => {
223224
if (
@@ -241,6 +242,11 @@ function updateTailwindConfigFile(
241242
);
242243
}
243244
});
245+
} else if (colorProp.value.type === 'StringLiteral') {
246+
colorProp.value.value = colorProp.value.value.replace(
247+
new RegExp(`--${parentKey}`, 'g'),
248+
`--${newName}`,
249+
);
244250
}
245251
}
246252
} else {
@@ -399,20 +405,37 @@ async function updateTailwindCssVariable(
399405
Once(root: Root) {
400406
let rootValue: string | undefined;
401407
let darkValue: string | undefined;
408+
let hasRootVar = false;
409+
let hasDarkVar = false;
402410

403411
root.walkRules(':root', (rule) => {
404412
rule.walkDecls(`--${originalName}`, (decl) => {
405413
rootValue = decl.value;
414+
hasRootVar = true;
406415
});
407416
});
408417

409418
root.walkRules('.dark', (rule) => {
410419
rule.walkDecls(`--${originalName}`, (decl) => {
411420
darkValue = decl.value;
421+
hasDarkVar = true;
412422
});
413423
});
414424

415-
// Process both :root and .dark rules
425+
// Create new variables if they don't exist and we have both newVarName and newColor
426+
if (newVarName && newColor) {
427+
if (!hasRootVar) {
428+
root.walkRules(':root', (rule) => {
429+
rule.append({ prop: `--${newVarName}`, value: newColor });
430+
});
431+
}
432+
if (!hasDarkVar) {
433+
root.walkRules('.dark', (rule) => {
434+
rule.append({ prop: `--${newVarName}`, value: newColor });
435+
});
436+
}
437+
}
438+
416439
root.walkRules(/^(:root|\.dark)$/, (rule) => {
417440
const isDarkTheme = rule.selector === '.dark';
418441
const shouldUpdateValue =
@@ -464,6 +487,16 @@ async function updateTailwindCssVariable(
464487
}
465488
}
466489

490+
// Handle variable usages in other declarations
491+
if (decl.value.includes(`var(--${originalName})`)) {
492+
if (newVarName && newVarName !== originalName) {
493+
decl.value = decl.value.replace(
494+
new RegExp(`var\\(--${originalName}\\)`, 'g'),
495+
`var(--${newVarName})`,
496+
);
497+
}
498+
}
499+
467500
// Handle nested variables rename if existed
468501
if (
469502
newVarName &&
@@ -479,6 +512,37 @@ async function updateTailwindCssVariable(
479512
}
480513
});
481514
});
515+
516+
// update Tailwind classes that use the variable
517+
root.walkAtRules('layer', (layerRule) => {
518+
layerRule.walkRules((rule) => {
519+
rule.nodes?.forEach((node) => {
520+
// Check if this is an @apply at-rule
521+
if (node.type === 'atrule' && 'name' in node && node.name === 'apply') {
522+
const value = 'params' in node ? node.params : '';
523+
524+
const utilityPattern = new RegExp(
525+
`[a-z-]+-${originalName}\\b`,
526+
'g',
527+
);
528+
const hasMatch = utilityPattern.test(value);
529+
530+
if (hasMatch) {
531+
const newValue = value.replace(utilityPattern, (match) => {
532+
const replaced = match.replace(
533+
originalName,
534+
newVarName || originalName,
535+
);
536+
return replaced;
537+
});
538+
if ('params' in node) {
539+
node.params = newValue;
540+
}
541+
}
542+
}
543+
});
544+
});
545+
});
482546
},
483547
},
484548
]);
@@ -636,7 +700,7 @@ async function updateClassReferences(
636700
const oldClassPattern = new RegExp(`(^|-)${oldClass}(-|$)`);
637701
if (oldClassPattern.test(currentClass)) {
638702
hasChanges = true;
639-
return currentClass.replace(oldClass, newClass);
703+
return newClass ? currentClass.replace(oldClass, newClass) : '';
640704
}
641705
}
642706
return currentClass;
@@ -669,6 +733,7 @@ async function updateClassReferences(
669733
async function deleteColorGroup(
670734
{ configPath, cssPath, configContent, cssContent }: ColorUpdate,
671735
groupName: string,
736+
projectRoot: string,
672737
colorName?: string,
673738
): Promise<UpdateResult> {
674739
const camelCaseName = camelCase(groupName);
@@ -757,6 +822,14 @@ async function deleteColorGroup(
757822
const output = generate(updateAst, { retainLines: true, compact: false }, configContent);
758823
fs.writeFileSync(configPath, output.code);
759824

825+
// Also delete the color group in the class references
826+
const replacements: ClassReplacement[] = [];
827+
replacements.push({
828+
oldClass: camelCaseName,
829+
newClass: '',
830+
});
831+
await updateClassReferences(projectRoot, replacements);
832+
760833
return { success: true };
761834
}
762835

@@ -771,7 +844,7 @@ export async function deleteTailwindColorGroup(
771844
return { success: false, error: 'Failed to prepare color update' };
772845
}
773846

774-
return deleteColorGroup(colorUpdate, groupName, colorName);
847+
return deleteColorGroup(colorUpdate, groupName, projectRoot, colorName);
775848
} catch (error) {
776849
return {
777850
success: false,

‎apps/studio/src/lib/editor/engine/theme/index.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { Color } from '@onlook/utility';
88
import { makeAutoObservable } from 'mobx';
99
import colors from 'tailwindcss/colors';
1010
import type { EditorEngine } from '..';
11+
import { camelCase } from 'lodash';
1112

1213
interface ColorValue {
1314
value: string;
@@ -297,8 +298,8 @@ export class ThemeManager {
297298
try {
298299
await invokeMainChannel(MainChannels.UPDATE_TAILWIND_CONFIG, {
299300
projectRoot,
300-
originalKey: oldName.toLowerCase(),
301-
newName: newName.toLowerCase(),
301+
originalKey: oldName,
302+
newName: newName,
302303
});
303304

304305
// Refresh colors after rename
@@ -317,7 +318,7 @@ export class ThemeManager {
317318
try {
318319
await invokeMainChannel(MainChannels.DELETE_TAILWIND_CONFIG, {
319320
projectRoot,
320-
groupName: groupName.toLowerCase(),
321+
groupName: groupName,
321322
colorName,
322323
});
323324

@@ -344,7 +345,10 @@ export class ThemeManager {
344345

345346
try {
346347
// For new colors, pass empty originalKey and parentName
347-
const originalKey = this.brandColors[groupName]?.[index]?.originalKey || '';
348+
const originalGroupName = camelCase(groupName);
349+
const originalParentName = camelCase(parentName);
350+
351+
const originalKey = this.brandColors[originalGroupName]?.[index]?.originalKey || '';
348352

349353
// If is selected element, update the color in real-time
350354
// Base on the class name, find the styles to update
@@ -356,7 +360,7 @@ export class ThemeManager {
356360
originalKey,
357361
newColor: newColor.toHex(),
358362
newName,
359-
parentName,
363+
parentName: originalParentName,
360364
theme,
361365
});
362366

@@ -455,7 +459,15 @@ export class ThemeManager {
455459
: colorToDuplicate.lightColor,
456460
);
457461

458-
await this.update(groupName, group.length, color, newName, groupName.toLowerCase());
462+
await this.update(
463+
groupName,
464+
group.length,
465+
color,
466+
newName,
467+
groupName.toLowerCase(),
468+
theme,
469+
true,
470+
);
459471

460472
this.scanConfig();
461473
}

‎apps/studio/src/routes/editor/EditPanel/StylesTab/single/ColorInput/ColorBrandPicker.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Icons } from '@onlook/ui/icons/index';
55
import { Input } from '@onlook/ui/input';
66
import { Popover, PopoverContent, PopoverTrigger } from '@onlook/ui/popover';
77
import { Tabs, TabsContent, TabsList, TabsTrigger } from '@onlook/ui/tabs';
8-
import { Color } from '@onlook/utility';
8+
import { Color, toNormalCase } from '@onlook/utility';
99
import { memo, useEffect, useRef, useState } from 'react';
1010
import { isBackgroundImageEmpty } from '.';
1111
import ColorButton from './ColorButton';
@@ -53,7 +53,7 @@ const ColorGroup = ({
5353
onClick={() => setExpanded(!expanded)}
5454
>
5555
<div className="flex items-center gap-1 flex-1">
56-
<span className="text-xs font-normal capitalize">{name}</span>
56+
<span className="text-xs font-normal capitalize">{toNormalCase(name)}</span>
5757
{isDefault && (
5858
<span className="ml-2 text-xs text-muted-foreground">Default</span>
5959
)}

‎apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/ColorPalletGroup.tsx

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { Tooltip, TooltipContent, TooltipPortal, TooltipTrigger } from '@onlook/
1414
import { Color, toNormalCase } from '@onlook/utility';
1515
import { useState } from 'react';
1616
import { ColorPopover } from './ColorPopover';
17+
import { camelCase } from 'lodash';
1718

1819
export interface ColorItem {
1920
name: string;
@@ -81,7 +82,7 @@ export const BrandPalletGroup = ({
8182
parentName?: string,
8283
) => {
8384
if (onColorChange) {
84-
onColorChange(title.toLowerCase(), index, newColor, newName, parentName);
85+
onColorChange(title, index, newColor, newName, parentName);
8586
}
8687
};
8788

@@ -92,7 +93,7 @@ export const BrandPalletGroup = ({
9293
parentName?: string,
9394
) => {
9495
if (onColorChangeEnd) {
95-
onColorChangeEnd(title.toLowerCase(), index, newColor, newName, parentName);
96+
onColorChangeEnd(title, index, newColor, newName, parentName);
9697
}
9798
setEditingColorIndex(null);
9899
setIsAddingNewColor(false);
@@ -103,12 +104,17 @@ export const BrandPalletGroup = ({
103104
};
104105

105106
const handleRenameClick = () => {
106-
setNewGroupName(title);
107+
setNewGroupName(toNormalCase(title));
107108
setIsRenaming(true);
108109
setLocalError(null);
109110
};
110111

111112
const validateName = (value: string) => {
113+
// Only allow text characters, numbers, and spaces and not start with number
114+
if (!/^[a-zA-Z0-9\s]+$/.test(value) || /^[0-9]/.test(value)) {
115+
return 'Group name can only contain text, numbers, and spaces and not start with number';
116+
}
117+
112118
if (value.trim() === '') {
113119
return 'Group name cannot be empty';
114120
}
@@ -117,7 +123,10 @@ export const BrandPalletGroup = ({
117123
return null;
118124
}
119125

120-
if (Object.keys(themeManager.colorGroups).includes(value.toLowerCase())) {
126+
if (
127+
Object.keys(themeManager.colorGroups).includes(camelCase(value)) &&
128+
camelCase(value) !== title
129+
) {
121130
return 'Group name already exists';
122131
}
123132

@@ -133,7 +142,8 @@ export const BrandPalletGroup = ({
133142

134143
const handleRenameSubmit = () => {
135144
if (!localError && newGroupName.trim() && newGroupName !== title) {
136-
onRename(title.toLowerCase(), newGroupName.trim());
145+
const newName = camelCase(newGroupName);
146+
onRename(title, newName);
137147
}
138148
setIsRenaming(false);
139149
setLocalError(null);
@@ -192,7 +202,7 @@ export const BrandPalletGroup = ({
192202
</Tooltip>
193203
) : (
194204
<span className="text-small text-foreground-secondary font-normal">
195-
{title}
205+
{toNormalCase(title)}
196206
</span>
197207
)}
198208
{!isDefaultPalette && (
@@ -402,20 +412,10 @@ export const BrandPalletGroup = ({
402412
brandColor="New Color"
403413
onClose={() => setIsAddingNewColor(false)}
404414
onColorChange={(newColor, newName) =>
405-
handleColorChange(
406-
colors?.length || 0,
407-
newColor,
408-
newName,
409-
title.toLowerCase(),
410-
)
415+
handleColorChange(colors?.length || 0, newColor, newName, title)
411416
}
412417
onColorChangeEnd={(newColor, newName) =>
413-
handleColorChangeEnd(
414-
colors?.length || 0,
415-
newColor,
416-
newName,
417-
title.toLowerCase(),
418-
)
418+
handleColorChangeEnd(colors?.length || 0, newColor, newName, title)
419419
}
420420
existedName={existedName}
421421
/>

‎apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/ColorPopover.tsx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,21 @@ export const ColorPopover = ({
3535
}
3636
};
3737
const handleSave = () => {
38-
const camelCaseName =
39-
editedName === DEFAULT_COLOR_NAME ? editedName : camelCase(editedName);
38+
let camelCaseName = editedName === DEFAULT_COLOR_NAME ? editedName : camelCase(editedName);
4039

4140
if (existedName?.includes(camelCaseName) && camelCaseName !== brandColor) {
4241
setError('Color name already exists');
4342
return;
4443
}
4544

45+
if (!editedName) {
46+
if (!brandColor) {
47+
setError('Color name is required');
48+
return;
49+
}
50+
camelCaseName = brandColor;
51+
}
52+
4653
if (onColorChangeEnd) {
4754
onColorChangeEnd(editedColor, camelCaseName);
4855
}
@@ -81,7 +88,7 @@ export const ColorPopover = ({
8188
'w-full rounded-md border bg-background-secondary px-2 py-1 text-sm',
8289
error ? 'border-red-500' : 'border-white/10',
8390
)}
84-
disabled={isDefaultPalette || editedName === DEFAULT_COLOR_NAME}
91+
disabled={isDefaultPalette || brandColor === DEFAULT_COLOR_NAME}
8592
onKeyDown={(e) => {
8693
if (e.key === 'Enter') {
8794
handleSave();

‎apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/index.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ const ColorPanel = observer(({ onClose }: ColorPanelProps) => {
117117
<BrandPalletGroup
118118
key={groupName}
119119
theme={theme}
120-
title={groupName.charAt(0).toUpperCase() + groupName.slice(1)}
120+
title={groupName}
121121
colors={colors}
122122
onRename={handleRename}
123123
onDelete={(colorName) => handleDelete(groupName, colorName)}
@@ -168,7 +168,7 @@ const ColorPanel = observer(({ onClose }: ColorPanelProps) => {
168168
<BrandPalletGroup
169169
key={colorName}
170170
theme={theme}
171-
title={colorName.charAt(0).toUpperCase() + colorName.slice(1)}
171+
title={colorName}
172172
colors={colors}
173173
onRename={handleRename}
174174
onDelete={(colorItem) => handleDelete(colorName, colorItem)}

0 commit comments

Comments
 (0)