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

fix: ignore relative selector lists when resolving multiple selectors #921

Conversation

1000hz
Copy link
Contributor

@1000hz 1000hz commented Aug 20, 2024

Description

fela-multiple-selectors-plugin currently naively splits selectors on ,, but this doesn't account for relative selector lists that may be within a pseudo-class. This results in invalid selectors being resolved.

This PR fixes that issue by making use of lookahead and lookbehind assertions when splitting to avoid matching within pseudo-class selector lists.

Example

:not(:hover, :focus) {}

currently resolves to :not(:hover and :focus), both of which are invalid.

With this PR, the selector remains whole as :not(:hover, :focus).

Packages

List all packages that have been changed.

  • fela-plugin-multiple-selectors

Versioning

Patch

Checklist

Quality Assurance

You can also run pnpm run check to run all 4 commands at once.

  • The code was formatted using Prettier (pnpm run format)
  • The code has no linting errors (pnpm run lint)
  • All tests are passing (pnpm run test)

Changes

If one of the following checks doesn't make sense due to the type of PR, just check them.

  • Tests have been added/updated
  • Documentation has been added/updated

Copy link

vercel bot commented Aug 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fela ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 20, 2024 3:43pm

@robinweser robinweser merged commit 6883525 into robinweser:master Nov 6, 2024
2 of 3 checks passed
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.

2 participants