Skip to content
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

Fix handling of dynamic className in tailwind input #849

Merged
merged 7 commits into from
Dec 6, 2024
Merged

Fix handling of dynamic className in tailwind input #849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
700f90b
Fix handling of dynamic className in tailwind input
cleobnvntra Dec 4, 2024
21df662
Fix handling of dynamic className in tailwind input
cleobnvntra Dec 4, 2024
e5ef2e4
Merge branch 'bugfix/dynamic-css-classnames' of https://github.com/cl…
cleobnvntra Dec 6, 2024
8ed0826
Merge branch 'main' into bugfix/dynamic-css-classnames
cleobnvntra Dec 6, 2024
36efeca
reformat logic for handling template literals
cleobnvntra Dec 6, 2024
8479ab3
Separate error from classNames
Kitenite Dec 6, 2024
c303ffe
Move model for shared
Kitenite Dec 6, 2024
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
56 changes: 46 additions & 10 deletions apps/studio/electron/main/code/classes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,39 +1,75 @@
import * as t from '@babel/types';
import type { ClassParsingResult, TemplateNode } from '@onlook/models/element';
import { readCodeBlock } from '.';
import { parseJsxCodeBlock } from './helpers';
import type { TemplateNode } from '@onlook/models/element';

export async function getTemplateNodeClass(templateNode: TemplateNode): Promise<string[]> {
export async function getTemplateNodeClass(
templateNode: TemplateNode,
): Promise<ClassParsingResult> {
const codeBlock = await readCodeBlock(templateNode);
const ast = parseJsxCodeBlock(codeBlock);

if (!ast) {
return [];
return { type: 'error', reason: 'AST could not be parsed.' };
}

const classes = getNodeClasses(ast);
return classes || [];
return getNodeClasses(ast);
}

function getNodeClasses(node: t.JSXElement): string[] {
function getNodeClasses(node: t.JSXElement): ClassParsingResult {
const openingElement = node.openingElement;
const classNameAttr = openingElement.attributes.find(
(attr): attr is t.JSXAttribute => t.isJSXAttribute(attr) && attr.name.name === 'className',
);

if (!classNameAttr) {
return [];
return {
type: 'error',
reason: 'No className attribute found.',
};
}

if (t.isStringLiteral(classNameAttr.value)) {
return classNameAttr.value.value.split(/\s+/).filter(Boolean);
return {
type: 'classes',
value: classNameAttr.value.value.split(/\s+/).filter(Boolean),
};
}

if (
t.isJSXExpressionContainer(classNameAttr.value) &&
t.isStringLiteral(classNameAttr.value.expression)
) {
return classNameAttr.value.expression.value.split(/\s+/).filter(Boolean);
return {
type: 'classes',
value: classNameAttr.value.expression.value.split(/\s+/).filter(Boolean),
};
}

if (
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also return an error straight from here since we're only handling classes. Everything else passes through.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reformatted the code into:

    if (
        t.isJSXExpressionContainer(classNameAttr.value) &&
        t.isTemplateLiteral(classNameAttr.value.expression)
    ) {
        const templateLiteral = classNameAttr.value.expression;

        // Immediately return error if dynamic classes are detected within the template literal
        if (templateLiteral.expressions.length > 0) {
            return {
                type: 'error',
                reason: 'Dynamic classes detected. Dynamic variables in the className prevent extraction of Tailwind classes.',
            };
        }

        // Extract and return static classes from the template literal if no dynamic classes are used
        const quasis = templateLiteral.quasis.map((quasi) => quasi.value.raw.split(/\s+/));
        return {
            type: 'classes',
            value: quasis.flat().filter(Boolean),
        };
    }

This allows the code to exit the block early if it is an error (dynamic classes are used).
I am unsure if this is exactly what you meant. Can you please clarify further if I misinterpreted it? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

This works great, thanks!

t.isJSXExpressionContainer(classNameAttr.value) &&
t.isTemplateLiteral(classNameAttr.value.expression)
) {
const templateLiteral = classNameAttr.value.expression;

// Immediately return error if dynamic classes are detected within the template literal
if (templateLiteral.expressions.length > 0) {
return {
type: 'error',
reason: 'Dynamic classes detected.',
};
}

// Extract and return static classes from the template literal if no dynamic classes are used
const quasis = templateLiteral.quasis.map((quasi) => quasi.value.raw.split(/\s+/));
return {
type: 'classes',
value: quasis.flat().filter(Boolean),
};
}

return [];
return {
type: 'error',
reason: 'Unsupported className format.',
};
}
90 changes: 79 additions & 11 deletions apps/studio/src/routes/editor/EditPanel/StylesTab/single/TailwindInput/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { StyleMode } from '@/lib/editor/engine/style';
import { invokeMainChannel, sendAnalytics } from '@/lib/utils';
import type { CodeDiffRequest } from '@onlook/models/code';
import { MainChannels } from '@onlook/models/constants';
import type { DomElement } from '@onlook/models/element';
import type { ClassParsingResult, DomElement } from '@onlook/models/element';
import { Icons } from '@onlook/ui/icons';
import { Textarea } from '@onlook/ui/textarea';
import { Tooltip, TooltipContent, TooltipPortal, TooltipTrigger } from '@onlook/ui/tooltip';
Expand All @@ -16,6 +16,7 @@ interface History {
past: string[];
present: string;
future: string[];
error?: string;
}

const TailwindInput = observer(() => {
Expand Down Expand Up @@ -139,31 +140,45 @@ const TailwindInput = observer(() => {
const newInstance = await editorEngine.ast.getTemplateNodeById(domEl.instanceId);

if (newInstance) {
const instanceClasses: string[] = await invokeMainChannel(
const instanceClasses: ClassParsingResult = await invokeMainChannel(
MainChannels.GET_TEMPLATE_NODE_CLASS,
newInstance,
);
const classes = instanceClasses.join(' ');

if (instanceClasses.type === 'error') {
console.warn(instanceClasses.reason);
}

setInstanceHistory({
past: [],
present: classes,
present:
instanceClasses.type === 'classes'
? instanceClasses.value.join(' ')
: instanceClasses.type,
future: [],
error: instanceClasses.type === 'error' ? instanceClasses.reason : undefined,
});
}
}

async function getRootClasses(domEl: DomElement) {
const newRoot = await editorEngine.ast.getTemplateNodeById(domEl.oid);
if (newRoot) {
const rootClasses: string[] = await invokeMainChannel(
const rootClasses: ClassParsingResult = await invokeMainChannel(
MainChannels.GET_TEMPLATE_NODE_CLASS,
newRoot,
);
const classes = rootClasses.join(' ');

if (rootClasses.type === 'error') {
console.warn(rootClasses.reason);
}

setRootHistory({
past: [],
present: classes,
present:
rootClasses.type === 'classes' ? rootClasses.value.join(' ') : rootClasses.type,
future: [],
error: rootClasses.type === 'error' ? rootClasses.reason : undefined,
});
}
}
Expand Down Expand Up @@ -211,6 +226,19 @@ const TailwindInput = observer(() => {
textarea.style.height = `${textarea.scrollHeight + 20}px`;
};

const navigateToTemplateNode = async (oid: string | null) => {
if (!oid) {
console.error('No templateNode ID provided for navigation.');
return;
}

try {
await window.api.invoke(MainChannels.VIEW_SOURCE_CODE, oid);
} catch (error) {
console.error('Error opening TemplateNode in IDE:', error);
}
};

useEffect(() => {
if (instanceRef.current) {
adjustHeight(instanceRef.current);
Expand Down Expand Up @@ -295,7 +323,12 @@ const TailwindInput = observer(() => {
: 'bg-background-secondary/75 focus:bg-background-tertiary',
)}
placeholder="Add tailwind classes here"
value={rootHistory.present}
value={
rootHistory.error
? 'Warning: ' + rootHistory.error + ' Open the code to edit.'
: rootHistory.present
}
readOnly={!!rootHistory.error}
onInput={(e) => handleInput(e, rootHistory, setRootHistory)}
onKeyDown={(e) => handleKeyDown(e, rootHistory, setRootHistory)}
onBlur={(e) => {
Expand Down Expand Up @@ -331,7 +364,21 @@ const TailwindInput = observer(() => {
/>
)}
</div>
{isRootFocused && <EnterIndicator />}
{rootHistory.error ? (
<div className="absolute bottom-1 right-2 text-xs flex items-center text-blue-500 cursor-pointer">
<button
onClick={(e) => {
e.stopPropagation(); // Prevents unfocusing the textarea
navigateToTemplateNode(selectedEl?.oid);
}}
className="underline"
>
Go to source
</button>
</div>
) : (
isRootFocused && <EnterIndicator />
)}
</div>
)}

Expand Down Expand Up @@ -374,7 +421,14 @@ const TailwindInput = observer(() => {
: 'bg-background-secondary/75 text-foreground-muted border-background-secondary/75 group-hover:bg-purple-100/50 group-hover:text-purple-900 group-hover:border-purple-200 dark:group-hover:bg-purple-900/30 dark:group-hover:text-purple-100 dark:group-hover:border-purple-900/30 cursor-pointer',
)}
placeholder="Add tailwind classes here"
value={instanceHistory.present}
value={
instanceHistory.error
? 'Warning: ' +
instanceHistory.error +
' Open the code to edit.'
: instanceHistory.present
}
readOnly={!!instanceHistory.error}
onInput={(e) => handleInput(e, instanceHistory, setInstanceHistory)}
onKeyDown={(e) => handleKeyDown(e, instanceHistory, setInstanceHistory)}
onBlur={(e) => {
Expand Down Expand Up @@ -410,7 +464,21 @@ const TailwindInput = observer(() => {
/>
)}
</div>
{isInstanceFocused && <EnterIndicator isInstance={true} />}
{instanceHistory.error ? (
<div className="absolute bottom-1 right-2 text-xs flex items-center text-blue-500 cursor-pointer">
<button
onClick={(e) => {
e.stopPropagation(); // Prevents unfocusing the textarea
navigateToTemplateNode(selectedEl?.oid);
}}
className="underline"
>
Go to source
</button>
</div>
) : (
isInstanceFocused && <EnterIndicator />
)}
</div>
)}
</div>
Expand Down
11 changes: 11 additions & 0 deletions packages/models/src/element/classes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
interface ParsedClasses {
type: 'classes';
value: string[];
}

interface ClassParsingError {
type: 'error';
reason: string;
}

export type ClassParsingResult = ParsedClasses | ClassParsingError;
33 changes: 33 additions & 0 deletions packages/models/src/element/element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { z } from 'zod';

const BaseDomElementSchema = z.object({
domId: z.string(),
webviewId: z.string(),
oid: z.string().nullable(),
instanceId: z.string().nullable(),
rect: z.instanceof(DOMRect),
});

export const ParentDomElementSchema = BaseDomElementSchema;

export const DomElementSchema = BaseDomElementSchema.extend({
tagName: z.string(),
styles: z.record(z.string(), z.string()),
parent: ParentDomElementSchema.nullable(),
});

export const ElementPositionSchema = z.object({
x: z.number(),
y: z.number(),
});

export const DropElementPropertiesSchema = z.object({
tagName: z.string(),
styles: z.record(z.string(), z.string()),
textContent: z.string().nullable(),
});

export type DomElement = z.infer<typeof DomElementSchema>;
export type ParentDomElement = z.infer<typeof ParentDomElementSchema>;
export type ElementPosition = z.infer<typeof ElementPositionSchema>;
export type DropElementProperties = z.infer<typeof DropElementPropertiesSchema>;
36 changes: 2 additions & 34 deletions packages/models/src/element/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,4 @@
import { z } from 'zod';

const BaseDomElementSchema = z.object({
domId: z.string(),
webviewId: z.string(),
oid: z.string().nullable(),
instanceId: z.string().nullable(),
rect: z.instanceof(DOMRect),
});

export const ParentDomElementSchema = BaseDomElementSchema;

export const DomElementSchema = BaseDomElementSchema.extend({
tagName: z.string(),
styles: z.record(z.string(), z.string()),
parent: ParentDomElementSchema.nullable(),
});

export const ElementPositionSchema = z.object({
x: z.number(),
y: z.number(),
});

export const DropElementPropertiesSchema = z.object({
tagName: z.string(),
styles: z.record(z.string(), z.string()),
textContent: z.string().nullable(),
});

export type DomElement = z.infer<typeof DomElementSchema>;
export type ParentDomElement = z.infer<typeof ParentDomElementSchema>;
export type ElementPosition = z.infer<typeof ElementPositionSchema>;
export type DropElementProperties = z.infer<typeof DropElementPropertiesSchema>;

export * from './classes';
export * from './element';
export * from './layers';
export * from './templateNode';