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

Master container style fix for 4235 #4237

Merged
merged 3 commits into from
Jan 18, 2024

Conversation

puckowski
Copy link
Contributor

What:

Fixes issue #4235

style keyword would have invalid space after keyword in container queries.

Why:

With the invalid space, the CSS is not parsed correctly by browsers.

Checklist:

  • Documentation
  • Added/updated unit tests
  • Code complete

Tests passing locally. Updated one test case for container queries.

parse the correct entities for a comma separated list so that all URLs are
rewritten correctly.
Copy link
Member

@matthew-dean matthew-dean left a comment

Choose a reason for hiding this comment

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

style is much too specific. More function-like constructs are being added and more may follow. Please make parsing more generic and look at function parsing to correctly capture parsing here.

fixes issue less#4235 where container query style would have invalid space
after keyword
@puckowski
Copy link
Contributor Author

Rewrote solution to use function parsing to be more flexible for future functions in declarations of at-rules. Updated container at-rule tests. Tests pass locally. @matthew-dean

@matthew-dean
Copy link
Member

@puckowski Thanks!

@puckowski
Copy link
Contributor Author

@iChenLei Do you believe we can merge this pull request?

@iChenLei
Copy link
Member

cc @matthew-dean

@puckowski
Copy link
Contributor Author

@matthew-dean Do you believe we can merge this pull request?

I'd like to update some dependent projects to add support for --style.

@matthew-dean
Copy link
Member

@puckowski Oh yup, let's do it. Holidays + covid made me miss this.

@matthew-dean matthew-dean merged commit 53f84f0 into less:master Jan 18, 2024
8 checks passed
@dominikschreiber
Copy link

Hi @matthew-dean, @iChenLei ! Would you be willing to create a new less release that contains this fix? Depending on the less master branch can only be a temporary solution (there are environments where only access to the official npmjs.com registry or a mirror is allowed, not to github.com). Still, this PR fixes a rather unforeseen bug, in which only the first call() entity of a comma-separated list of entities is properly parsed:

lessc -v
# lessc 4.2.0 (Less Compiler) [JavaScript]
echo ".good {box-shadow: 0 0 1px fade(#000, 20%), 0 0 2px fade(#000, 50%);}" | lessc -
# .good {
#   box-shadow: 0 0 1px rgba(0, 0, 0, 0.2), 0 0 2px rgba(0, 0, 0, 0.5);
# }
echo ".bad {--shadow: 0 0 1px fade(#000, 20%), 0 0 2px fade(#000, 50%); box-shadow: var(--shadow);}" | lessc -
# .bad {
#   --shadow: 0 0 1px rgba(0, 0, 0, 0.2), 0 0 2px fade(#000, 50%);
#   box-shadow: var(--shadow);
# }

@puckowski
Copy link
Contributor Author

CSS Container Style Queries are available in Safari 18 beta.

https://caniuse.com/css-container-queries-style
https://www.webkit.org/blog/15443/news-from-wwdc24-webkit-in-safari-18-beta/

Would be great if we could prepare a 4.2.1 release with support for style queries.

@iChenLei @matthew-dean

@iChenLei
Copy link
Member

Hi, I would love to help publish a new version, but I don't have publishing permissions actually,I'm so sorry.

@woody-li
Copy link

@matthew-dean Could you help to publish a new version for this?

@matthew-dean
Copy link
Member

@iChenLei You may not have publishing permissions but you (or anyone?) could prepare a release branch.

  1. Create a branch from master release/v4.2.1
  2. Bump the Less version. (Unfortunately, this is still clunky, it's changed at package.json in root, and in /packages/less/ for historical reasons)
  3. Update the Changelog
  4. Run grunt dist - (Also clunky -- in a perfect world, we would not be committing dist files, but there are also tricky backwards compatibility problems which caused this initially.)
  5. Commit changed files to branch.
  6. Submit a PR to this repo.

@puckowski puckowski mentioned this pull request Sep 26, 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.

5 participants