Skip to content

fix: prevent duplicate variable in tailwind #1671

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.

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

Merged
merged 5 commits into from
Mar 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 1 addition & 9 deletions apps/studio/electron/main/assets/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,6 @@ export async function initializeTailwindColorContent(
return { configPath, cssPath, configContent, cssContent };
}

export function toCamelCase(str: string): string {
return str
.toLowerCase()
.replace(/(?:^\w|[A-Z]|\b\w|\s+)/g, (letter, index) =>
index === 0 ? letter.toLowerCase() : letter.trim() ? letter.toUpperCase() : '',
);
}

export function addTailwindRootColor(
colorObj: ObjectExpression,
newName: string,
Expand All @@ -121,7 +113,7 @@ export function addTailwindRootColor(
type: 'ObjectProperty',
key: {
type: 'Identifier',
name: toCamelCase(newName),
name: newName,
},
value: {
type: 'ObjectExpression',
Expand Down
41 changes: 24 additions & 17 deletions apps/studio/electron/main/assets/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { Theme } from '@onlook/models/assets';
import type { CodeDiffRequest } from '@onlook/models/code';
import { parseHslValue } from '@onlook/utility';
import fs from 'fs';
import { camelCase } from 'lodash';
import path from 'path';
import type { Root, Rule } from 'postcss';
import postcss from 'postcss';
Expand All @@ -27,7 +28,6 @@ import {
initializeTailwindColorContent,
isColorsObjectProperty,
isObjectExpression,
toCamelCase,
} from './helpers';

export async function updateTailwindColorConfig(
Expand All @@ -53,9 +53,11 @@ export async function updateTailwindColorConfig(
await updateDefaultTailwindColor(colorUpdate, colorFamily, colorIndex, newColor, theme);
return { success: true };
}

const camelCaseName = camelCase(newName);
return originalKey
? updateTailwindColorVariable(colorUpdate, originalKey, newColor, newName, theme)
: createTailwindColorVariable(colorUpdate, newColor, newName, parentName);
? updateTailwindColorVariable(colorUpdate, originalKey, newColor, camelCaseName, theme)
: createTailwindColorVariable(colorUpdate, newColor, camelCaseName, parentName);
} catch (error) {
console.error('Error updating Tailwind config:', error);
return {
Expand Down Expand Up @@ -102,7 +104,7 @@ function addTailwindNestedColor(
type: 'ObjectProperty',
key: {
type: 'Identifier',
name: toCamelCase(newName),
name: newName,
},
value: {
type: 'StringLiteral',
Expand All @@ -118,7 +120,7 @@ function addTailwindNestedColor(
type: 'ObjectProperty',
key: {
type: 'Identifier',
name: toCamelCase(newName),
name: newName,
},
value: {
type: 'StringLiteral',
Expand All @@ -137,13 +139,18 @@ async function createTailwindColorVariable(
newName: string,
parentName?: string,
): Promise<UpdateResult> {
const camelCaseName = toCamelCase(newName);
const newCssVarName = parentName?.length ? `${parentName}-${newName}` : newName;

const newCssVarName = parentName?.length ? `${parentName}-${camelCaseName}` : camelCaseName;
// Check if CSS variable already exists
const cssVariables = extractTailwindCssVariables(cssContent);

// Update CSS file
const updatedCssContent = await addTailwindCssVariable(cssContent, newCssVarName, newColor);
fs.writeFileSync(cssPath, updatedCssContent);
if (cssVariables.root[newCssVarName] || cssVariables.dark[newCssVarName]) {
return { success: false, error: `CSS variable --${newCssVarName} already exists` };
} else {
// Variable doesn't exist, add it
const updatedCssContent = await addTailwindCssVariable(cssContent, newCssVarName, newColor);
fs.writeFileSync(cssPath, updatedCssContent);
}

// Update config file
const updateAst = parse(configContent, {
Expand All @@ -160,9 +167,9 @@ async function createTailwindColorVariable(
}

if (!parentName) {
addTailwindRootColor(colorObj, camelCaseName, newCssVarName);
addTailwindRootColor(colorObj, newName, newCssVarName);
} else {
addTailwindNestedColor(colorObj, parentName, camelCaseName, newCssVarName);
addTailwindNestedColor(colorObj, parentName, newName, newCssVarName);
}
}
},
Expand Down Expand Up @@ -206,7 +213,7 @@ function updateTailwindConfigFile(
// If the keyName is not provided, we are renaming the root color
if (!keyName) {
if (parentKey && newName !== parentKey) {
colorProp.key.name = toCamelCase(newName);
colorProp.key.name = newName;
keyUpdated = true;

// Then we need to update the child css variables
Expand All @@ -224,8 +231,8 @@ function updateTailwindConfigFile(
: `${parentKey}-${nestedProp.key.name}`;
const newVarName =
nestedProp.key.name === 'DEFAULT'
? toCamelCase(newName)
: `${toCamelCase(newName)}-${nestedProp.key.name}`;
? newName
: `${newName}-${nestedProp.key.name}`;

nestedProp.value.value = nestedProp.value.value.replace(
new RegExp(`--${oldVarName}`, 'g'),
Expand All @@ -247,7 +254,7 @@ function updateTailwindConfigFile(
nestedProp.key.name === keyName
) {
if (newName !== keyName) {
nestedProp.key.name = toCamelCase(newName);
nestedProp.key.name = newName;
keyUpdated = true;
}

Expand Down Expand Up @@ -661,7 +668,7 @@ async function deleteColorGroup(
groupName: string,
colorName?: string,
): Promise<UpdateResult> {
const camelCaseName = toCamelCase(groupName);
const camelCaseName = camelCase(groupName);

// Update config file
const updateAst = parse(configContent, {
Expand Down
2 changes: 1 addition & 1 deletion apps/studio/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"i18next": "^24.2.2",
"istextorbinary": "^9.5.0",
"js-string-escape": "^1.0.1",
"lodash": "^4.17.21",
"lodash-es": "^4.17.21",
"mime-lite": "^1.0.3",
"mixpanel": "^0.18.0",
"monaco-editor": "^0.52.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Color, isColorEmpty } from '@onlook/utility';
import { observer } from 'mobx-react-lite';
import { memo, useCallback, useEffect, useMemo, useState } from 'react';
// import { BrandPopoverPicker } from './ColorBrandPicker';
import { PopoverPicker } from './Popover';
import { BrandPopoverPicker } from './ColorBrandPicker';
Copy link
Contributor

Choose a reason for hiding this comment

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

Replaced the import of PopoverPicker with BrandPopoverPicker. Remove any outdated commented code to keep the file clean.


const stripUrlWrapper = (url: string) => {
return url.replace(/^url\((['"]?)(.*)\1\)/, '$2');
Expand Down Expand Up @@ -200,21 +200,21 @@ const ColorInput = observer(
};
return (
<div className="w-32 p-[6px] gap-2 flex flex-row rounded cursor-pointer bg-background-onlook/75">
{/* <BrandPopoverPicker
<BrandPopoverPicker
color={color}
onChange={sendStyleUpdate}
onChangeEnd={sendStyleUpdate}
backgroundImage={backgroundImage}
compoundStyle={compoundStyle}
/> */}
/>

<PopoverPicker
{/* <PopoverPicker
color={color}
onChange={sendStyleUpdate}
onChangeEnd={sendStyleUpdate}
backgroundImage={backgroundImage}
compoundStyle={compoundStyle}
/>
/> */}
<ColorTextInput
value={value}
isFocused={isFocused}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
DropdownMenuTrigger,
} from '@onlook/ui/dropdown-menu';
import { Icons } from '@onlook/ui/icons';
import { Color } from '@onlook/utility';
import { Color, toNormalCase } from '@onlook/utility';
import { useState } from 'react';
import { ColorPopover } from './ColorPopover';
import { MainChannels } from '@onlook/models/constants';
Expand Down Expand Up @@ -105,6 +105,7 @@ export const BrandPalletGroup = ({
const handleRenameClick = () => {
setNewGroupName(title);
setIsRenaming(true);
setLocalError(null);
};

const validateName = (value: string) => {
Expand Down Expand Up @@ -279,7 +280,9 @@ export const BrandPalletGroup = ({
<TooltipContent side="top">
<div className="flex flex-col">
<span className="text-sm">
{color.name}
{toNormalCase(
color.name,
)}
</span>
<span className="text-xs text-muted-foreground">
{getColorValue(color)}
Expand All @@ -305,7 +308,7 @@ export const BrandPalletGroup = ({
/>
<div className="flex flex-col">
<span className="text-sm text-foreground">
{color.name}
{toNormalCase(color.name)}
</span>
<span className="text-xs text-muted-foreground">
{getColorValue(color)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Popover, PopoverContent, PopoverTrigger } from '@onlook/ui/popover';
import type { Color } from '@onlook/utility';
import { useState } from 'react';
import ColorPickerContent from '../../../EditPanel/StylesTab/single/ColorInput/ColorPicker';
import { Tooltip, TooltipContent, TooltipTrigger } from '@onlook/ui/tooltip';
import { toNormalCase, type Color } from '@onlook/utility';
import { camelCase } from 'lodash';
Copy link
Contributor

Choose a reason for hiding this comment

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

Using lodash instead so we don't have repeat code

import { useEffect, useState } from 'react';
import ColorPickerContent from '../../../EditPanel/StylesTab/single/ColorInput/ColorPicker';

export const ColorPopover = ({
color,
Expand Down Expand Up @@ -31,22 +32,27 @@ export const ColorPopover = ({
onColorChange(newColor, editedName);
}
};

const handleSave = () => {
if (existedName?.includes(editedName) && editedName !== brandColor) {
const camelCaseName = camelCase(editedName);

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

if (onColorChangeEnd) {
onColorChangeEnd(editedColor, editedName);
onColorChangeEnd(editedColor, camelCaseName);
}

if (onClose) {
onClose();
}
};

useEffect(() => {
setEditedName(toNormalCase(brandColor));
}, [brandColor]);

return (
<Popover onOpenChange={(open) => !open && handleSave()} open={true}>
<PopoverTrigger asChild>
Expand All @@ -72,6 +78,11 @@ export const ColorPopover = ({
error ? 'border-red-500' : 'border-white/10'
} bg-background-secondary px-2 py-1 text-sm`}
disabled={isDefaultPalette || editedName === 'DEFAULT'}
onKeyDown={(e) => {
if (e.key === 'Enter') {
handleSave();
}
}}
/>
</TooltipTrigger>
{error && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ const ColorPanel = observer(({ onClose }: ColorPanelProps) => {
newColor: Color,
newName: string,
parentName?: string,
theme?: Theme,
) => {
themeManager.update(groupName, index, newColor, newName, parentName, theme, false);
};
Expand All @@ -50,17 +49,11 @@ const ColorPanel = observer(({ onClose }: ColorPanelProps) => {
newColor: Color,
newName: string,
parentName?: string,
theme?: Theme,
) => {
themeManager.update(groupName, index, newColor, newName, parentName, theme, true);
};

const handleDuplicate = (
groupName: string,
colorName: string,
isDefaultPalette?: boolean,
theme?: Theme,
) => {
const handleDuplicate = (groupName: string, colorName: string, isDefaultPalette?: boolean) => {
themeManager.duplicate(groupName, colorName, isDefaultPalette, theme);
};

Expand All @@ -69,12 +62,7 @@ const ColorPanel = observer(({ onClose }: ColorPanelProps) => {
setIsAddingNewGroup(false);
};

const handleDefaultColorChange = (
groupName: string,
colorIndex: number,
newColor: Color,
theme?: Theme,
) => {
const handleDefaultColorChange = (groupName: string, colorIndex: number, newColor: Color) => {
themeManager.handleDefaultColorChange(groupName, colorIndex, newColor, theme);
};

Expand Down
2 changes: 1 addition & 1 deletion apps/studio/src/routes/editor/LayersPanel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export const LayersPanel = observer(() => {

<button
className={cn(
'w-16 h-16 rounded-xl flex flex-col items-center justify-center gap-1.5 p-2 hidden',
'w-16 h-16 rounded-xl flex flex-col items-center justify-center gap-1.5 p-2',
selectedTab === TabValue.BRAND && isLocked
? 'bg-accent text-foreground border-[0.5px] border-foreground/20'
: 'text-muted-foreground hover:text-foreground hover:bg-accent/50',
Expand Down
Loading