-
Notifications
You must be signed in to change notification settings - Fork 687
Fix(frontend): auto completion and render of tailwind classes in app editor #5817
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to a39c6a1 in 1 minute and 59 seconds. Click for details.
- Reviewed
89
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
2
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/src/lib/components/SimpleEditor.svelte:484
- Draft comment:
Review the startColumn calculation (using lastIndexOf(' ') + 2). It may not correctly cover cases where a Tailwind class follows punctuation. Consider using a more robust delimiter or boundary detection. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment raises a valid point - the code assumes classes are only separated by spaces, but Tailwind classes could appear after other characters like brackets, quotes, etc. However, this is a new feature being added and the current implementation is a reasonable first version. The comment is speculative ("may not correctly cover") rather than pointing out a definite issue. The comment identifies a real edge case that could be problematic. The current implementation could miss valid completion opportunities. While technically correct, this is a new feature addition and the current approach handles the common case. The comment is speculative and doesn't demonstrate an actual problem. Delete the comment. While it raises a valid potential edge case, it's speculative and the current implementation is a reasonable first version of this new feature.
2. frontend/tailwind.config.cjs:85
- Draft comment:
Good job including the imported tailwindClasses in the safelist. Ensure that tailwindClasses is always an array to avoid potential runtime errors during Tailwind’s build. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
Workflow ID: wflow_u78d2rTqc3ioCci6
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
// Define a custom word definition for Tailwind classes | ||
languages.setMonarchTokensProvider('tailwindcss', { | ||
tokenizer: { | ||
root: [[/[a-zA-Z0-9-]+/, 'tailwind-class']] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider updating the Monarch token regex to include additional characters (like :
) for Tailwind variant syntax (e.g. md:text-center
).
root: [[/[a-zA-Z0-9-]+/, 'tailwind-class']] | |
root: [[/[a-zA-Z0-9:-]+/, 'tailwind-class']] |
Deploying windmill with
|
Latest commit: |
f67d393
|
Status: | ✅ Deploy successful! |
Preview URL: | https://a56da97b.windmill.pages.dev |
Branch Preview URL: | https://glm-fix-frontend-tailwind-au.windmill.pages.dev |
/ai remove tailwind_full.css asset and links in apppreview and appeditor and add tailwindUtils to the exported classes in package.json |
Claude finished @rubenfiszel's task —— View job ✅ Completed: Removed tailwind_full.css and added tailwindUtils exportChanges Made:
Summary: Note: Frontend dependencies were installed successfully. You may want to run Commit: |
…exports - Removed `<link rel="stylesheet" href="/tailwind_full.css" />` from AppEditor.svelte and AppPreview.svelte - Added `"./tailwindUtils"` export to package.json exports section for external consumption - Added tailwindUtils to typesVersions section for TypeScript support Co-authored-by: rubenfiszel <rubenfiszel@users.noreply.github.com>
const textUntilPosition = lineContent.substring(0, position.column - 1) | ||
// Find the last space before the cursor | ||
const lastSpaceIndex = textUntilPosition.lastIndexOf(' ') | ||
const startColumn = lastSpaceIndex === -1 ? 1 : lastSpaceIndex + 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The calculation lastSpaceIndex + 2
appears to position the cursor two characters after a space, which may skip a character. Consider using lastSpaceIndex + 1
instead to position the cursor immediately after the space. This would ensure the completion range starts at the correct position for the current word being typed.
const startColumn = lastSpaceIndex === -1 ? 1 : lastSpaceIndex + 2 | |
const startColumn = lastSpaceIndex === -1 ? 1 : lastSpaceIndex + 1 |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
// Define a custom word definition for Tailwind classes | ||
languages.setMonarchTokensProvider('tailwindcss', { | ||
tokenizer: { | ||
root: [[/[a-zA-Z0-9-]+/, 'tailwind-class']] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern [a-zA-Z0-9-]+
doesn't account for all valid Tailwind class characters. Tailwind classes commonly include colons for variants (e.g., hover:
, focus:
) and slashes for fractional values (e.g., w-1/2
).
Consider updating the pattern to:
/[a-zA-Z0-9-:\/]+/
This would ensure proper recognition of the full range of Tailwind class syntax during autocompletion.
root: [[/[a-zA-Z0-9-]+/, 'tailwind-class']] | |
root: [[/[a-zA-Z0-9-:\/]+/, 'tailwind-class']] |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Important
Fixes Tailwind class rendering and improves autocompletion in the app editor by updating
SimpleEditor.svelte
andtailwind.config.cjs
.tailwindClasses
to the safelist intailwind.config.cjs
.SimpleEditor.svelte
by adjusting word detection and completion range.addTailwindClassCompletions()
inSimpleEditor.svelte
.SimpleEditor.svelte
.triggerCharacters
for Tailwind class completion inSimpleEditor.svelte
.This description was created by
for a39c6a1. You can customize this summary. It will automatically update as commits are pushed.