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

[bug] Handle dynamic className in tailwind input #799

Closed
Kitenite opened this issue Nov 18, 2024 · 7 comments · Fixed by #849
Closed

[bug] Handle dynamic className in tailwind input #799

Kitenite opened this issue Nov 18, 2024 · 7 comments · Fixed by #849
Assignees
Labels
bug Something isn't working

Comments

@Kitenite
Copy link
Contributor

Describe the bug

We're currently not returning anything if the className is anything but a string. We may want to handle this better by warning the user that this is dynamic and let them open it in code.

Screenshot 2024-11-18 at 4 06 55 PM
@Kitenite Kitenite added the bug Something isn't working label Nov 18, 2024
@cleobnvntra
Copy link
Contributor

Hello, I would like to give this issue a try!

@Kitenite
Copy link
Contributor Author

Go for it @cleobnvntra. This is a good place to start. This is used in TailwindInput and we can augment the return type to potentially pass a warning

export async function getTemplateNodeClass(templateNode: TemplateNode): Promise<string[]> {

@cleobnvntra
Copy link
Contributor

Hello @Kitenite ! So I've been able to understand and figure out how to determine whether a className is a static string or a template literal.

I am not sure however what kind/type of warning you prefer, so for now I made it so that it displays a message in the Tailwind Classes textarea which also has the readOnly prop:

image

With that said, I also wanted to clarify, do we only want to show a warning message when a dynamic variable is used? or do we display a warning message regardless as long as a template literal is used (even without a dynamic variable).

@cleobnvntra
Copy link
Contributor

UPDATE:

I realized that I was still working on an older version of the code, so I have pulled the latest main of the repo and noticed that the Tailwind Classes textarea doesn't seem to be getting displayed.
image

I just want to confirm is this only an issue on my end.

@Kitenite
Copy link
Contributor Author

Kitenite commented Dec 3, 2024

so I have pulled the latest main of the repo and noticed that the Tailwind Classes textarea doesn't seem to be getting displayed.

@cleobnvntra , in the new version, onlook will only work if ran from the terminal from the toolbar. Could you confirm that it is running? The tailwind class missing is from Onlook not being enabled usually

@Kitenite
Copy link
Contributor Author

Kitenite commented Dec 3, 2024

I am not sure however what kind/type of warning you prefer, so for now I made it so that it displays a message in the Tailwind Classes textarea which also has the readOnly prop

That looks great, could we surface a button that opens that templateNode as well in the warning?

With that said, I also wanted to clarify, do we only want to show a warning message when a dynamic variable is used? or do we display a warning message regardless as long as a template literal is used (even without a dynamic variable).

I think we should display when it's a situation where we can't simply text edit. If it's a template variable wrapping a string, we should be able to handle it (if we're not already). Does that help? @cleobnvntra

@cleobnvntra
Copy link
Contributor

@cleobnvntra , in the new version, onlook will only work if ran from the terminal from the toolbar. Could you confirm that it is running? The tailwind class missing is from Onlook not being enabled usually

Oh I didn't realize this was necessary with the new version. Yep it works now! :)

That looks great, could we surface a button that opens that templateNode as well in the warning?

Sure! I was thinking maybe a button on the lower right corner where the Enter to apply text appears would show a button instead in this situation.

I think we should display when it's a situation where we can't simply text edit. If it's a template variable wrapping a string, we should be able to handle it (if we're not already). Does that help? @cleobnvntra

Yes that helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants