-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add proper support for :not() #115
Conversation
@ghengeveld after this PR, I also have a change that adds support for |
@ghengeveld seems like this stalled out. Anything I can do to help? |
@m-akinc We had a company retreat last week so I was preoccupied. I'll take a look soon. |
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.
Nice work as always!
Dang, seems like this broke a bunch of stories. Could you check it out @m-akinc? Perhaps address it in that follow-up PR? |
@ghengeveld Sorry for that! Fix is up in #117. Another PR adding support for |
🚀 PR was released in |
The code avoided generating invalid CSS when a rule included
:not()
, but it did not provide correct support for thepseudo-*-all
classes. For example,:not(:hover)
would be transformed to.pseudo-hover-all :not()
, but we would skip writing that rule, because of the invalid (i.e. empty):not()
. The correct rewriting of that rule is:not(.pseudo-hover-all *)
, i.e. "Not a descendant of an element with class.pseudo-hover-all
". Similarly, for shadow DOM styles, we need special logic to turn the example rule into:not(:host(.pseudo-hover-all) *)
.In general, the approach is:
:not(...<pseudo-state>...)
with:not(.pseudo-<state>-all ...)
(or:not(:host(.pseudo-<state>-all) ...)
if for shadow DOM)...<pseudo-state>...
->.pseudo-<state>-all ...
Additionally, the refactoring I did allowed the removal of special logic related to
::slotted()
and:has()
.