Skip to content

Fix for issue 4313 selector list not expanding #4314

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Fix for issue 4313 selector list not expanding #4314

wants to merge 14 commits into from

Conversation

puckowski
Copy link
Contributor

What:

PR contains a fix for issue #4313 selector list not expanding with & symbol.

Why:

In Less.js version 4.2.2, this Less:

.broken {
  &:not(&--a, &--b):hover {
    background-color: green;
  }
}

produces the incorrect CSS:

.broken:not(&--a, &--b):hover {
  background-color: green;
}

the & symbol should now be properly expanded with the fix.

Checklist:

  • Documentation
  • Added/updated unit tests
  • Code complete

@puckowski
feat: add support for container scroll-state
55f7448 
* Add support for CSS container query scroll-state.
@puckowski
fix: issue less#4313 selector with comma
6c1f2f3 
* Fix an issue where selector with comma would not expand correctly.
@puckowski
fix: issue less#4313 make solution more flexible
ab0e64f 
* Make fix for issue less#4313 selector list not expanding correctly more
  flexible.
@puckowski
fix: issue less#4313 add more tests
8a71e56 
* Add more tests for fix for issue less#4313.
@puckowski
fix: issue less#4313 cleanup merge issues
75e212c 
* Cleanup merge issues for fix for issue less#4313.
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Feb 1, 2025
@puckowski
Copy link
Contributor Author

Looks like CI may have an issue. Tests pass locally for me.

browserType.launch: Executable doesn't exist at /home/runner/.cache/ms-playwright/chromium_headless_shell-1148/chrome-linux/headless_shell
╔═════════════════════════════════════════════════════════════════════════╗
║ Looks like Playwright Test or Playwright was just installed or updated. ║
║ Please run the following command to download new browsers:              ║
║                                                                         ║
║     npx playwright install                                              ║
║                                                                         ║
║ <3 Playwright Team                                                      ║
╚═════════════════════════════════════════════════════════════════════════╝

@puckowski
Copy link
Contributor Author

I re-ran CI but ran into issues. Branch passes all tests locally for me. I wouldn't expect files changed to impact CI (two test files, one source file). Would you be able to take a look? @iChenLei

@puckowski puckowski mentioned this pull request Feb 1, 2025
Open
@puckowski
fix: revise CI for missing browser issue
4768561 
* Revise CI for missing browser issue.
@puckowski
fix: trying to resolve CI issues
d137778 
* Trying to resolve CI issues with Playwright.
@puckowski
fix: trying to resolve CI issues
0111faf 
* Trying to resolve CI issues with Playwright.
@puckowski
fix: trying to resolve CI issues
f5ac003 
* Trying to resolve CI issues with Playwright.
@puckowski
fix: fix CI by pinning Playwright version
2f69d30 
* Fix CI by pinning CI version to resolve incorrect browser binary
  downloads.
@puckowski
Copy link
Contributor Author

FYI figured out the new CI issue. Had to revise CI file to pin Playwright version to resolve browser binary mismatches. @iChenLei @matthew-dean

See files changed in this PR.

WORMSS
WORMSS reviewed Feb 1, 2025
View reviewed changes
matthew-dean
matthew-dean reviewed Feb 12, 2025
View reviewed changes
@puckowski
fix: imrpove CI stability and add comments
9e65c09 
* Improve CI stability by only grabbing patch updates. This may when npm
  install runs we will not download a version of Playwright which will
  be incompatible with the browser binaries installed by CI.
* Add CI code comments about the Playwright issue that we ran into.
@puckowski
chore: remove package.json changes
97770e4 
* Remove package.json changes per master branch updates.
@puckowski
Merge branch 'master' into master_fix_4313
520a37b
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Mar 3, 2025
@puckowski
chore: cleanup CI changes
4449a86 
* Cleanup CI changes per master branch updates to CI.
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Mar 3, 2025
@puckowski
Copy link
Contributor Author

PR was updated and is now more narrow in scope. Let me know if you have an opportunity to review. @matthew-dean

@woody-li woody-li mentioned this pull request Aug 28, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthew-dean matthew-dean matthew-dean left review comments

+1 more reviewer

@WORMSS WORMSS WORMSS left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@puckowski @matthew-dean @WORMSS