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

Implementation of a insideNonNestedAtRule variable #2142

Closed
wants to merge 10 commits into from
Closed

Implementation of a insideNonNestedAtRule variable #2142

wants to merge 10 commits into from

Conversation

mho22
Copy link
Contributor

@mho22 mho22 commented Apr 18, 2023

Description

  • Source branch in your fork has meaningful name (not main)

Fixes Issue:

Implementation of a insideAtApply variable to determine if it is a Tailwind ruleset and avoid whitespace after variant [ currently thinking it is a property ]

It fixes issue #2012 :

Before :

@apply flex flex-col lg:flex-row space-y-3 lg:space-x-12 items-start;

After :

@apply flex flex-col lg: flex-row space-y-3 lg:space-x-12 items-start;

Before Merge Checklist

These items can be completed after PR is created.

(Check any items that are not applicable (NA) for this PR)

  • JavaScript implementation
  • Python implementation (NA if HTML beautifier)
  • Added Tests to data file(s)
  • Added command-line option(s) (NA if
  • README.md documents new feature/option(s)

Implementation of a insideAtApply variable to determine if it is a Tailwind ruleset and avoid whitespace after variant [ currently thinking it is a property ]
@mho22 mho22 changed the title Implementation of a insideAtApply variable Implementation of a insideNonNestedAtRule variable Apr 19, 2023
@mho22
Copy link
Contributor Author

mho22 commented Apr 19, 2023

I previously focused only on an implementation to make @apply at-rule statement work. However, this was too focused on Tailwindcss utility-first framework. Instead I supposed it was a custom non nested at-rule like others could be.

I figured out it could maybe be a generic functionality for @import an @extends too. I didn't want to modify the existing code before asking here, but maybe insideNonNestedAtRule could replace insideAtImport and insideAtExtends variables ?

Copy link
Member

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

Great! Please add some tests showing the fixed functionality!

@mho22
Copy link
Contributor Author

mho22 commented Apr 23, 2023

@bitwiseman I added some commits to improve my proposition. I added the tests, launched make js and everything worked fine.

  • I separated the '@' and '$' identification because of losing information about the this._ch after this part :
  variableOrRule = this.eatString(": ").replace(/\s$/, '');
  this.print_string(variableOrRule);
  • I considered after determining if it was not a less variable, it should be an at rule. I checked if it was in the NESTED_AT_RULE array [ and because it had no more its @ character, it never entered in the condition, I had to rename the array elements removing the particular @character ] else it was probably a non nested at rule.

  • It made a generic check of default and custom at rules. I cleaned the @extends and @import variables.

  • I ran the tests and everything went fine.

What do I need to do to make this work correctly for you ?

@mho22 mho22 requested a review from bitwiseman April 23, 2023 13:27
@bitwiseman
Copy link
Member

@itemshopp
Can you please verify something for me. When I go to https://beautifier.io/ , select CSS, and put in:

@apply flex flex-col lg:flex-row space-y-3 lg:space-x-12 items-start;
@apply text-[clamp(0.75rem,0.5625rem+0.3906vw,0.875rem)];

What I get currently is:

@apply flex flex-col lg:flex-row space-y-3 lg:space-x-12 items-start;
@apply text-[clamp(0.75rem, 0.5625rem+0.3906vw, 0.875rem)];

That is without this PR. Are the spaces after the commas acceptable? If so, we can close #2012 without taking this PR.

@mho22
Copy link
Contributor Author

mho22 commented Apr 28, 2023

@bitwiseman Hi !

I did what you asked :

Chose Beautify CSS

before

@apply flex flex-col lg:flex-row space-y-3 lg:space-x-12 items-start;
@apply text-[clamp(0.75rem,0.5625rem+0.3906vw,0.875rem)];

Clicked on Beautify Code

after :

@apply flex flex-col lg:flex-row space-y-3 lg:space-x-12 items-start;
@apply text-[clamp(0.75rem, 0.5625rem+0.3906vw, 0.875rem)];

It works ! I suppose this was probably already corrected in a previous merge ? If so, I'm sorry about my PR suggestion.

@bitwiseman
Copy link
Member

Thanks for the PR! There are probably still other issues with tailwind beautification, so I'll leave the original issue open for now.

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.

None yet

2 participants