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.

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

Conversation

cleobnvntra
Copy link
Contributor

@cleobnvntra cleobnvntra commented Dec 5, 2024

Description

Fixes #799

  • Added logic to display a warning message when dynamic variables are used in className.
  • Displays Tailwind classes if no dynamic variables are present in the template literal.
  • Shows a button in the UI to navigate to the relevant templateNode source code when dynamic variables are used.
  • Updated the button name to improve user clarity.

With template literals

Now properly displays Tailwind Classes even if template literals were used, but no dynamic variables were used.

Selected div for reference:

<div
  className={`w-full min-h-screen flex items-center justify-center bg-red-50 relative overflow-hidden flex-col`}
  data-oid=":gge2lo"
>
// ...
</div>

Tailwind Classes textarea:
image

Template literals with dynamic variables

Selected div for reference:

const someClassName = "text-center text-gray-900 relative z-10";

<div className={`m-8 p-8 ${someClassName}`} data-oid="adqx-me">
// ...
</div>

Tailwind Classes textarea:
image

As asked, clicking the Go to source button in the textarea brings the user to the relevant template node in the source code.

What is the purpose of this pull request?

  • New feature
  • Documentation update
  • Bug fix
  • Refactor
  • Release
  • Other

- Added logic to display a warning message when dynamic variables are used in className.
- Displays Tailwind classes if no dynamic variables are present in the template literal.
- Shows a button in the UI to navigate to the relevant templateNode source code when dynamic variables are used.
- Updated the button name to improve user clarity.
Copy link
Contributor

@Kitenite Kitenite left a comment

Choose a reason for hiding this comment

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

This is great! You got the core of the application down at this point. Just a few suggested improvements. Lmk what you think :)

apps/studio/electron/main/code/classes.ts Outdated Show resolved Hide resolved
@@ -35,5 +35,23 @@ function getNodeClasses(node: t.JSXElement): string[] {
return 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!

cleobnvntra and others added 6 commits December 5, 2024 19:19
- Added logic to display a warning message when dynamic variables are used in className.
- Displays Tailwind classes if no dynamic variables are present in the template literal.
- Shows a button in the UI to navigate to the relevant templateNode source code when dynamic variables are used.
- Updated the button name to improve user clarity.
- Added special return types for the `getTemplateNodeClass` function.
Copy link
Contributor

@Kitenite Kitenite left a comment

Choose a reason for hiding this comment

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

Hey this is working great! Gonna be super helpful since this is a super common occurrence.

I just did some refactoring and moved the error reason outside of the classes value object so it doesn't interfere with edit history. Nice work!

@Kitenite Kitenite merged commit df61500 into onlook-dev:main Dec 6, 2024
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.

[bug] Handle dynamic className in tailwind input
2 participants